[CS] Allow bidirectional inference for ExprPatterns #64387
Closed
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
hamishknight
force-pushed
the
platypus-party
branch
from
691278b to
23d8e6a
Compare
hamishknight
force-pushed
the
platypus-party
branch
from
23d8e6a to
cc0e7d7
Compare
hamishknight
force-pushed
the
platypus-party
branch
2 times, most recently
from
e7e3c5b to
4184ebd
Compare
hamishknight
force-pushed
the
platypus-party
branch
4 times, most recently
from
72a8a67 to
a8b28da
Compare
hamishknight
force-pushed
the
platypus-party
branch
2 times, most recently
from
0990549 to
3ecd55f
Compare
The TypedPattern and IsPattern constraints were incorrectly written, with conversions propagating out of the patterns, when really conversions ought to propagate into patterns. In any case, it seems like we really want equality here. Fix the constraints to use equality, and have the cast constraint operate on the external pattern type instead of the subpattern type.
Order them such that if they were changed to conversions, they would be sound. This shouldn't make a difference, but unfortunately it turns out pattern equality is not symmetric. As such, tweak the pattern equality logic to account for the reversed types. This allows us to remove a special case from function matching.
This shouldn't be necessary, we should be able to solve with type variables instead. This makes sure we don't end up with weird special cases that only occur when an external type is present.
Push the only null case that can occur up into the caller.
We should never CSGen a null Type for patterns.
Instead of walking the single ASTNode from the target, walk all AST nodes associated with the target to find the completion expr. This is needed to find the completion expr in a pattern for an initialization target.
Previously we would wait until CSApply, which would trigger their type-checking in `coercePatternToType`. This caused a number of bugs, and hampered solver-based completion, which does not run CSApply. Instead, form a conjunction of all the ExprPatterns present, which preserves some of the previous isolation behavior (though does not provide complete isolation). We can then modify `coercePatternToType` to accept a closure, which allows the solver to take over rewriting the ExprPatterns it has already solved. This then sets the stage for the complete removal of `coercePatternToType`, and doing all pattern type-checking in the solver.
This is wrong because there's nowhere to put any conversion that is introduced, meaning that we'll likely crash in SILGen. Change the constraint to equality, which matches what we do outside of the constraint system. rdar://107709341
There's still plenty of more work to do here for pattern diagnostics, including introducing a bunch of new locator elements, and handling things like argument list mismatches. This at least lets us fall back to a generic mismatch diagnostic.
Previously if the cast was unresolved, we would emit a warning and bail with `nullptr`. This is wrong, because the caller expects a `nullptr` return to mean we emitted an error. Change the diagnostic to an error to fix this. This may appear source breaking, but in reality previously we were failing to add the cast at all in this case, which lead to a crash in SILGen. We really do want to reject these cases as errors, as this will give us a better opportunity to fall back to type-checking as ExprPatterns, and better matches the constraint solver type-checking. Also while we're here, change the diagnostic for the case where we don't have an existential context type from the confusing "enum doesn't have member" diagnostic to the pattern mismatch diagnostic. rdar://107420031
Rather than using a conjunction, just form the constraints for the match expression right away.
hamishknight
force-pushed
the
platypus-party
branch
from
3ecd55f to
07ef390
Compare
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
|
@swift-ci please SourceKit stress test |
|
Moving this work into #64280 |
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.
Alternative version of #64280 that allows bidirectional inference for ExprPatterns.