-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Constraint system] Eliminate global flag UnderlyingTypeForOpaqueReturnType #29487
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Constraint system] Eliminate global flag UnderlyingTypeForOpaqueReturnType #29487
Conversation
SolutionResult better captures what can happen when solving a constraint system for the given expression occurs than the ad hoc SolutionKind return & small vector of Solution results. Also, try to make this logic less convoluted.
…ForOpaqueReturnType.
…rnType Make this information contextual (per type) rather than global (per constraint system) so we don’t misapply it.
@swift-ci please smoke test |
Only the last two commits are unmerged. Huh. |
solutions.assign(std::make_move_iterator(ambiguousSolutions.begin()), | ||
std::make_move_iterator(ambiguousSolutions.end())); | ||
dumpSolutions(); | ||
solution.markAsDiagnosed(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the same 'use-after-move' problem @CodaFi mentioned is present here as well...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. It's the same one. I'll roll this into an upcoming PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, it does make more sense to have opaque result bit as a contextual information instead of a flag.
Make this information contextual (per type) rather than global (per
constraint system) so we don’t misapply it.