-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[AST] Bring 'mutating' and 'inout self' in sync. #10375
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
@swift-ci Please smoke test |
LGTM, thanks for adding the new verifier check too! |
- A mutating method or accessor always has 'inout self'. - A nonmutating method or accessor never has 'inout self'. - Only instance members can be mutating. - Addressors are still addressors even when on static members. Came up after reviewing another patch that confused the two as possibly distinct concepts.
@swift-ci Please test |
Build failed |
Build failed |
@@ -188,12 +188,16 @@ struct S: Fooable { | |||
// CHECK-NOT: destroy_addr [[SELF_ADDR]] | |||
|
|||
// Witness thunk for prop3 nonmutating materializeForSet | |||
// CHECK-LABEL: sil private [transparent] [thunk] @_T015guaranteed_self1SVAA7FooableA2aDP5prop3SifmTW : $@convention(witness_method) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @inout S) -> (Builtin.RawPointer, Optional<Builtin.RawPointer>) | |||
// CHECK-LABEL: sil private [transparent] [thunk] @_T015guaranteed_self1SVAA7FooableA2aDP5prop3SifmTW : $@convention(witness_method) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @in_guaranteed S) -> (Builtin.RawPointer, Optional<Builtin.RawPointer>) |
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.
@slavapestov Additional sanity check that this change is reasonable? It certainly seems desirable, but I don't want it to break something at the other end of the compiler.
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 think it makes sense.
inout self
.inout self
.Came up after reviewing another patch that confused the two as possibly distinct concepts.