-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Orphanck: Reject uncovered opaque types #135910
base: master
Are you sure you want to change the base?
Conversation
// FIXME(fmease): This is a temporary HACK; rework it. In the next solver, | ||
// we normalize structurally meaning opaque types "remain behind" weak | ||
// aliases. I kind of want to expand_weak_alias_tys this but that's | ||
// pretty hacky, too. |
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.
The HACK mentioned in the PR description.
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.
Yeah 🤔 I don't really know what to do here. I feel like ideally we potentially don't need to recompute things here and report an error using only the original uncovered type as this folder is somewhat :/
I am personally fine with slightly regressing diagnostics here if necessary for that, but don't have a clear idea of how bad this would be
} | ||
} else { | ||
// Regarding *opaque types* specifically, we choose to treat them as non-local, |
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.
We probably want to bring back some of these nicely written points.
☔ The latest upstream changes (presumably #136135) made this pull request unmergeable. Please resolve the merge conflicts. |
if let Some(local_ty) = self.local_ty { | ||
diag.arg("local_ty", local_ty); | ||
} | ||
} |
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.
if you've changing this anyways, do you mind changing this back into non-translateable diagnostics? 🤔 it feels somewhat hard to review otherwise cc #132181
Fixes #130978.
TODO: Get rid of a HACK.
TODO: Add new tests.
Feel free to review already though!
r? lcnr