[stdlib] Add withContiguousStorageIfAvailable to SubString.UTF8View #29094
Conversation
|
@swift-ci please smoke test |
Lukasa
force-pushed
the
cb-substring-fast-access
branch
from
1fa79f2 to
d22b2c2
Compare
|
@swift-ci please smoke test |
Lukasa
force-pushed
the
cb-substring-fast-access
branch
from
d22b2c2 to
df07967
Compare
|
@swift-ci please smoke test |
stdlib/public/core/Substring.swift
Outdated
| @@ -389,6 +389,13 @@ extension Substring.UTF8View: BidirectionalCollection { | |||
| return _slice.distance(from: start, to: end) | |||
| } | |||
|
|
|||
| @inlinable | |||
There was a problem hiding this comment.
this will need either availability or emitting into the client
There was a problem hiding this comment.
(that's why the test failed)
There was a problem hiding this comment.
Which should it have? It seems like it should be safe to emit into the client, but you've got more practice thinking about ABI compatibility than I do.
stdlib/public/core/Substring.swift
Outdated
| @@ -389,6 +389,13 @@ extension Substring.UTF8View: BidirectionalCollection { | |||
| return _slice.distance(from: start, to: end) | |||
| } | |||
|
|
|||
| @inlinable | |||
Due to an oversight it seems that we never added a withContigousStorageIfAvailable implementation to SubString.UTF8View, which meant that if you sliced a String you lost the ability to get fast access to the backing storage. There's no good reason for this functionality to be missing, so this patch adds it in by delegating to the Slice implementation. Resolves SR-11999.
Lukasa
force-pushed
the
cb-substring-fast-access
branch
from
df07967 to
68f0816
Compare
|
@swift-ci please smoke test |
|
Ok, looks like we're ready to go. I don't think I have write on this repo so I don't think I can ask the CI robot to merge, though I might be wrong about that. |
|
Looks like this broke a Swift in the OS bot: https://ci.swift.org/job/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/1055/ |
|
I'm sorry, I thought this had passed full testing, not just smoke testing. Personally, I'm opposed to treating smoke testing as sufficient for merging, unless there's real crisis / time pressure. I'll revert for now and re-open with a test run |
|
Hm, what’s this bot testing? Older binaries against newer tests and runtimes? |
|
@swift-ci please test |
|
Ah, sorry, I forgot to follow up with a full test when the smoke test passed. |
|
Oh, if this is a backwards deployment checker bot, then maybe even that would not have catched it |
|
The only case I can think of where this would fail is if the binary was built with a Swift before the new standard library, where it either inlined or called the old default implementation, which unconditionally returns nil. |
|
Right, but the test should be recompiled. What we need is a guarantee that AEIC will shadow the default implementation (from Sequence) present in an old stdlib binary. @slavapestov, do you know if this is guaranteed? |
|
@Lukasa can you re-open and retry? |
|
Re-added in #29146. |
Due to an oversight it seems that we never added a
withContigousStorageIfAvailable implementation to SubString.UTF8View,
which meant that if you sliced a String you lost the ability to get fast
access to the backing storage. There's no good reason for this
functionality to be missing, so this patch adds it in by delegating to
the Slice implementation.
Resolves SR-11999.