-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[SE-0494][StdLib] Add isTriviallyIdentical(to:) Methods to String and Substring
#82055
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
b140d00 to
7634c46
Compare
isIdentical Methods for Quick Comparisons to String and SubstringisIdentical Methods for Quick Comparisons to String and Substring
|
@lorentey No big rush… but if you had any advice about these implementations that would be a big help. I'm planning to post another pitch to swift forums on monday. It's just a pitch thread… so it's not blocked on implementation diffs. I would expect at least one more week before I would have some more implementations for the rest of the types to unblock a proposal review thread. |
7462596 to
e6db03f
Compare
isIdentical Methods for Quick Comparisons to String and SubstringisIdentical Methods for Quick Comparisons to String and Substring
09d49a8 to
56da8c0
Compare
3a0f013 to
2b63da1
Compare
d26fa93 to
753979f
Compare
753979f to
e0a2726
Compare
isIdentical Methods for Quick Comparisons to String and SubstringisTriviallyIdentical(to:) Methods for Quick Comparisons to String and Substring
e0a2726 to
e694f86
Compare
lorentey
left a comment
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.
(In hindsight, we should've proposed adding the same methods to [Sub]String.UnicodeScalarView as well -- the scalar views are supposed to be fully formed string types. Oh well, there is always tomorrow!)
@lorentey No… we did actually add |
| tags: [.validation, .String])) | ||
| } | ||
|
|
||
| if #available(macOS 9999, iOS 9999, tvOS 9999, watchOS 9999, visionOS 9999, *) { |
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.
@lorentey I think this is the right way to guard from benchmarks… and then we have to go in and change this after 6.3 goes live to prod?
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.
No, we have SwiftStdlib 6.3 that you should use instead.
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.
@Azoy I'm pretty sure I tried that before… but SwiftStdlib 6.3 was not available from benchmarks. Was there a recent change to support that?
|
@swift-ci Please smoke test |
26a64f5 to
8c9f084
Compare
|
@swift-ci Please test |
|
@swift-ci please Apple silicon benchmark |
|
@swift-ci Please benchmark |
|
@swift-ci Please Build Toolchain macOS Platform |
| @_alwaysEmitIntoClient | ||
| public static var v6_3_0: Self { Self(_value: 0x060300) } | ||
| @_alwaysEmitIntoClient | ||
| public static var v6_4_0: Self { Self(_value: 0x060400) } |
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.
[2025-12-09T04:52:54.512Z] FAIL: Swift(macosx-x86_64) :: abi/macOS/x86_64/stdlib-asserts.swift (10622 of 20420, 6 of 6 attempts)
[2025-12-09T04:52:54.512Z] ******************** TEST 'Swift(macosx-x86_64) :: abi/macOS/x86_64/stdlib-asserts.swift' FAILED ********************
[2025-12-09T04:52:54.512Z] Exit Code: 1
[2025-12-09T04:52:54.512Z]
[2025-12-09T04:52:54.512Z] Command Output (stdout):
[2025-12-09T04:52:54.512Z] --
[2025-12-09T04:52:54.512Z] --- /Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/swift/test/abi/macOS/x86_64/../../Inputs/macOS/x86_64/stdlib/baseline-asserts 2025-12-09 02:24:28
[2025-12-09T04:52:54.512Z] +++ /Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/abi/macOS/x86_64/Output/stdlib-asserts.swift.tmp/symbols 2025-12-09 04:52:54
[2025-12-09T04:52:54.512Z] @@ -3385,6 +3385,7 @@
[2025-12-09T04:52:54.512Z] _$sSo19_SwiftStdlibVersionas23CustomStringConvertiblesWP
[2025-12-09T04:52:54.512Z] _$sSo19_SwiftStdlibVersionasE11descriptionSSvg
[2025-12-09T04:52:54.512Z] _$sSo19_SwiftStdlibVersionasE11descriptionSSvpMV
[2025-12-09T04:52:54.512Z] +_$sSo19_SwiftStdlibVersionasE6v6_4_0ABvpZMV
[2025-12-09T04:52:54.512Z] _$sSo19_SwiftStdlibVersionasE7currentABvgZ
[2025-12-09T04:52:54.512Z] _$sSp10deallocate8capacityySi_tF
[2025-12-09T04:52:54.512Z] _$sSp10deallocateyyF
@lorentey I think this one led to a failure from the stdlib-asserts.swift… but I thought aeic did not trigger the ABI tests?
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.
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.
@lorentey I think this one led to a failure from the stdlib-asserts.swift… but I thought aeic did not trigger the ABI tests?
This is a property descriptor to support reflection; its export is somewhat questionable, but it's expected! It should be added to the expectation files (for both arm64 and Intel), just like the others.
The property itself is backdeployable; do not add an availability declaration to it.
|
@swift-ci Please test |
|
@swift-ci please Apple silicon benchmark |
|
@swift-ci Please benchmark |
|
@swift-ci Please Build Toolchain macOS Platform |
|
@lorentey This one LGTM. Can you please help us land these changes on main? Thanks! |
|
This pull appears to have broken a C++ Interop test on the trunk Android CI: I checked the @madsodgaard, any idea what's going on here? |
|
@finagolfin Ahh… crap. I did not think to run CI jobs on Android before landed. That's on me. Sorry about that! Hmm… can I repro this error building locally from a macOS machine? How would I repro this other than the CI job? |
No, not at all, Android is not officially supported yet, just some early snapshots, and there is no way to trigger the Android CI manually as of yet, still adding that.
You can't, so nobody is faulting you for this. We will have to look into it locally on a linux host and see what we find. If you have any idea why this might be happening, your advice would be useful. |
Hmm… trying to think… this diff introduces a new SwiftStdlib 6.2:macOS 26.0, iOS 26.0, watchOS 26.0, tvOS 26.0, visionOS 26.0
SwiftStdlib 6.3:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, visionOS 9999
+ SwiftStdlib 6.4:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, visionOS 9999And our build scripts then implicitly generate for us the If If we are blocked on a local repro one experiment might be to try and see if refactoring the BTW… am I able to download the build archive artifacts of the job that failed? I am not able to see that from the Jenkins link. |
Thanks, we will look into that.
You want to see the Android SDK that was being built or the test scripts/results? I don't think any of the CI jobs make available the latter, while the former is not fully built when the CI job fails, which it does if any of the compiler validation tests do not pass. |
I was thinking about looking at the actual output of |
|
Alright, looked into this, by applying your pull locally in an Android test run and examining the resulting generated stdlib String headers in that test, plus how other Swift stdlib APIs apply these annotations. It appears that your pull, which merely uses these availability annotations more, turned up a latent bug in how these C++ Interop tests are run for non-Darwin availability platforms, with this C++ Interop test that was working for more than a year now failing. The tests use the file Once you added these new String APIs, this single test that checks for the public String APIs exported to a C++ header exposed this availability bug, which we'll now have to fix. @lorentey and @Xazax-hun, let me know if I got that right, as I'm using these availability features for the first time, and how we should fix this. Until then, I'm |
…stdlib 6.4 annotations in #82055 (#86034) This should [get the Android CI green](https://ci.swift.org/job/oss-swift-package-swift-sdk-for-android/137/console) again, until we can figure out how we want to handle this matter.
Unfortunately, I am not too familiar with availability but everything you wrote sounds reasonable to me. Could you open a github ticket to address this issue? |
|
Done, filed in #86085. |
Background
SE-0494 has been accepted by LSG.
Changes
We already expose a public-but-underscored method on
Stringto return identity equality.1 We can add a new public-and-supported method:What happens to the existing underscored method?
For now… we don't need to have a strong opinion about the underscored method. The underscored method is
alwaysEmitIntoClientand already deploys back to earlier releases. Deleting the underscored method would then mean a breaking change for apps that already depend on the underscored method.We do not currently expose identity equality on
Substring. We can follow a similar pattern toString:Availability
This code will require 6.4. We do not currently have a 6.4 version definition ready. We add that here.
Test Plan
New tests were added for
StringandSubstring.Footnotes
https://github.com/swiftlang/swift/blob/swift-6.1.2-RELEASE/stdlib/public/core/String.swift#L397-L415 ↩