Skip to content

[region-isolation] Add the ability to write SIL tests for actor isolation #70394

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

I did this by adding the ability to specify callee/caller isolation on apply, begin_apply, try_apply. The reason why this is needed is currently we grab the isolation from the applyexpr attached to a SILLocation. If we don't have an AST though that doesn't work of course. Instead, we can use these optional markers so we can at least write tests.

rdar://118521597

This will let me add functionality into the SILParser into other files. The main
file is already very large.
…g at the SIL level on apply, begin_apply, try_apply.

Some notes:

This is not emitted by SILGen. This is just intended to be used so I can write
SIL test cases for transfer non sendable. I did this by adding an
ActorIsolationCrossing field to all FullApplySites rather than adding it into
the type system on a callee. The reason that this makes sense from a modeling
perspective is that an actor isolation crossing is a caller concept since it is
describing a difference in between the caller's and callee's isolation. As a
bonus it makes this a less viral change.

For simplicity, I made it so that the isolation is represented as an optional
modifier on the instructions:

  apply [callee_isolation=XXXX] [caller_isolation=XXXX]

where XXXX is a printed representation of the actor isolation.

When neither callee or caller isolation is specified then the
ApplyIsolationCrossing is std::nullopt. If only one is specified, we make the
other one ActorIsolation::Unspecified.

This required me to move ActorIsolationCrossing from AST/Expr.h ->
AST/ActorIsolation.h to work around compilation issues... Arguably that is where
it should exist anyways so it made sense.

rdar://118521597
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm gottesmm force-pushed the pr-f328e7893b344f5252d43a4358baf49fb4589abe branch from 65bf927 to a43e5e6 Compare December 12, 2023 03:33
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test linux platform

@gottesmm gottesmm merged commit 486b7ce into swiftlang:main Dec 12, 2023
@gottesmm gottesmm deleted the pr-f328e7893b344f5252d43a4358baf49fb4589abe branch December 12, 2023 21:47
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