[region-isolation] Some cleanups in preparation for later work #72476
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci smoke test |
|
Just getting this in since the thing I actually am using this for is taking a little more tweezing to get done. |
|
Forgot to update the unit test. |
|
@swift-ci smoke test |
|
Looks like we include libswiftAST in libswiftBasic so the unit tests for libswiftBasic failed to compile. |
|
@swift-ci smoke test |
gottesmm
added a commit
to gottesmm/swift
that referenced
this pull request
Mar 21, 2024
…ests since libswiftBasic can include contents from libswiftAST. I hit this in swiftlang#72476. I put the declaration that hit this in a header, but as I thought about it... there was no real harm in just fixing the issue and preventing future breakage.
|
@swift-ci smoke test macOS platform |
… ptr type from a pointer type. I need to start tracking the dynamic IsolationRegionInfo for the transferring operand so I can ignore uses that are part of the same IsolationRegionInfo. IsolationRegionInfo doesn't fit into a pointer, so just to keep things the same, I am going to just allocate it. This is an initial staging commit that tests out the bump ptr allocating without expanding the type yet.
This enables one to use ActorIsolation in a FoldingSetNode. I also fixed the hash combine of the type to contain state.silParsed (which I think was an oversight).
Specifically, I made it so that when one calls dump(), we put in a newline afterwards so we get a nice easy to read msg in the debugger. I also defined dumpForDiagnostics() to make it easy to dump out in the debugger how the ActorIsolation will show up in the diagnostics.
…ls.h This is in preparation for beginning to track an IsolationRegionInfo in TransferringOperand. Just splitting this into a separate commit to make it easier to read.
Importantly, if we have an actor isolation, we always defer to it.
Just slicing down a larger diff to make it easier to review.
…a helper factory method on IsolationRegionInfo. Long term I would like to get region analysis and transfer non sendable out of the business of directly interpreting the AST... but if we have to do it now, I would rather us do it through a helper struct. At least the helper struct can be modified later to work with additional SIL concurrency support when it is added.
… entire region into a helper. I also did a little bit of renaming of variable names to make the code a little clearer.
That is really what IsolationRegionInfo is.
… from an instruction. I am making this specific API since I am going to make it so that SILIsolationInfo::get(SILInstruction *) can infer isolation info from self even from functions that are not apply isolation crossing points. For example, in the following, we need to understand that test is main actor isolated and we shouldn't emit an error. ```swift @mainactor func test(_ x: NonSendable) {} @OtherActor func doSomething() { let x = NonSendable() Task.init { @mainactor in print(x) } test(x) } ```
Just doing a little cleaning.
…tionArgument onto SILIsolationInfo::get(...).
…of non-isolation crossing apply.
…ftBasicTests since libswiftBasic can include contents from libswiftAST." This reverts commit aac3cd7.
…ation::dumpForDiagnostics out of line.
|
@swift-ci smoke test |
gottesmm
added a commit
to gottesmm/swift
that referenced
this pull request
Mar 26, 2024
…ests since libswiftBasic can include contents from libswiftAST. I hit this in swiftlang#72476. I put the declaration that hit this in a header, but as I thought about it... there was no real harm in just fixing the issue and preventing future breakage. (cherry picked from commit aac3cd7)
gottesmm
added a commit
to gottesmm/swift
that referenced
this pull request
Mar 26, 2024
…ests since libswiftBasic can include contents from libswiftAST. I hit this in swiftlang#72476. I put the declaration that hit this in a header, but as I thought about it... there was no real harm in just fixing the issue and preventing future breakage. (cherry picked from commit aac3cd7)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Just chopping down a larger diff with a bunch of refactoring/cleanups/NFC work.