Skip to content
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

[stdlib] Start adopting noncopyable generics in the stdlib #71688

Merged
merged 55 commits into from
Mar 19, 2024

Conversation

lorentey
Copy link
Member

@lorentey lorentey commented Feb 16, 2024

Relax the copyability requirement on the type parameters of several crucial stdlib generic types and functions:

  • struct MemoryLayout<T: ~Copyable>
  • struct UnsafePointer<Pointee: ~Copyable>
  • struct UnsafeMutablePointer<Pointee: ~Copyable>
  • struct UnsafeBufferPointer<Element: ~Copyable>
  • struct UnsafeMutableBufferPointer<Element: ~Copyable>
  • enum Optional<Wrapped: ~Copyable>: ~Copyable
  • enum Result<Success: ~Copyable, Failure>: ~Copyable
  • func swap<T: ~Copyable>
  • func withExtendedLifetime<T: ~Copyable, R: ~Copyable>
  • func withUnsafeTemporaryAllocation<T: ~Copyable, R> (generalizing R is a to do item for later)
  • class ManagedBuffer<Header, Element: ~Copyable> (generalizing Header is a to do for later)
  • struct ManagedBufferPointer<Header, Element: ~Copyable> (ditto)

Except for Optional and Result, making the parameter non-copyable will not affect the copyability of the type itself.

Notes, limitations:

  • The _Pointer protocol's Pointee associated type is no longer required to be copyable. This is a source-breaking change. _Pointer is not public API, so we're hoping it'll be okay.
  • The API-level ABI checker does not understand the weird semantics of our new marker protocols, so it is becoming completely useless at this point.
  • The plan as of today is to have ~Copyable clauses change the mangling of generic functions defined over specific types (as opposed to protocols). This PR uses the new @_preInverseGenerics attribute to avoid having to duplicate existing entry points.
  • By mistake, UnsafeMutablePointer.initialize(to:) was originally defined to take a __shared argument. This PR changes its new version to take a consuming parameter.
  • Ditto for the second parameter of UnsafeMutableBufferPointer.initialize(at:to:) and UnsafeMutableRawPointer.initialize(as:to:).
  • Similarly, Result.mapError(_:) and Result.get() are now redefined as consuming methods.
  • Tests are mostly missing; they are a work in progress.
  • I've made C++ interop build by going through UnsafeCxx*Iterator protocols and changing whatever needed to be changed to support non-copyable pointees, with no regards to compatibility.

@lorentey lorentey added the swift evolution proposal needed Flag → feature: A feature that warrants a Swift evolution proposal label Feb 16, 2024
@lorentey lorentey requested a review from Azoy February 16, 2024 20:32
@glessard
Copy link
Contributor

@swift-ci Please Build Toolchain macOS Platform

