-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Add sending to subscript sending result #79560
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
LGTM but @gottesmm should take a look as well! |
@swift-ci please test |
Is there any additional action required from me? |
I don't think there is anything for you to do. I'll ping @gottesmm about this again. |
|
||
// CHECK: sil hidden [ossa] @$s17sending_subscript1SVyAA16NonSendableKlassCAEncig : $@convention(method) (@sil_sending @owned NonSendableKlass, S) -> @sil_sending @owned NonSendableKlass { | ||
struct S { | ||
subscript(_: sending NonSendableKlass) -> sending NonSendableKlass { NonSendableKlass() } |
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.
Can you add a test case with multiple results. IIRC there was some sort of code I put in that forced all result types to also need to have sending result as well. The idea is that with time, top level sending result means all results have sending results... but once we support sending on specific results, then we allow for functions not to set the top level sending result flag and vary that specific sending result flag on results. I at least want some more test cases with a tuple result. Let me see if I can find the code in question.
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.
Actually, now that I think about it... that property is actually something at the SIL level. Not something at the AST level where we just IIRC represent results with a type instead of using ResultInfo sort of things. Can you just out of an abundance of caution add a subscript with a tuple result test case.
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.
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.
@gottesmm
Is anything else needed?
@swift-ci please test |
The Windows test failed. Do we need special handling for Windows? |
@swift-ci please test Windows platform |
1 similar comment
@swift-ci please test Windows platform |
Resolves #79559
When the
sending
is applied to the return type in a subscript and SIL is emitted, thesending
disappears from the output.Here's an example to demonstrate this behavior:
The SIL output(the result of
emit-silgen
) shows :Note that while the parameter retains its
sending
in the SIL output, the return type'ssending
is missing.