-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Emit warning when bound is trivially false #100090
Emit warning when bound is trivially false #100090
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
r? types |
Let's specifically r? @nikomatsakis, since he has some specific thoughts on this :) (Though I personally do think a warning is good) |
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.
Warnings here...aren't great.
@@ -0,0 +1,14 @@ | |||
warning: the where-clause bound `<T as Broken>::Assoc: Foo` is impossible to satisfy |
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.
It's a bit unfortunate that the bound we say is not the bound that's written.
@@ -0,0 +1,26 @@ | |||
warning: the where-clause bound `[for<'r> fn(&'r ())]: Copy` is impossible to satisfy |
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.
This is certainly not great.
☔ The latest upstream changes (presumably #100676) made this pull request unmergeable. Please resolve the merge conflicts. |
As per RFC 2056, they should not be rejected. The HRTB's this targets can currently be used to achieve trivial bounds without Warning makes sense though. |
I agree with warning, but I feel strongly that we should not be rejecting where-clauses that are known to be false. As @QuineDot noted, RFC 2056 decided against it, and I think that is well motivated. |
We really should be rejecting bounds that are global, even if they have higher-ranked lifetimes. We have the ability to prove whether
for<'a> &'a mut String: Clone
is true or not, so there's no reason to silently allow this to typecheck, especially because it's inconsistent with a non-HRTB like&'static mut String: Clone
and is the source of ICEs.For now, though, just add a warning, so that when an ICE does occur (because sometimes it will), then the user knows what might've caused it.
This is a revival of #95611.