[stdlib] Round out ~Copyable generalizations in stdlib primitives #73045
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lorentey
requested review from
ahoppen,
bnbarham,
hamishknight,
rintaro,
hborla,
xedin and
a team
as code owners
|
There'll be some api-digester failures (at least); I'll fix those later. |
- `Optional.map`, `.flatMap`: Allow noncopyable results. Implement typed throws. - `Result.map`, `.flatMap`: Allow noncopyable types for the new success. - `Result.flatMapError`: Generalize for noncopyable Success. - `Unsafe[Mutable][Buffer]Pointer.withMemoryRebound`: Alllow typed throws. - `Unsafe[Mutable]BufferPointer: Generalize CustomDebugStringConvertible conformance for noncopyable `Element`s. - `OpaquePointer.init(_: UnsafeMutablePointer)`, `.init(_: UnsafeMutablePointer?)`: Allow noncopyable pointee types. - `ManagedBuffer.withUnsafeMutablePointerToHeader`, `.withUnsafeMutablePointerToElements`, `.withUnsafeMutablePointers`: Generalize for typed throws and noncopyable return types. Avoid `@_preInverseGenerics`. - `ManagedBufferPointer`: Ditto. - `withExtendedLifetime`: Use typed throws. Stop using `@_preInverseGenerics`. rdar://117753275
We can’t do typed throws here yet. (They involve implicit stack allocations that interfere with the explicit stack allocation builtins.)
lorentey
force-pushed
the
noncopyable-primitives-refinements
branch
from
2a1417a to
fab25dc
Compare
|
@swift-ci test |
|
@swift-ci test source compatibility |
|
Source compat failures: GRDB: The obvious workaround is to revert to untyped rethrows for AsyncNinja: This is the same issue as triggered the original revert in #72477. |
swiftlang#73045 has uncovered a compiler crash with existing code of this form: try withExtendedLifetime(statement) { // 💥silgen crash do { try dbPool.close() XCTFail("Expected Error") } catch DatabaseError.SQLITE_BUSY { } } This patterns can happen with any of the newly typed throwing entry points, but the source compat suite only seems to have caught this with `withExtendedLifetime`; let’s see if we can get away with only rolling back this one.
…ound a shadowing issue The new ~Copyable generalizations have changed the function signature enough that alternative definitions of `map`/`flatMap` in existing code that used to be considered to shadow the originals no longer do so. This leads to use sites becoming ambiguous — a source break. While we consider approaches to resolve this on the compiler side, let’s try slapping a `@_disfavoredOverload` on these and see if that helps.
|
@swift-ci test |
|
@swift-ci test source compatibility |
|
@swift-ci build toolchain macOS platform |
|
Hmm, this source compat issue in swift-distributed-actors is a preexisting regression on main. (It actually showed up in the earlier run, too.) It is unrelated to this PR; I'm ignoring it. |
|
macOS failure is in the stability-stdlib-source check. I'm very tempted to delete that test entirely, as it keeps getting in the way and it has so limited understanding of recent Swift changes that it no longer provides much value. But for now, I'll add another patch to its exception list. |
|
@swift-ci test |
lorentey
added a commit
to lorentey/swift
that referenced
this pull request
Apr 17, 2024
swiftlang#73045 has uncovered a compiler crash with existing code of this form: try withExtendedLifetime(statement) { // 💥silgen crash do { try dbPool.close() XCTFail("Expected Error") } catch DatabaseError.SQLITE_BUSY { } } This patterns can happen with any of the newly typed throwing entry points, but the source compat suite only seems to have caught this with `withExtendedLifetime`; let’s see if we can get away with only rolling back this one. (cherry picked from commit 153c001)
|
@swift-ci please test Windows platform |
glessard
approved these changes
Apr 18, 2024
|
Joe's fix for the typed throw issue is up at #73092. I'll submit a separate PR to re-generalize |
This was referenced Apr 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a followup to #71688, completing the generalization of pointer types,
Optional,ResultandManagedBuffer, as well as the temporary allocation facility.Optional.map,.flatMap: Allow noncopyable results. Implement typed throws.Result.map,.flatMap: Allow noncopyable types for the new success. (This reapplies the changes that got temporarily reverted in [stdlib] Undo the generalization ofResult.map&.flatMapfor now #72477.)Result.flatMapError: Generalize for noncopyable Success.Unsafe[Mutable]BufferPointer: Generalize CustomDebugStringConvertible conformance for noncopyableElement`s.Unsafe[Mutable][Buffer]Pointer.withMemoryRebound: Convert to typed throws.OpaquePointer.init(_: UnsafeMutablePointer),.init(_: UnsafeMutablePointer?): Allow noncopyable pointee types.ManagedBuffer.withUnsafeMutablePointerToHeader,.withUnsafeMutablePointerToElements,.withUnsafeMutablePointers: Generalize for noncopyable return types and typed throw . Avoid@_preInverseGenerics.ManagedBufferPointer: Ditto.withUnsafeTemporaryAllocation: Generalize for noncopyable result types. (Not typed throws though -- they involve stack allocations that don't mix with explicitwithExtendedLifetime:Use typed throws. Stop using@_preInverseGenerics.rdar://117753275