Skip to content

[SIL] Hollow out Builtin.copy and deprecate _copy. #73422

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
merged 1 commit into from
May 4, 2024

Conversation

nate-chandler
Copy link
Contributor

@nate-chandler nate-chandler commented May 3, 2024

The copy operator has been implemented and doesn't use it. Remove Builtin.copy and _copy as much as currently possible.

Source compatibility requires that _copy remain in the stdlib. It is deprecated here and just uses the copy operator.

Handling old swiftinterfaces requires that Builtin.copy be defined. Redefine it here as a passthrough--SILGen machinery will produce the necessary copy_addr.

rdar://127502242

Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

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

Seems like we should be able to remove this.

@nate-chandler nate-chandler requested review from lorentey and tshortli May 3, 2024 19:09
@nate-chandler
Copy link
Contributor Author

nate-chandler commented May 3, 2024

@tshortli @lorentey Is it safe to remove the _copy function?

@tshortli
Copy link
Contributor

tshortli commented May 3, 2024

It looks like the main concern is source compatibility with the removal of the always-emit-into-client _copy decl; I don't see any other changes that would affect clients of the stdlib. Hopefully there aren't any projects we can't break that have adopted it, but I can't evaluate that with confidence.

@@ -265,3 +265,12 @@ extension String {
}
}
#endif

@available(swift, deprecated: 6, message: "Use the copy operator")
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to rather deprecate this in all language modes, not just in Swift 6+:

Suggested change
@available(swift, deprecated: 6, message: "Use the copy operator")
@available(*, deprecated, message: "Use the copy operator")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@nate-chandler
Copy link
Contributor Author

/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/lib/swift/Swift.swiftmodule/x86_64-apple-macos.swiftinterface:12443:5: error: module 'Builtin' has no member named 'copy'
12441 | @_alwaysEmitIntoClient @inlinable @_transparent @_semantics("lifetimemanagement.copy") public func _copy<T>(_ value: T) -> T {
12442 |   #if $BuiltinCopy
12443 |     Builtin.copy(value)
      |     `- error: module 'Builtin' has no member named 'copy'
12444 |   #else
12445 |     value

The copy operator has been implemented and doesn't use it.  Remove
`Builtin.copy` and `_copy` as much as currently possible.

Source compatibility requires that `_copy` remain in the stdlib.  It is
deprecated here and just uses the copy operator.

Handling old swiftinterfaces requires that `Builtin.copy` be defined.
Redefine it here as a passthrough--SILGen machinery will produce the
necessary copy_addr.

rdar://127502242
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please test source compatibility

@nate-chandler
Copy link
Contributor Author

@swift-ci please apple silicon benchmark

@nate-chandler nate-chandler changed the title [SIL] Remove Builtin.copy and _copy. [SIL] Hollow out Builtin.copy and deprecate _copy. May 3, 2024
@nate-chandler nate-chandler marked this pull request as ready for review May 4, 2024 05:19
@nate-chandler
Copy link
Contributor Author

Source compat failures match failures in baseline.

@nate-chandler nate-chandler merged commit 768b3a4 into swiftlang:main May 4, 2024
@nate-chandler nate-chandler deleted the rdar127502242 branch May 4, 2024 05:19
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.

4 participants