[6.0][region-isolation] Do not ignore non-trivial results that are Sendable to be more permissive in the face of lazy typechecker issues. #75542
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 test |
|
Original: #75541 |
…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 (cherry picked from commit 8604480)
gottesmm
force-pushed
the
release/6.0-rdar132347404
branch
from
2e0d795 to
4d2f337
Compare
|
@swift-ci test |
hborla
approved these changes
Jul 31, 2024
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.
Explanation: 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.
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.
Radars:
Original PRs:
Risk: Low. All I am doing is making it so that we actually "create" a valid value that can be looked up in the case where we initially were Sendable but now have become non-Sendable. We will still get weird semantics, but we will not get a crash.
Reviewer: