-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Disallow @_lifetime(borrow) for trivial 'inout' arguments #82189
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
Conversation
To avoid diff/merge confusion.
@_lifetime(borrow holder) // ERROR func test(holder: inout Holder) -> NE Fixes rdar://153040843 ([nonescapable] disallow @_lifetime(borrow) for trivial 'inout' arguments)
@swift-ci test |
@swift-ci test |
// An owned/consumed BitwiseCopyable value can be effectively borrowed | ||
// because its lifetime can be indefinitely extended. | ||
if (loweredOwnership == ValueOwnership::Owned | ||
&& isBitwiseCopyable(type, ctx)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the same rule apply for @_lifetime(&)
? In which case we will need a similar check when kind == ParsedLifetimeDependenceKind::Inout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
struct C {}
struct NE : ~Escapable {
@_lifetime(immortal)
init() {}
}
@_lifetime(&c)
func foo(_ c: consuming C) -> NE {
NE()
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be legal:
@_lifetime(&c)
func foo(_ c: consuming C) -> NE {
We have a special case for BitwiseCopyable because they are frequently used in initializers where the ownership defaults to consuming but the only legal way to express the dependency is
@_lifetime(borrow c)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, makes sense!
I left a minor comment on inout lifetime dependencies. These changes itself lgtm. |
[NFC] LifetimeDependence: fix indentation
To avoid diff/merge confusion.
Disallow @_lifetime(borrow) for trivial 'inout' arguments
@_lifetime(borrow holder) // ERROR
func test(holder: inout Holder) -> NE
Fixes rdar://153040843 ([nonescapable] disallow @_lifetime(borrow)
for trivial 'inout' arguments)