Skip to content

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

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