[NFC] Refactor Subscript Expr Rebuilding a Bit #28809
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
Have callers resolve the SelectedOverload instead of the callee. Then make the error paths more apparent, and flush Optional<SelectedOverload> wherever it can be found.
|
@swift-ci please smoke test |
CodaFi
commented
Dec 16, 2019
| // have a proper overload selected from subscript call, might be because | ||
| // solver was allowed to return free or unresolved types, which can | ||
| // happen while running diagnostics on one of the expressions. | ||
| if (!overload) { |
There was a problem hiding this comment.
@xedin A quick grep reveals we have no tests that emit these diagnostics. Should this just use getOverloadChoice?
There was a problem hiding this comment.
Looks like there are some in d111e9b but they are crash tests so diagnostic is not verified. I have ran a couple with -debug-constraints and it looks like new framework is handling them well now, so I'd say let's remove this logic from here and maybe assert instead?
There was a problem hiding this comment.
Confirmed, this passes the compiler crasher suite.
|
@swift-ci please smoke test |
CodaFi
force-pushed
the
constant-contact
branch
from
f6daec7 to
7dd429b
Compare
|
@swift-ci please smoke test |
1 similar comment
|
@swift-ci please smoke test |
|
⛵️ |
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.
Const-qualify the members of
SelectedOverloadand change up the interface to the subscript-y bits of CSApply so it's a bit clearer what all they're doing with it. This pushes the failure path up and out to its callers, where it's now apparent that onlyvisitSubscriptExprever needed to use it. As it's CSApply's responsibility to createDynamicSubscriptExprs - we'd be in quite a lot of trouble if we somehow were able to build one then lost the overload that got us there.