Sema: Improve the infinite opaque return type check #83141
Merged
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
slavapestov
force-pushed
the
fix-rdar82992151
branch
2 times, most recently
from
5dbcd18 to
9a5811c
Compare
slavapestov
requested review from
artemcm,
tshortli and
DougGregor
as code owners
slavapestov
force-pushed
the
fix-rdar82992151
branch
from
9a5811c to
f42d9a6
Compare
release/6.2 chokes on this file. Nice work @xedin :)
…transformRec() The semantics of SubstFlags::SubstituteOpaqueArchetypes are changing so this is no longer useful here.
…ault for opaque types
…lying types Tracking seen declarations and substitution maps only detects the situation where the opaque type's underlying type contains itself with the same substitution map. However, it is also possible to recurse with a different substitution map. In general termination is undecidable with a problem like this, so instead of trying to catch cycles, just impose a termination limit. This converts a stack overflow into an assertion, which is still not ideal; we should really diagnose something instead. But this is a first step.
Now look through other opaque return types that appear in the underlying type. This catches various forms of recursion that otherwise would cause a SILGen or SILOptimizer crash. - Fixes rdar://82992151.
A substitution map contains conformances, and a conformance can contain a substitution map. This will always be a DAG, but not a tree, and to avoid exponential blowup in certain edge cases, let's cache the work to avoid visiting the same substitution map repeatedly, if multiple conformances refer to the same substitution map.
Now that we cache substituted substitution maps inside the InFlightSubstitution, we can re-enable this.
slavapestov
force-pushed
the
fix-rdar82992151
branch
from
f42d9a6 to
4eaa7e3
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please test source compatibility |
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.
The underlying type of an opaque return type may refer to other opaque return types, and their underlying types may refer to other opaque return types, and so on.
In a valid program, replacing each one with its underlying type must eventually terminate. We had a couple of places where we check for recursion; once in MiscDiagnostics when we try to catch this in Sema, and in another spot where would silently bail out without reporting anything, which could then cause a runtime crash in the user's program.
The old check did not catch all cases, and it replaced an earlier check that was too conservative. This termination problem is undecidable in general, so we can't actually "catch" the cycle by analyzing the problem instance.
Instead, replace both checks with a simpler one based on a counter. This diagnoses more cases that were not caught before in Sema. It is still possible to construct something where the recursion is only apparent after SIL optimization, in which case we now assert, instead of crashing with a stack overflow.
Eventually, I'd like to always diagnose something, even if we're in the SIL optimizer.
Fixes rdar://82992151.