Skip to content

[region-isolation] Make store_borrow a store operation that does not require #74123

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

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Jun 4, 2024

store_borrow is an instruction intended to be used to initialize temporary alloc_stack with borrows. Since it is a temporary, we do not want to error on the temporaries initialization... instead, we want to error on the use of the temporary parameter.

This is achieved by making it so that store_borrow still performs an assign/merge, but does not require that src/dest be alive. So the regions still merge (yielding diagnostics for later uses).

It also required me to make it so that PartitionOp::{Assign,Merge} do not require by default. Instead, we want the individual operations to always emit a PartitionOp::Require explicitly (which they already did).

One thing to be aware of is that when it comes to diagnostics, we already know how to find a temporaries original value and how to handle that. So this is the last part of making store_borrow behave nicely.

rdar://129237675

@gottesmm gottesmm requested a review from ktoso as a code owner June 4, 2024 20:32
@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 4, 2024

@swift-ci smoke test

@gottesmm gottesmm enabled auto-merge June 4, 2024 20:32
…require.

TLDR:

The reason why I am doing this is it ensures that temporary store_borrow that we
create when materializing a value before were treated as uses. So we would error
on this:

```swift
@mainactor func transferToMain<T>(_ t: T) async {}

func test() async {
  let x = NonSendableKlass()
  await transferToMain(x)
  await transferToMain(x)
}
```

----

store_borrow is an instruction intended to be used to initialize temporary
alloc_stack with borrows. Since it is a temporary, we do not want to error on
the temporaries initialization... instead, we want to error on the use of the
temporary parameter.

This is achieved by making it so that store_borrow still performs an
assign/merge, but does not require that src/dest be alive. So the regions still
merge (yielding diagnostics for later uses).

It also required me to make it so that PartitionOp::{Assign,Merge} do not
require by default. Instead, we want the individual operations to always emit a
PartitionOp::Require explicitly (which they already did).

One thing to be aware of is that when it comes to diagnostics, we already know
how to find a temporaries original value and how to handle that. So this is the
last part of making store_borrow behave nicely.

rdar://129237675
@gottesmm gottesmm force-pushed the pr-9e8378fdeee3204a34f48ea8d2ff8f0be40a4674 branch from 60d87c7 to bd472b1 Compare June 12, 2024 22:05
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm gottesmm merged commit 6d15e41 into swiftlang:main Jun 13, 2024
3 checks passed
@gottesmm gottesmm deleted the pr-9e8378fdeee3204a34f48ea8d2ff8f0be40a4674 branch June 13, 2024 04:45
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.

1 participant