Skip to content
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

[6.0][region-isolation] Upstreaming some fixes #73483

Merged
merged 17 commits into from
May 16, 2024

Conversation

gottesmm
Copy link
Member

@gottesmm gottesmm commented May 7, 2024

Explanation: This contains a series of fixes from main into release/6.0.

  • 99fee4e. This commit enables GlobalActorIsolatedTypesUsability as an upcoming feature for swift 6 and updates tests.
  • bf6905d. This commit allows for Sendable global actor isolated closures to use transferred non-Sendable parameters. Beyond being the correct modeling, enabling GlobalActorIsolatedTypesUsability caused this to show up a lot more, so we fixed it.
  • a415084. In this commit, I made it so that we properly compute the isolation of generic typed things that were constrained to actor or any actor. Before, we were bailing early and going down the wrong path.
  • 8b3d645. This added a SILVerifier check that proved that SILFunction's only ever have a single sil_isolated parameter and that said parameter conforms to AnyActor.
  • d7a1a01. This is part of the work enabling 8b3d645.
  • 8304ceb This is part of the work enabling 8b3d645.
  • edb2dae. This is part of the work enabling 8b3d645.
  • cee9d0f. This is part of the work enabling 8b3d645.
  • 1296393. This is part of the work enabling 8b3d645.
  • 541f702. This just updated some tests since we are now printing out the isolation of SIL functions if SILGen provides it.
  • 61f955c. I discovered while doing some other experiments that we were not handling init accessors correctly. The problem was that init accessors are isolated to self, but are not passed self. So I had to create a special way of representing that a function can be self isolated but we don't have self in the internal machinery of the infrastructure.
  • 41609ff. In this change, I made it so that we inferred isolation from isolated parameters correctly.
  • e241344. This was a gardening update in preparation for 41609ff.
  • de2963c. This was fixing a test that merge conflicted on 6.0 since I haven't yet cherry-picked a specific async let change.
  • d6e8a9e. This makes it so that we properly produce suppressing diagnostics for transferring.
  • f576462. This ensures that we treat actor isolated non-Sendable enums correctly.
  • 6a0a190. This ensures that we treat actor isolated non-Sendable structs correctly.

Radars:

  • rdar://118244451.
  • rdar://125200006.
  • rdar://128021548.
  • rdar://127844737
  • rdar://127295657
  • rdar://126780823
  • rdar://127006035.

Original PRs:

Risk: Low. Just affects region isolation.
Testing: Added tests to the test suite.
Reviewer: N/A

@gottesmm gottesmm requested a review from a team as a code owner May 7, 2024 17:51
@gottesmm
Copy link
Member Author

gottesmm commented May 7, 2024

@swift-ci test

2 similar comments
@gottesmm
Copy link
Member Author

gottesmm commented May 7, 2024

@swift-ci test

@gottesmm
Copy link
Member Author

gottesmm commented May 9, 2024

@swift-ci test

@gottesmm
Copy link
Member Author

@swift-ci test

@gottesmm gottesmm changed the title [6.0][region-isolation] Upstreaming two fixes [6.0][region-isolation] Upstreaming some fixes May 14, 2024
@gottesmm
Copy link
Member Author

@swift-ci test

@gottesmm
Copy link
Member Author

@swift-ci test

@gottesmm
Copy link
Member Author

Linux test failed b/c of java exception.

@gottesmm
Copy link
Member Author

@swift-ci test linux platform

@gottesmm gottesmm enabled auto-merge May 15, 2024 02:47
@gottesmm
Copy link
Member Author

Again some sort of weird Jenkins errors.

@gottesmm
Copy link
Member Author

@swift-ci test linux platform

@gottesmm
Copy link
Member Author

@swift-ci test macOS platform

@gottesmm
Copy link
Member Author

@swift-ci test linux platform

@gottesmm
Copy link
Member Author

@swift-ci test macOS platform

…lement_addr geps.

Just an initial commit. Going to add more tests.

rdar://127006035.
(cherry picked from commit 14f5623)
…ake_enum_data_addr/struct_element_addr fields.

(cherry picked from commit 077f62c)
… suppress transferring in swift interface files.

Importantly I added tests that validated this behavior.

rdar://126780823
(cherry picked from commit dc25857)
…e next commit.

Specifically, I added a few helper methods and improved the logging printing.
This all makes the next commit a more focused commit.

(cherry picked from commit 9bfb3b7)
…on-self isolated parameters as well as self parameters that are actor isolated.

As part of this I went through how we handled inference and rather than using a
grab-bag getActorIsolation that was confusing to use, I created split APIs for
specific use cases (actor instance, global actor, just an apply expr crossing)
that makes it clearer inside the SILIsolationInfo::get* APIs what we are
actually trying to model. I found a few issues as a result and fixed most of
them if they were small. I also fixed one bigger one around computed property
initializers in the next commit. There is a larger change I didn't fix around allowing function
ref/partial_apply with isolated self parameters have a delayed flow sensitive
actor isolation... this will be fixed in a subsequent commit.

This also fixes a bunch of cases where we were printing actor-isolated instead
of 'self' isolated.

rdar://127295657
(cherry picked from commit 50c2d67)
rdar://127844737
(cherry picked from commit 35a5a25)
(cherry picked from commit 9504dc9)
…use when printing SIL instructions.

(cherry picked from commit 085f3d7)
… a header.

This works around a layering violation where libSwiftBasic includes parts of the
AST even though it shouldn't.

(cherry picked from commit 5e6b247)
…g isolation of a sil_isolated parameter.

It is unnecessary and seems to be slightly out of sync sometimes around
closures.

(cherry picked from commit c1d0c8c)
…ctor types.

I also added some docs to SIL.rst about sil_isolated as well.

(cherry picked from commit f51a050)
The specific problem was that the AST was looking for Actor/AnyActor in
_Concurrency... but I named the module of the test borrowing (for some reason).
So the machinery was failing to think that my stubbed out protocols where the
true known protocols. By changing the module name to _Concurrency, everything
worked out.

(cherry picked from commit 4aea60c)
After following up with @slavapestov and @xedin, I was right to be suspicious of
my changes here.

Instead of attempting to hard code this, I do the right thing and I map the
relevant type into the function's generic context and then do the check. This
ensures that when isAnyActorType() performs the conformance check,
ModuleDecl::lookupConformance() has a generic signature to work with.

(cherry picked from commit 4412fff)
…use getAnyActor instead of getNominalOrBoundGenericNominal().

This ensures that we can properly compute isolation for generic types that
conform to AnyActor.

I found this by playing with test cases from the previous commit. We would not
find an actor type for the actor instance isolation and would fall back along an
incorrect path.

rdar://128021548
(cherry picked from commit fe2dc11)
…to use transferred non-Sendable parameters.

This is safe since:

1. We transfer in the non-Sendable parameter into the global actor isolation
region so we know that we will not use the non-Sendable paramter again except on
that actor.

2. Since the closure is global actor isolated, we know that despite the fact
that it is Sendable, it will only ever be executed serially on said global actor
implying that we do not need to worry about different executions of the Sendable
closure running concurrently with each other.

rdar://125200006
(cherry picked from commit c3309d6)
…t 6 feature.

rdar://118244451
(cherry picked from commit de85b79)
@gottesmm
Copy link
Member Author

Hit a merge conflict with the borrow switch enablement.

@gottesmm
Copy link
Member Author

@swift-ci test

@gottesmm gottesmm merged commit abfa475 into apple:release/6.0 May 16, 2024
5 checks passed
@gottesmm gottesmm deleted the release/6.0-more-fixes branch May 16, 2024 21: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.

None yet

2 participants