-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Adding helper to report_unused_parameter #108252
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ use crate::constrained_generic_params as cgp; | |
use min_specialization::check_min_specialization; | ||
|
||
use rustc_data_structures::fx::FxHashSet; | ||
use rustc_errors::struct_span_err; | ||
use rustc_errors::{struct_span_err, Applicability}; | ||
use rustc_hir::def::DefKind; | ||
use rustc_hir::def_id::LocalDefId; | ||
use rustc_middle::ty::query::Providers; | ||
|
@@ -181,6 +181,15 @@ fn report_unused_parameter(tcx: TyCtxt<'_>, span: Span, kind: &str, name: Symbol | |
name | ||
); | ||
err.span_label(span, format!("unconstrained {} parameter", kind)); | ||
err.span_suggestion( | ||
span, | ||
format!( | ||
"Either remove the type parameter '{}' or make use of it, for example ` for S<{}>`.", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: our messages start lowercase There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should not just suggest |
||
name, name | ||
), | ||
"", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it's a good idea to give a suggestion to remove it. Simply removing the parameter is usually not the solution to this error in my experience. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I based myself on the "desired output" on #107295 |
||
Applicability::MaybeIncorrect, | ||
); | ||
if kind == "const" { | ||
err.note( | ||
"expressions using a const parameter must map each value to a distinct output value", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,11 @@ LL | impl <const N: usize> Collatz<{Some(N)}> {} | |
| | ||
= note: expressions using a const parameter must map each value to a distinct output value | ||
= note: proving the result of expressions other than the parameter are unique is not supported | ||
help: Either remove the type parameter 'N' or make use of it, for example ` for S<N>`. | ||
| | ||
LL - impl <const N: usize> Collatz<{Some(N)}> {} | ||
LL + impl <> Collatz<{Some(N)}> {} | ||
| | ||
Comment on lines
+9
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in this case the suggestion is wrong, removing it is never the right thing if it is used, but in a way that isn't legal. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since we know it in this case (we are emitting the two notes above after all), you can re-use the condition for that and not emit the suggestion in that case |
||
|
||
error[E0207]: the const parameter `N` is not constrained by the impl trait, self type, or predicates | ||
--> $DIR/issue-68366.rs:17:6 | ||
|
@@ -15,6 +20,11 @@ LL | impl<const N: usize> Foo {} | |
| | ||
= note: expressions using a const parameter must map each value to a distinct output value | ||
= note: proving the result of expressions other than the parameter are unique is not supported | ||
help: Either remove the type parameter 'N' or make use of it, for example ` for S<N>`. | ||
| | ||
LL - impl<const N: usize> Foo {} | ||
LL + impl<> Foo {} | ||
| | ||
|
||
error: aborting due to 2 previous errors | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,11 @@ LL | impl <const N: usize> Collatz<{Some(N)}> {} | |
| | ||
= note: expressions using a const parameter must map each value to a distinct output value | ||
= note: proving the result of expressions other than the parameter are unique is not supported | ||
help: Either remove the type parameter 'N' or make use of it, for example ` for S<N>`. | ||
| | ||
LL - impl <const N: usize> Collatz<{Some(N)}> {} | ||
LL + impl <> Collatz<{Some(N)}> {} | ||
| | ||
|
||
error[E0207]: the const parameter `N` is not constrained by the impl trait, self type, or predicates | ||
--> $DIR/issue-68366.rs:17:6 | ||
|
@@ -24,6 +29,11 @@ LL | impl<const N: usize> Foo {} | |
| | ||
= note: expressions using a const parameter must map each value to a distinct output value | ||
= note: proving the result of expressions other than the parameter are unique is not supported | ||
help: Either remove the type parameter 'N' or make use of it, for example ` for S<N>`. | ||
| | ||
LL - impl<const N: usize> Foo {} | ||
LL + impl<> Foo {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should remove the generic parameters entirely instead of leaving empty ones around |
||
| | ||
|
||
error: aborting due to 3 previous errors | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,6 +59,12 @@ error[E0207]: the type parameter `T` is not constrained by the impl trait, self | |
| | ||
LL | impl<T,T> Qux<T,T> for Option<T> {} | ||
| ^ unconstrained type parameter | ||
| | ||
help: Either remove the type parameter 'T' or make use of it, for example ` for S<T>`. | ||
| | ||
LL - impl<T,T> Qux<T,T> for Option<T> {} | ||
LL + impl<T,> Qux<T,T> for Option<T> {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. stray comma in the suggestion |
||
| | ||
|
||
error: aborting due to 8 previous errors | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,12 @@ error[E0207]: the type parameter `T` is not constrained by the impl trait, self | |
| | ||
LL | impl<T: Default> Foo { | ||
| ^ unconstrained type parameter | ||
| | ||
help: Either remove the type parameter 'T' or make use of it, for example ` for S<T>`. | ||
| | ||
LL - impl<T: Default> Foo { | ||
LL + impl<: Default> Foo { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to remove the bounds, too |
||
| | ||
|
||
error: aborting due to previous error | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,12 @@ error[E0207]: the type parameter `I` is not constrained by the impl trait, self | |
| | ||
LL | impl<'q, Q, I, F> A for TestB<Q, F> | ||
| ^ unconstrained type parameter | ||
| | ||
help: Either remove the type parameter 'I' or make use of it, for example ` for S<I>`. | ||
| | ||
LL - impl<'q, Q, I, F> A for TestB<Q, F> | ||
LL + impl<'q, Q, , F> A for TestB<Q, F> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. stray comma |
||
| | ||
|
||
error: aborting due to previous error | ||
|
||
|
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.
"Make use of it" is a little misleading, the user might already use it in a where clause. I'm not sure what the best wording for this would be.
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.
maybe just removing the "make" and leave it Either remove the type parameter or use it?
I am not sure on the words 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.
yea that sounds better