Skip to content

[6.0][stdlib] Round out ~Copyable generalizations in stdlib primitives #73079

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

Conversation

lorentey
Copy link
Member

@lorentey lorentey commented Apr 17, 2024

  • Explanation: This is a followup to #71688/#72436 that resolves usability issues and obvious omissions that were uncovered while using the new primitives.
  • Issue: rdar://117753275&117753867
  • Original PR: [stdlib] Round out ~Copyable generalizations in stdlib primitives #73045
  • Risk: This is a high risk change. It changes core parts of the Standard Library, triggering brand new code paths in the compiler, and carrying nontrivial source and binary compatibility risks.
  • Testing: Regression tests using the in-tree test suite, at-desk manual code analysis, and source compatibility test runs to validate compatibility with open source projects.
  • Reviewer: @glessard

- `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
(cherry picked from commit cafc993)
We can’t do typed throws here yet. (They involve implicit stack allocations that interfere with the explicit stack allocation builtins.)

(cherry picked from commit 5679bf6)
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)
…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.

(cherry picked from commit c6bc196)
@lorentey
Copy link
Member Author

Note: The original PR is still in review. I'm submitting this draft early so that I can get a head start on any test failures specific to this branch.

@lorentey
Copy link
Member Author

@swift-ci test

@lorentey
Copy link
Member Author

@swift-ci test source compatibility

@lorentey
Copy link
Member Author

There is a single source compat defect in swift-distributed-actors, and it is the same (unrelated) issue we've seen on main.

https://ci.swift.org/job/swift-PR-source-compat-suite-debug-macos/1530/artifact/swift-source-compat-suite/FAIL_swift-distributed-actors_5.7_BuildSwiftPackage.log

Assertion failed: (Mapping.count(requirement) == 0 && "Witness already known"), function setWitness, file ProtocolConformance.cpp, line 687.

This issue isn't related to the changes in this PR. (It appears a fix may have recently landed, as more recent runs have turned green.)

@lorentey lorentey marked this pull request as ready for review April 18, 2024 00:49
@lorentey lorentey requested a review from a team as a code owner April 18, 2024 00:49
@lorentey lorentey merged commit 9083c69 into swiftlang:release/6.0 Apr 18, 2024
6 of 7 checks passed
@lorentey lorentey deleted the noncopyable-primitives-refinements-6.0 branch April 18, 2024 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants