Skip to content

[stdlib] String.Index: conform to CustomDebugStringConvertible #75433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 9, 2024

Conversation

lorentey
Copy link
Member

@lorentey lorentey commented Jul 24, 2024

This PR conforms String.Index to CustomStringConvertible, making it easier to understand what these indices actually are, and considerably simplifying the debugging experience when working with string indices.

String indices currently print by dumping their raw contents, which is extraordinarily unhelpful:

let string = "👋🏼 هلو"

print(string.startIndex) // ⟹ Index(_rawBits: 15)
print(string.endIndex) // ⟹ Index(_rawBits: 983047)

With this PR, indices print by displaying their storage offset and encoding in a succinct format:

print(string.startIndex) // ⟹ "0[any]"
print(string.endIndex) // ⟹ "15[utf8]"

The specific format of the returned string and the information in it remains subject to change between Swift releases.

Related previous PRs:

Swift Evolution proposal:

rdar://22794467

@lorentey lorentey added the swift evolution proposal needed Flag → feature: A feature that warrants a Swift evolution proposal label Jul 24, 2024
@lorentey
Copy link
Member Author

@swift-ci test

@lorentey
Copy link
Member Author

The first test run is expected to fail. (The ABI verifier will need updates, but I'm hoping that's all.)

/// A textual representation of this instance.
@_alwaysEmitIntoClient
@backDeployed(before: SwiftStdlib 6.1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the Sequence.count(where:) method was re-implemented, there was a comment that the standard library can't effectively use @backDeployed yet.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the issue was back then, but hopefully it's been resolved since then 🤞

Copy link
Member Author

@lorentey lorentey Jul 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One seemingly minor issue that keeps causing trouble is that @backDeployed does not gracefully handle mid-release ABI transitions -- people whose SDK is out of phase with their running OS may end up building code that fails to link at runtime.

Another issue I recently discovered is that @backDeployed functions cannot be used to fulfill a member requirement in a retroactive protocol conformance. It is tempting to try backdeploying the CustomStringConvertible conformance by putting this in a public package somewhere:

extension String.Index: @retroactive CustomStringConvertible {
 // `description` comes straight from the stdlib
}

However, if description is marked @backDeployed, then this fails at runtime on versions predating its introduction. (This is clearly a bug, as @_alwaysEmitIntoClient functions do work in the same context.)


I think we'll want to keep String.Index.description as opaque as possible -- it cannot be defined fully @_aeic, or we would give up the flexibility to change its implementation later. At the same time, I also want the function to deploy back to earlier versions.

This is the alternative I'm considering -- it's kind of a homegrown @backDeploy, built from chewing gum and twine.

extension String.Index {
  @_alwaysEmitIntoClient
  public var description: String {
    if #available(SwiftStdlib 6.1, *) {
      return _descriptionImpl()
    }
    return _description
  }

  @_alwaysEmitIntoClient
  @inline(never)
  public var _description: String { // Preexisting member
    // 23[utf8]+1
    var d = "\(_encodedOffset)[\(_encodingDescription)]"
    if transcodedOffset != 0 {
      d += "+\(transcodedOffset)"
    }
    return d
  }

  @available(SwiftStdlib 6.1, *)
  @usableFromInline
  internal func _descriptionImpl() -> String {
    _description
  }
}

I don't love that the exported symbol name is different from String.description, but this may be a workable alternative. It makes it easy to tweak the description in future releases of the stdlib, without affecting its result when run on older versions.

Luckily, these are implementation details that do not matter for Swift Evolution. (We do promise that the property will be backdeployable, but the mechanism for that and the precise list of new symbols that the stdlib will expose are not normative.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the alternative I'm considering -- it's kind of a homegrown @backDeploy, built from chewing gum and twine.

If that alternative works, perhaps it should be documented alongside CRISP. However, if the pre-existing _description can still be used, why does description need to be back-deployable, when the conformance to CustomStringConvertible cannot be back-deployed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swift does not allow the stdlib to backdeploy the conformance, but it also doesn't forbid user code to ship a retroactive conformance with the same effect:

It is tempting to try backdeploying the CustomStringConvertible conformance by putting this in a public package somewhere:

extension String.Index: @retroactive CustomStringConvertible {
 // `description` comes straight from the stdlib
}

I am toying with the idea to suggest publishing an official stdlib companion package to ship such things. (This is not crucial for CustomStringConvertible, but back-deployment will be very desirable for all the upcoming Equatable/Hashable conformances on string views -- we cannot live without them.)

@lorentey
Copy link
Member Author

lorentey commented Aug 1, 2024

@swift-ci test macOS platform

@lorentey
Copy link
Member Author

@swift-ci test

@lorentey lorentey added swift evolution approved Flag → feature: A feature that was approved through the Swift evolution process and removed swift evolution proposal needed Flag → feature: A feature that warrants a Swift evolution proposal labels Oct 7, 2024
This better exposes the internals of string indices, demystifying their operation and radically simplifying working with them.
…format is not stable

This tends to be generally through for most `description` properties, but explicitly calling it out should help preventing people from trying to parse the result in this case.
@lorentey lorentey force-pushed the string-index-printing2 branch from a02dd26 to d816f88 Compare October 7, 2024 23:09
@lorentey lorentey changed the title [stdlib] String.Index: conform to CustomStringConvertible [stdlib] String.Index: conform to CustomDebugStringConvertible Oct 7, 2024
@lorentey lorentey force-pushed the string-index-printing2 branch from d816f88 to d91d8da Compare October 7, 2024 23:13
@lorentey
Copy link
Member Author

lorentey commented Oct 7, 2024

@swift-ci test

Apply the LSG’s modifications as detailed in their review notes.
@lorentey lorentey force-pushed the string-index-printing2 branch from d91d8da to 29cf262 Compare October 8, 2024 00:00
@lorentey
Copy link
Member Author

lorentey commented Oct 8, 2024

@swift-ci test

@lorentey
Copy link
Member Author

lorentey commented Oct 8, 2024

@swift-ci test

@lorentey
Copy link
Member Author

lorentey commented Oct 8, 2024

@swift-ci test source compatibility

@lorentey lorentey marked this pull request as ready for review October 8, 2024 18:12
@lorentey lorentey requested a review from a team as a code owner October 8, 2024 18:12
@lorentey lorentey merged commit ee4d4a3 into swiftlang:main Oct 9, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
swift evolution approved Flag → feature: A feature that was approved through the Swift evolution process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants