[SE-0456] Span properties over utf8 views #80116

glessard · 2025-03-19T12:14:57Z

Implements most of the String-related parts of SE-0456 not implemented in #78561

Spans vended from the small string representation are disabled at the moment

Addresses the remainder of rdar://137710901

glessard · 2025-03-19T22:22:31Z

@swift-ci please smoke test

stdlib/public/core/StringGuts.swift

glessard · 2025-03-20T00:01:41Z

@swift-ci please test

glessard · 2025-03-20T15:54:23Z

@swift-ci please test

glessard · 2025-03-20T21:04:51Z

@swift-ci please test macOS platform

glessard · 2025-03-21T05:53:28Z

@swift-ci please test macOS platfom

glessard · 2025-03-21T15:30:00Z

The full macOS test is failing due to swift-testing. Falling back to smoke-test.

glessard · 2025-03-21T15:30:09Z

@swift-ci please smoke test macOS platform

glessard · 2025-03-24T20:19:01Z

@swift-ci please test

glessard · 2025-03-25T16:50:05Z

@swift-ci please test

stdlib/public/core/StringUTF8View.swift

Catfish-Man · 2025-03-27T21:58:15Z

stdlib/public/core/Substring.swift

+      // handle non-UTF8 Objective-C bridging cases here
+      if !_wholeGuts.isFastUTF8 && _wholeGuts._object.hasObjCBridgeableObject {
+        let base: String.UTF8View = self._base
+        let first = base._foreignDistance(from: base.startIndex, to: startIndex)


This is fine and I'm not suggesting changing it now, but I wonder if we can avoid a bunch of -characterAtIndex: calls by making a version of this that operates on the associated buffer directly

In this case the indices are to the UTF16 buffer, so a translation is needed. I'd like @lorentey's eyes on this one.

The implementation looks correct to me, but I think David is right, performance is going to be a real issue. 😫

The point of these span properties is that they provide fast direct access to in-memory storage; doing a full UTF-16 → UTF-8 transcoding of an arbitrarily large prefix in the base string (plus the entire contents of the substring itself) every time span is accessed is violating this goal. We want to encourage developers to think of these properties as cheap enough to call whenever and wherever they need them; loops like the one below should not raise any eyebrows:

for word in string.split(separator: / /) { let span = word.span // oops ... }

The one-time transcoding of the base string is often amortized away to near-zero with repeated calls to span, but the two index conversions in the substring's implementation will be repeated from scratch on every iteration. Straightforward-looking loops will become accidentally quadratic.

Unfortunately the only way to avoid this footgun I can see is to not provide this property on this type at all; we should consider doing that!

If we do end up wanting to keep this property, then I think the doc comment needs to loudly explain this major gotcha, and it should recommend strategies to avoid hitting it. (E.g., by first copying the substring into a standalone string instance.)

for word in string.split(separator: / /) { let str = String(word) let span = str.span // OK ... }

It's worth noting that we're steadily reducing the number of cases this will apply to, but yes, we don't currently have a strategy for getting that number to zero

stdlib/public/core/Substring.swift

test/stdlib/Span/BridgedStringUTF8ViewSpanTests.swift

Catfish-Man

The associated objects approach is miserable, but we knew that. Changes look good

lorentey · 2025-03-28T01:36:58Z

stdlib/public/core/StringUTF8View.swift

+extension String.UTF8View {
+
+  @available(SwiftStdlib 6.2, *)
+  public var span: Span<UTF8.CodeUnit> {


I think we're missing an #if !(os(watchOS) && _pointerBitWidth(_32)) around this. 😭

Oh, and we'll need a doc comment, too -- the complexity (amortized over many calls) is particularly relevant.

lorentey · 2025-03-28T02:08:18Z

stdlib/public/core/Substring.swift

+      // handle non-UTF8 Objective-C bridging cases here
+      if !_wholeGuts.isFastUTF8 && _wholeGuts._object.hasObjCBridgeableObject {
+        let base: String.UTF8View = self._base
+        let first = base._foreignDistance(from: base.startIndex, to: startIndex)


The implementation looks correct to me, but I think David is right, performance is going to be a real issue. 😫

The point of these span properties is that they provide fast direct access to in-memory storage; doing a full UTF-16 → UTF-8 transcoding of an arbitrarily large prefix in the base string (plus the entire contents of the substring itself) every time span is accessed is violating this goal. We want to encourage developers to think of these properties as cheap enough to call whenever and wherever they need them; loops like the one below should not raise any eyebrows:

for word in string.split(separator: / /) { let span = word.span // oops ... }

The one-time transcoding of the base string is often amortized away to near-zero with repeated calls to span, but the two index conversions in the substring's implementation will be repeated from scratch on every iteration. Straightforward-looking loops will become accidentally quadratic.

Unfortunately the only way to avoid this footgun I can see is to not provide this property on this type at all; we should consider doing that!

If we do end up wanting to keep this property, then I think the doc comment needs to loudly explain this major gotcha, and it should recommend strategies to avoid hitting it. (E.g., by first copying the substring into a standalone string instance.)

for word in string.split(separator: / /) { let str = String(word) let span = str.span // OK ... }

lorentey · 2025-03-28T02:15:43Z

stdlib/public/core/Substring.swift

+extension Substring.UTF8View {
+
+  @available(SwiftStdlib 6.2, *)
+  public var span: Span<UTF8.CodeUnit> {


I think in this particular case it is extremely important for this property to have a doc comment that explains its complexity.

Unlike the other span properties, this one has linear time complexity for bridged strings, and that requires developers to be careful to avoid calling it in a loop. (E.g., most Substring.UTF8View operations in the stdlib (and elsewhere) must not use this property, as they have no way to avoid getting called in a loop.)

stdlib/public/core/Substring.swift

stdlib/public/core/StringGuts.swift

lorentey · 2025-03-28T02:50:46Z

stdlib/public/core/StringGuts.swift

+  private static var associationKey: UnsafeRawPointer {
+    // We never dereference this, we only use this address as a unique key
+    unsafe unsafeBitCast(
+      ObjectIdentifier(_StringGuts.self),


How fast is this metatype lookup in practice? (Elsewhere we sometimes decided to use the address of a dummy symbol instead; but this may in fact be the better pattern.)

Oh, on second look, this isn't exactly right -- _StringGuts is a public type, so it is trivial to generate this ID outside the stdlib. It would be better to use the metatype of an internal type here.

(It's not that hard to work out what ID the stdlib uses, but making it too easy sends the wrong signal.)

The two options are to cast about for a suitable type, or make up a private type and use that

Suggested change

ObjectIdentifier(_StringGuts.self),

ObjectIdentifier(__StringStorage.self),

__StringStorage is technically internal, but it is the isa of native Swift strings bridged to ObjC, so its identity is still very public. Something like _StringObject would be a better choice, I think -- a pure internal (non-@usableFromInline) or private struct would be even better.

I'll go with a new made-up private type, then.

stdlib/public/core/StringGuts.swift

- move to a new file - create a new `NSString` object for each new test

glessard · 2025-04-01T05:25:15Z

@swift-ci please test

glessard · 2025-04-01T14:54:33Z

@swift-ci please smoke test macOS platform

glessard · 2025-04-02T00:14:04Z

@swift-ci please test

glessard · 2025-04-02T23:24:49Z

@swift-ci please test macOS platform

Co-authored-by: Karoy Lorentey <[email protected]>

- with tweaks in associated object code

glessard · 2025-04-03T00:54:22Z

@swift-ci please test

glessard mentioned this pull request Mar 19, 2025

[SE-0456] Span properties #78561

Merged

glessard force-pushed the rdar137710901-utf8view-span-notsmol branch 2 times, most recently from 6129e40 to fed549e Compare March 19, 2025 22:22

glessard commented Mar 19, 2025

View reviewed changes

stdlib/public/core/StringGuts.swift Outdated Show resolved Hide resolved

glessard requested review from lorentey, Catfish-Man and milseman March 19, 2025 23:14

glessard force-pushed the rdar137710901-utf8view-span-notsmol branch from fed549e to deee376 Compare March 20, 2025 00:01

glessard force-pushed the rdar137710901-utf8view-span-notsmol branch from deee376 to aab1e19 Compare March 20, 2025 15:53

glessard marked this pull request as ready for review March 20, 2025 15:54

glessard requested a review from a team as a code owner March 20, 2025 15:54

glessard force-pushed the rdar137710901-utf8view-span-notsmol branch from aab1e19 to f1f8c8f Compare March 22, 2025 00:27

glessard force-pushed the rdar137710901-utf8view-span-notsmol branch from a5b3374 to 94a16f3 Compare March 25, 2025 16:49

glessard requested a review from jmschonfeld March 25, 2025 20:02

milseman mentioned this pull request Mar 26, 2025

UTF8Span #78531

Merged

benrimmington reviewed Mar 26, 2025

View reviewed changes

stdlib/public/core/StringUTF8View.swift Show resolved Hide resolved

Catfish-Man reviewed Mar 27, 2025

View reviewed changes

stdlib/public/core/Substring.swift Outdated Show resolved Hide resolved

Catfish-Man reviewed Mar 27, 2025

View reviewed changes

test/stdlib/Span/BridgedStringUTF8ViewSpanTests.swift Show resolved Hide resolved

Catfish-Man approved these changes Mar 27, 2025

View reviewed changes

lorentey reviewed Mar 28, 2025

View reviewed changes

glessard added 8 commits March 31, 2025 12:05

[stdlib] lazy-eager bridging for non-UTF8 NSString instances

af0aa94

[stdlib] span properties for bridged String instances

9ae58c9

[test] spans over bridged Substring instances

03208a6

[stdlib] spans over bridged Substring instances

85a9be6

[stdlib] simplify spans of bridged substrings

6059be4

[stdlib] a better pointer as associated object id

e3234f8

[test] update bridged string tests

17412a9

- move to a new file - create a new `NSString` object for each new test

[test] acknowledge our intentions

9842a79

glessard force-pushed the rdar137710901-utf8view-span-notsmol branch from c772470 to 279c036 Compare March 31, 2025 19:07

glessard requested a review from lorentey March 31, 2025 19:07

glessard force-pushed the rdar137710901-utf8view-span-notsmol branch from 5291f1e to 4d23610 Compare April 1, 2025 05:24

glessard and others added 9 commits April 2, 2025 17:53

[temporary] disable small-string support

ea44ff9

Apply suggestions from code review

0976b61

Co-authored-by: Karoy Lorentey <[email protected]>

[test] adjust test values

c37e152

[stdlib] a less public association key

0bbec0c

- with tweaks in associated object code

[stdlib] harden bounds checking in Substring’s span

bde7a18

[stdlib] document limitations for Substring’s span

f3a0166

[stdlib] document performance limitations on String’s span

5eab446

[stdlib] remove a repetition

278f76b

[stdlib] rename internal and private symbols

7539366

glessard force-pushed the rdar137710901-utf8view-span-notsmol branch from bacebab to 7539366 Compare April 3, 2025 00:54

glessard enabled auto-merge April 3, 2025 00:54

glessard merged commit aedb869 into swiftlang:main Apr 3, 2025
4 of 5 checks passed

This was referenced Apr 4, 2025

[SE-0456, -0467] enable span properties for inline-storage types #80257

Merged

[SE-0456] enable span properties for the small-String representation #80684

Merged

	ObjectIdentifier(_StringGuts.self),
	ObjectIdentifier(__StringStorage.self),

[SE-0456] Span properties over utf8 views #80116

[SE-0456] Span properties over utf8 views #80116

Uh oh!

Conversation

glessard commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glessard commented Mar 19, 2025

Uh oh!

Uh oh!

glessard commented Mar 20, 2025

Uh oh!

glessard commented Mar 20, 2025

Uh oh!

glessard commented Mar 20, 2025

Uh oh!

glessard commented Mar 21, 2025

Uh oh!

glessard commented Mar 21, 2025

Uh oh!

glessard commented Mar 21, 2025

Uh oh!

glessard commented Mar 24, 2025

Uh oh!

glessard commented Mar 25, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Catfish-Man left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lorentey Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lorentey Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

glessard commented Apr 1, 2025

Uh oh!

glessard commented Apr 1, 2025

Uh oh!

glessard commented Apr 2, 2025

Uh oh!

glessard commented Apr 2, 2025

Uh oh!

glessard commented Apr 3, 2025

Uh oh!

Uh oh!

Uh oh!

glessard commented Mar 19, 2025 •

edited

Loading

lorentey Mar 28, 2025 •

edited

Loading

lorentey Mar 28, 2025 •

edited

Loading