stdlib/public/Cxx/UnsafeCxxIterators.swift Show resolved Hide resolved
stdlib/public/core/MemoryLayout.swift Outdated Show resolved Hide resolved
stdlib/public/core/MemoryLayout.swift Outdated Show resolved Hide resolved
stdlib/public/core/Pointer.swift Outdated Show resolved Hide resolved
// This unavailable implementation uses the expected mangled name
// of `withMemoryRebound<T, Result>(to:capacity:_:)`, and provides
// an entry point for any binary linked against the stdlib binary
// for Swift 5.6 and older.
@available(*, unavailable)
@available(swift, obsoleted: 5.11)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be worth putting this in LegacyABI.swift instead to clear up this file a little? (You don't have to, just adding a suggestion)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but let's wait for the dust to settle a bit.

stdlib/public/core/UnsafePointer.swift Outdated Show resolved Hide resolved
@Azoy
Copy link
Contributor

Azoy commented Feb 20, 2024

@swift-ci please build toolchain macOS platform

@Azoy
Copy link
Contributor

Azoy commented Feb 20, 2024

@swift-ci please build toolchain macOS platform

1 similar comment
@Azoy
Copy link
Contributor

Azoy commented Feb 21, 2024

@swift-ci please build toolchain macOS platform

@airspeedswift
Copy link
Member

@swift-ci please build toolchain macOS platform

@airspeedswift
Copy link
Member

@swift-ci please build toolchain macOS platform

3 similar comments
@glessard
Copy link
Contributor

@swift-ci please build toolchain macOS platform

@airspeedswift
Copy link
Member

@swift-ci please build toolchain macOS platform

@airspeedswift
Copy link
Member

@swift-ci please build toolchain macOS platform

@glessard
Copy link
Contributor

@swift-ci please build toolchain macOS platform

@lorentey
Copy link
Member Author

@swift-ci please build toolchain macOS platform

@lorentey
Copy link
Member Author

error: expected ';' after expression

The shame!

@lorentey
Copy link
Member Author

@swift-ci test

@lorentey
Copy link
Member Author

@swift-ci please build toolchain macOS platform

@airspeedswift
Copy link
Member

@swift-ci please build toolchain macOS platform

@lorentey
Copy link
Member Author

@swift-ci smoke test

@lorentey
Copy link
Member Author

@swift-ci smoke test

@lorentey
Copy link
Member Author

@swift-ci test

@lorentey
Copy link
Member Author

@swift-ci benchmark

@lorentey
Copy link
Member Author

@swift-ci built toolchain macOS

@lorentey lorentey changed the title [stdlib] Start adopting noncopyable generics in the stdlib (phase 2) [stdlib] Start adopting noncopyable generics in the stdlib Mar 18, 2024
@lorentey
Copy link
Member Author

Interesting, locally I'm seeing an extra xpass that wasn't there at the last run:

XPASS: Swift(macosx-arm64) :: Interop/Cxx/class/move-only/move-only-cxx-value-type.swift (16 of 17553)

@lorentey
Copy link
Member Author

@swift-ci smoke test

@lorentey
Copy link
Member Author

@swift-ci test

@lorentey
Copy link
Member Author

@swift-ci benchmark

@lorentey
Copy link
Member Author

@swift-ci build toolchain macOS

@lorentey
Copy link
Member Author

lorentey commented Mar 18, 2024

Oh man, this got caught by the old duplicate allow-list entries thing again. Resolution is easy, it's just a shame these tests are Intel-only.

FAIL: Swift(macosx-x86_64) :: api-digester/stability-stdlib-abi-with-asserts.test (8824 of 17572)
******************** TEST 'Swift(macosx-x86_64) :: api-digester/stability-stdlib-abi-with-asserts.test' FAILED ********************
Script:
--
: 'RUN: at line 1';   rm -rf "/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp" && mkdir -p "/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp"
: 'RUN: at line 3';   /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/bin/swift-api-digester -target x86_64-apple-macosx10.13 -diagnose-sdk -module Swift -o /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/changes.txt -module-cache-path /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/module-cache -sdk /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/dummy.sdk -abi -avoid-location
: 'RUN: at line 4';   '/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/llvm-macosx-x86_64/bin/clang' -fmodules-cache-path='/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.13/clang-module-cache' -E -P -x c /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/swift/test/api-digester/stability-stdlib-abi-without-asserts.test -o - > /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/stability-stdlib-abi.swift.expected
: 'RUN: at line 5';   '/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/llvm-macosx-x86_64/bin/clang' -fmodules-cache-path='/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.13/clang-module-cache' -E -P -x c /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/swift/test/api-digester/stability-stdlib-abi-with-asserts.test -o - >> /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/stability-stdlib-abi.swift.expected
: 'RUN: at line 6';   '/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/llvm-macosx-x86_64/bin/clang' -fmodules-cache-path='/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.13/clang-module-cache' -E -P -x c /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/stability-stdlib-abi.swift.expected -o - | sed '/^\s*$/d' | sort > /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/stability-stdlib-abi.swift.expected.sorted
: 'RUN: at line 7';   '/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/llvm-macosx-x86_64/bin/clang' -fmodules-cache-path='/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.13/clang-module-cache' -E -P -x c /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/changes.txt -o - | sed -E -e '/^\s*$/d' -e 's/ in _[0-9A-F]{32}/ in #UNSTABLE ID#/g' | sort > /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/changes.txt.tmp
: 'RUN: at line 8';   diff -u /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/stability-stdlib-abi.swift.expected.sorted /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/changes.txt.tmp
--
Exit Code: 1

Command Output (stdout):
--
--- /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/stability-stdlib-abi.swift.expected.sorted	2024-03-18 21:35:34
+++ /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/changes.txt.tmp	2024-03-18 21:35:35
@@ -419,407 +419,213 @@
 Func withoutActuallyEscaping(_:do:) has been renamed to Func __abi_withoutActuallyEscaping(_:do:)
 Func withoutActuallyEscaping(_:do:) has mangled name changing from 'Swift.withoutActuallyEscaping<A, B>(_: A, do: (A) throws -> B) throws -> B' to 'Swift.__abi_withoutActuallyEscaping<A, B>(_: A, do: (A) throws -> B) throws -> B'
 Func withoutActuallyEscaping(_:do:) is now without @rethrows
-Protocol AdditiveArithmetic has added inherited protocol Copyable
 Protocol AdditiveArithmetic has added inherited protocol Copyable
-Protocol AdditiveArithmetic has added inherited protocol Escapable
 Protocol AdditiveArithmetic has added inherited protocol Escapable
 Protocol BidirectionalCollection has added inherited protocol Copyable
-Protocol BidirectionalCollection has added inherited protocol Copyable
 Protocol BidirectionalCollection has added inherited protocol Escapable
-Protocol BidirectionalCollection has added inherited protocol Escapable
 Protocol BinaryFloatingPoint has added inherited protocol Copyable
-Protocol BinaryFloatingPoint has added inherited protocol Copyable
 Protocol BinaryFloatingPoint has added inherited protocol Escapable
-Protocol BinaryFloatingPoint has added inherited protocol Escapable
 Protocol BinaryInteger has added inherited protocol Copyable
-Protocol BinaryInteger has added inherited protocol Copyable
 Protocol BinaryInteger has added inherited protocol Escapable
-Protocol BinaryInteger has added inherited protocol Escapable
 Protocol CVarArg has added inherited protocol Copyable
-Protocol CVarArg has added inherited protocol Copyable
 Protocol CVarArg has added inherited protocol Escapable
-Protocol CVarArg has added inherited protocol Escapable
 Protocol CaseIterable has added inherited protocol Copyable
-Protocol CaseIterable has added inherited protocol Copyable
 Protocol CaseIterable has added inherited protocol Escapable
-Protocol CaseIterable has added inherited protocol Escapable
 Protocol CodingKey has added inherited protocol Copyable
-Protocol CodingKey has added inherited protocol Copyable
 Protocol CodingKey has added inherited protocol Escapable
-Protocol CodingKey has added inherited protocol Escapable
 Protocol CodingKey has added inherited protocol Sendable
 Protocol CodingKey has generic signature change from <Self : Swift.CustomDebugStringConvertible, Self : Swift.CustomStringConvertible> to <Self : Swift.CustomDebugStringConvertible, Self : Swift.CustomStringConvertible, Self : Swift.Sendable>
 Protocol Collection has added inherited protocol Copyable
-Protocol Collection has added inherited protocol Copyable
 Protocol Collection has added inherited protocol Escapable
-Protocol Collection has added inherited protocol Escapable
 Protocol Collection has generic signature change from <Self : Swift.Sequence, Self.Element == Self.SubSequence.Element, Self.Index : Swift.Comparable, Self.Index == Self.Indices.Element, Self.Indices : Swift.Collection, Self.Indices == Self.Indices.SubSequence, Self.SubSequence : Swift.Collection, Self.SubSequence == Self.SubSequence.SubSequence, Self.Indices.Element == Self.Indices.Index, Self.Indices.Index == Self.SubSequence.Index, Self.SubSequence.Index == Self.Indices.Indices.Element, Self.Indices.Indices.Element == Self.Indices.Indices.Index, Self.Indices.Indices.Index == Self.SubSequence.Indices.Element, Self.SubSequence.Indices.Element == Self.SubSequence.Indices.Index, Self.SubSequence.Indices.Index == Self.SubSequence.Indices.Indices.Element, Self.SubSequence.Indices.Indices.Element == Self.SubSequence.Indices.Indices.Index> to <Self : Swift.Sequence, Self.Element == Self.SubSequence.Element, Self.Index : Swift.Comparable, Self.Index == Self.Indices.Element, Self.Indices : Swift.Collection, Self.Indices == Self.Indices.SubSequence, Self.SubSequence : Swift.Collection, Self.SubSequence == Self.SubSequence.SubSequence, Self.Indices.Element == Self.Indices.Index, Self.Indices.Index == Self.SubSequence.Index>
 Protocol Comparable has added inherited protocol Copyable
-Protocol Comparable has added inherited protocol Copyable
 Protocol Comparable has added inherited protocol Escapable
-Protocol Comparable has added inherited protocol Escapable
 Protocol CustomDebugStringConvertible has added inherited protocol Copyable
-Protocol CustomDebugStringConvertible has added inherited protocol Copyable
 Protocol CustomDebugStringConvertible has added inherited protocol Escapable
-Protocol CustomDebugStringConvertible has added inherited protocol Escapable
 Protocol CustomLeafReflectable has added inherited protocol Copyable
-Protocol CustomLeafReflectable has added inherited protocol Copyable
 Protocol CustomLeafReflectable has added inherited protocol Escapable
-Protocol CustomLeafReflectable has added inherited protocol Escapable
 Protocol CustomPlaygroundDisplayConvertible has added inherited protocol Copyable
-Protocol CustomPlaygroundDisplayConvertible has added inherited protocol Copyable
 Protocol CustomPlaygroundDisplayConvertible has added inherited protocol Escapable
-Protocol CustomPlaygroundDisplayConvertible has added inherited protocol Escapable
 Protocol CustomReflectable has added inherited protocol Copyable
-Protocol CustomReflectable has added inherited protocol Copyable
[... etc etc ad nauseam ...]
-Protocol __DefaultCustomPlaygroundQuickLookable has added inherited protocol Copyable
 Protocol __DefaultCustomPlaygroundQuickLookable has added inherited protocol Escapable
-Protocol __DefaultCustomPlaygroundQuickLookable has added inherited protocol Escapable
 Struct AnyHashable has added a conformance to an existing protocol _HasCustomAnyHashableRepresentation
 Struct ManagedBufferPointer has generic signature change from <Header, Element> to <Header, Element where Element : ~Copyable>
 Struct UnsafeBufferPointer has generic signature change from <Element> to <Element where Element : ~Copyable>

--
Command Output (stderr):
--
<unknown>:0: warning: no such sysroot directory: '/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/dummy.sdk'
<unknown>:0: warning: using sysroot for 'dummy' but targeting 'MacOSX'
clang: warning: argument unused during compilation: '-fmodules-cache-path=/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.13/clang-module-cache' [-Wunused-command-line-argument]
/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/swift/test/api-digester/stability-stdlib-abi-without-asserts.test:555:6: warning: trigraph ignored [-Wtrigraphs]
  555 | Func ??(_:_:) has been removed
      |      ^
1 warning generated.
clang: warning: argument unused during compilation: '-fmodules-cache-path=/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.13/clang-module-cache' [-Wunused-command-line-argument]
clang: warning: argument unused during compilation: '-fmodules-cache-path=/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.13/clang-module-cache' [-Wunused-command-line-argument]
/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/stability-stdlib-abi.swift.expected:458:6: warning: trigraph ignored [-Wtrigraphs]
  458 | Func ??(_:_:) has been removed
      |      ^
1 warning generated.
clang: warning: argument unused during compilation: '-fmodules-cache-path=/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.13/clang-module-cache' [-Wunused-command-line-argument]
/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/api-digester/Output/stability-stdlib-abi-with-asserts.test.tmp.tmp/changes.txt:314:6: warning: trigraph ignored [-Wtrigraphs]
  314 | Func ??(_:_:) has been removed
      |      ^
1 warning generated.

--

********************

@lorentey
Copy link
Member Author

Amusing preprocessor warning:

swift/test/api-digester/stability-stdlib-abi-without-asserts.test:555:6: warning: trigraph ignored [-Wtrigraphs]
  555 | Func ??(_:_:) has been removed
      |      ^
1 warning generated.

@lorentey
Copy link
Member Author

@swift-ci test

@lorentey
Copy link
Member Author

@swift-ci smoke test

@lorentey
Copy link
Member Author

@swift-ci benchmark

@lorentey
Copy link
Member Author

@swift-ci build toolchain macOS

@lorentey lorentey merged commit f24e18c into swiftlang:main Mar 19, 2024
7 checks passed
@lorentey lorentey deleted the noncopyable-primitives branch March 19, 2024 03:45
Comment on lines +54 to +55
// FIXME(NCG): Shouldn't this be also an error?
var moveOnly: MoveOnlyStruct
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we just diagnose one property and stop. I guess we could diagnose all of the noncopyable properties preventing conformance 🤷🏼

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.

7 participants