Skip to content

[sending] Change CheckedContinuation.resume and Async{Throwing,}Stream.yield to use transferring parameters. #73381

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 10 commits into from
Jun 2, 2024

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented May 1, 2024

[transferring] Change CheckedContinuation.resume and Async{Throwing,}Stream.yield to use transferring parameters.

Importantly, I used silgen_name to preserve the name since I have not yet
removed the mangling for transferring for named entities. This ensures that
this is an ABI neutral change until I make that other change.

rdar://120420024

@gottesmm gottesmm requested a review from ktoso as a code owner May 1, 2024 21:14
@gottesmm
Copy link
Contributor Author

gottesmm commented May 1, 2024

@swift-ci smoke test

1 similar comment
@gottesmm
Copy link
Contributor Author

gottesmm commented May 2, 2024

@swift-ci smoke test

@gottesmm
Copy link
Contributor Author

Found the type reconstruction issue. I need to do some more work here since we need to mark withCheckedContinuation as returning a transferring result.

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm
Copy link
Contributor Author

But before I do that, I would like to see what the bots think about this without that part of the change.

@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 1, 2024

Just doing a test. I still have to make sending parsed without the flag enabled, but the rest is here.

@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 1, 2024

@swift-ci smoke test

@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 2, 2024

@swift-ci smoke test

@gottesmm gottesmm changed the title [transferring] Change CheckedContinuation.resume and Async{Throwing,}Stream.yield to use transferring parameters. [sending] Change CheckedContinuation.resume and Async{Throwing,}Stream.yield to use transferring parameters. Jun 2, 2024
@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 2, 2024

Need to fix one thing for Adrian. Going to push and restart testing... but on macOS/Linux we got through the Swift tests

@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 2, 2024

@swift-ci smoke test

gottesmm added 2 commits June 1, 2024 23:25
… use the isolated parameter, not self.

This was ok in the small since most of the time we were processing a self
parameter as isolated... but that isn't always true...
gottesmm added 8 commits June 1, 2024 23:25
…actors as global actors rather than actor instance.

The specific example I ran into was in sendable_continuation.swift where we were
passing in the @mainactor instance as a sil_isolated parameter. We were thinking
it was an actor instance so we emitted the wrong message.
…pplies merged region as a value, just propagate its isolation.

The reason that I am doing this is it ensures that if we have a region isolation
merge failure due to a mismatch in between the actual args in the region and the
propagated callee isolation, we see it immediately when we translate the apply
into the pseudo-IR instead of later when we perform the actual diagnostic
emission. This makes it far easier to diagnose these issues since we get an
unknown pattern very early which can be asserted on via the option
-sil-region-isolation-assert-on-unknown-pattern.
…ce to infer disconnected if the alloc stack is used as a sending indirect result.

I also fixed an issue that I found where we were not substituting SILResultInfo
flags which was causing us to drop when substituting sil_sending. I added a
SILVerifier check to make sure that we do not break this again.
…ng into vars/storage.

We want to ensure that functions/methods themselves do not have sending mangled
into their names, but we do want sending mangled in non-top level positions. For
example: we do not want to mangle sending into a function like the following:

```swift
// We don't want to mangle this.
func test(_ x: sending NonSendableKlass) -> ()
```

But when  it comes  to actually  storing functions  into memory,  we do  want to
distinguish in  between function values  that use sending  vs those that  do not
since we do not want to allow for  them to alias. Thus we want to mangle sending
into things like the following:

```swift
// We want to distinguish in between Array<(sending T) -> ()> and
// Array((T) -> ()>
let a = Array<(sending T) -> ()>

// We want to distinguish in between a global contianing (sending T) -> () and a
// global containing (T) -> ().
var global: (sending T) -> ()
```

This commit achieves that by making changes to the ASTMangler in getDeclType
which causes getDeclType to set a flag that says that we have not yet recursed
through the system and thus should suppress the printing of sendable. Once we
get further into the system and recurse, that flag is by default set to true, so
we get the old sending parameter without having to update large amounts of code.

rdar://127383107
…aired with sending.

I am doing this b/c we are going to ban borrowing sending so that we can leave
open that space for further design. In the short term, we need the ability to
create +0 sending parameters without messing with mangling. By special casing
this, we get what we want.

rdar://129116141
…E instead of an UPCOMING_FEATURE.

TLDR: This makes it so that we always can parse sending/transferring but changes
the semantic language effects to be keyed on RegionBasedIsolation instead.

----

The key thing that makes this all work is that I changed all of the "special"
semantic changes originally triggered on *ArgsAndResults to now be triggered
based on RegionBasedIsolation being enabled. This makes a lot of sense since we
want these semantic changes specifically to be combined with the checkers that
RegionBasedIsolation turns on. As a result, even though this causes these two
features to always be enabled, we just parse it but we do not use it for
anything semantically.

rdar://128961672
… APIs to use sending parameters and results.

Specifically:

1. CheckedContinuation.resume now takes a sending parameter.
2. Async{Throwing,}Stream.yield takes a sending parameter.
3. withCheckedContinuation returns a transferring parameter.

Importantly due to the previous changes around mangling, this is a mangling
neutral change.

rdar://120420024
@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 2, 2024

@swift-ci smoke test

@gottesmm gottesmm enabled auto-merge June 2, 2024 07:45
@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 2, 2024

@swift-ci smoke test linux platform

@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 2, 2024

There might have been a mid-air collision with sourcekit-lsp

@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 2, 2024

@swift-ci smoke test linux platform

@gottesmm gottesmm merged commit b2808d1 into swiftlang:main Jun 2, 2024
3 checks passed
@gottesmm gottesmm deleted the rdar120420024 branch June 2, 2024 22:20
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.

2 participants