-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[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
Conversation
@swift-ci test |
The first test run is expected to fail. (The ABI verifier will need updates, but I'm hoping that's all.) |
stdlib/public/core/StringIndex.swift
Outdated
/// A textual representation of this instance. | ||
@_alwaysEmitIntoClient | ||
@backDeployed(before: SwiftStdlib 6.1) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 🤞
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.)
@swift-ci test macOS platform |
@swift-ci test |
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.
a02dd26
to
d816f88
Compare
d816f88
to
d91d8da
Compare
@swift-ci test |
Apply the LSG’s modifications as detailed in their review notes.
d91d8da
to
29cf262
Compare
@swift-ci test |
@swift-ci test |
@swift-ci test source compatibility |
This PR conforms
String.Index
toCustomStringConvertible
, 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:
With this PR, indices print by displaying their storage offset and encoding in a succinct format:
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