[DNM] A couple of binding fixes #85141
Draft
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
Make it more obvious it's different to `Decl::visitAuxiliaryDecls`.
We already reject attempts to reference this for `lazy` properties. For `lazy` locals let's just not expose it to name lookup to begin with. This ensures we don't attempt to prematurely kick the interface type computation for the var, fixing a couple of crashers.
…rgument Make sure we set types for any nested VarDecls in UnresolvedPatternExprs to ensure we don't crash when attempting to solve the body.
…ext` We can still apply the solution even if we emit an error here. No test since it's already covered in the test suite once the next commit is applied.
We had some cases where we weren't propagating failures to apply the solution, and as such weren't invalidating the resulting AST. Fix up these cases.
Previously we would allow these in Sema and diagnose them in SILGen, but allowing them in Sema is unsound because it means the constraint system ends up kicking interface type requests for declarations that should be type-checked as part of the closure itself. Adjust the name lookup logic to look through parent closures when detecting invalid forward references. For now we don't fallback to an outer result on encountering a use-before-declaration to preserve the current behavior. I'm planning on changing that in the next commit though.
If we encounter a variable declared after its use within a closure, we can fallback to using an outer result if present. This matches the behavior outside of a closure and generally seems more consistent with the behavior we have if we also find an inner result.
And make sure we mark any PatternBindingDecl entries as having been checked to avoid re-checking. This fixes a crash where we could attempt to re-check a property wrapper to compute its backing type.
Rather than computing these each time we need to solve an element that has a reference to them, let's just set them up-front when we generate constraints for the corresponding variable.
These methods can be simplified a bunch since the returned decl is always the input decl and we can refactor the lambdas to just return the auxiliary variable and have the type computation in the caller.
Make sure we query the constraint system for a type if we have a local property wrapper in a closure to avoid kicking interface type computation outside the closure, and make sure we map into context if we need to.
This allows the query to be consistent both during type-checking and after.
Bindings in closures must be type-checked together with the surrounding closure, add an assertion to make sure we don't try this. Carve out an exception for code completion which may still kick lazy type-checking on failure.
hamishknight
force-pushed
the
in-a-bind
branch
from
f0d5da7 to
9585582
Compare
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
|
@swift-ci please SourceKit stress test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Going to split this into multiple PRs, but want to run CI on the whole thing first