Skip to content

[6.0][region-isolation] Enable region isolation when strict concurrency is enabled #72867

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
Apr 5, 2024

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Apr 5, 2024

Explanation: A bunch of work that culminates in enabling region isolation by default when strict concurrency is enabled.
Issue: This is fixing a bunch of different issues. Radars:

  • rdar://125918028
  • rdar://122030520
  • rdar://125452372
  • rdar://121954871
  • rdar://121955895
  • rdar://122692698

Original PRs:

Risk: Low. Only affects region isolation.
Testing: Added many functional and unit tests.
Reviewer: N/A

gottesmm added 10 commits April 4, 2024 16:15
…o is enabled.

I noticed this while I was debugging a failure in unittest.

(cherry picked from commit a114a30)
ActorIsolation already has a "I have no value case": unspecified. Lets just use
that.

Just a mistake I made that I am trying to fix before anything further depends on
this code.

(cherry picked from commit 77dccac)
I fixed a bunch of small issues around here that resulted in a bunch of radars
being fixed. Specifically:

1. I made it so that we treat function_refs that are from an actor isolated
function as actor isolated instead of sendable.

2. I made it so that autoclosures which return global actor isolated functions
are treated as producing a global actor isolated function.

3. I made it so that we properly handle SILGen code patterns produced by
Sendable GlobalActor isolated things.

rdar://125452372
rdar://121954871
rdar://121955895
rdar://122692698
(cherry picked from commit 1cf4e99)
…emporaries.

Just eliminating more cases of the typed errors in favor of named errors.

(cherry picked from commit ce27305)
…erroring on a duplicate symbol definition.

This diagnostic is useful around silgen_name where it validates that we do not
have any weird collisions. Sadly, it just points where one of the conflicting
elements is... with this patch, we also emit an error on the other function if
we have a SILLocation.

(cherry picked from commit 19c72ac)
I had to change the APIs to always be always emit into client instead of back
deployable since silgen_name seems to interfere with @backDeployment. So I
switched the implementation so that it instead uses an always emit into client
thunk with the in source function name and a usableFromInline function that has
the silgen_name. This ensures that we still appropriately export the same symbol
as we did before, so it is ABI stable.

This was approved as part of se-0414.

rdar://122030520
(cherry picked from commit cf93476)
This occurs when working with ActorIsolation in SIL.

This lets us avoid needing to depend on the AST for getting ActorIsolation for
self parameters. Now, we can just create the actor isolation we need based off
of the decl that we have.

The code is based off of forActorInstanceSelf(ValueDecl *decl) along the path
where it just creates isolation based off of the decl's nominal type decl (which
is equivalent to what we are trying to do here).

(cherry picked from commit 04e8bad)
…olation instead of that or nominal type decls.

This should be NFC since the only case where I used this was with self... and I
found another way of doing that using the API I added in the previous commit.

(cherry picked from commit c2350df)
…currency.

I added a disable flag -disable-region-based-isolation-with-strict-concurrency
so that we do not need to update the current tests. It is only available when
asserts are enabled to ensure users cannot use it.

rdar://125918028
(cherry picked from commit b3e837c)
@gottesmm gottesmm requested a review from a team as a code owner April 5, 2024 05:57
@gottesmm
Copy link
Contributor Author

gottesmm commented Apr 5, 2024

@swift-ci test

@gottesmm
Copy link
Contributor Author

gottesmm commented Apr 5, 2024

@swift-ci test macOS platform

1 similar comment
@gottesmm
Copy link
Contributor Author

gottesmm commented Apr 5, 2024

@swift-ci test macOS platform

@gottesmm gottesmm enabled auto-merge April 5, 2024 20:01
@gottesmm gottesmm merged commit f67b21a into swiftlang:release/6.0 Apr 5, 2024
@gottesmm gottesmm deleted the release/6.0-region-iso branch April 5, 2024 20:01
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