Skip to content

[region-isolation] Do not ignore non-trivial results that are Sendable to be more permissive in the face of lazy typechecker issues. #75541

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

We have found certain cases due to the requestified typechecker, a type is initially Sendable and then is later non-Sendable. This can be seen by the attached test case where the first time one calls isNonSendableType on the test value, one would get that it is Sendable and then the second time one would get it was non-Sendable. The result of this is that the pass gets into an inconsistent state.

This patch is a small patch that makes the pass more permissive in the face of such an error by making it so that we do not ignore Sendable results of instructions (that is we make sure to track a value for them), so we do not break invariants.

The longer term better fix is to make it so that we have a cache in the pass for this query that way we just always use the first answer returned from the typechecker and cache that. If the typechecker has such a bug, we may get bogus results, but we at least do not break invariants.

As an example of this type of behavior, in the test case in this patch, we first find the Sendable conformance of MySubClass and then the typechecker after doing some more type checking while performing that query, the second time finds the inherited non-Sendable conformance of MyParentClass causing MySubClass to be considered to be non-Sendable.

rdar://132347404

@gottesmm gottesmm requested review from ktoso and eeckstein as code owners July 29, 2024 15:46
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm gottesmm enabled auto-merge July 29, 2024 15:47
…e to be more permissive in the face of lazy typechecker issues.

We have found certain cases due to the requestified typechecker, a type is
initially Sendable and then is later non-Sendable. This can be seen by the
attached test case where the first time one calls isNonSendableType on the test
value, one would get that it is Sendable and then the second time one would get
it was non-Sendable. The result of this is that the pass gets into an
inconsistent state.

This patch is a small patch that makes the pass more permissive in the face of
such an error by making it so that we do not ignore Sendable results of
instructions (that is we make sure to track a value for them), so we do not
break invariants.

The longer term better fix is to make it so that we have a cache in the pass for
this query that way we just always use the first answer returned from the
typechecker and cache that. If the typechecker has such a bug, we may get bogus
results, but we at least do not break invariants.

As an example of this type of behavior, in the test case in this patch, we first
find the Sendable conformance of MySubClass and then the typechecker after doing
some more type checking while performing that query, the second time finds the
inherited non-Sendable conformance of MyParentClass causing MySubClass to be
considered to be non-Sendable.

rdar://132347404
@gottesmm gottesmm force-pushed the pr-7133549c90f98b7c9be71f80b74a97d088aa62f2 branch from 59fae69 to 8604480 Compare July 29, 2024 16:48
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm gottesmm merged commit 72ad780 into swiftlang:main Jul 29, 2024
3 checks passed
@gottesmm gottesmm deleted the pr-7133549c90f98b7c9be71f80b74a97d088aa62f2 branch July 29, 2024 20:17
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