Skip to content
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

Add suggestion to diagnostic when user has array but trait wants slice. #91314

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,14 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
} else if !have_alt_message {
// Can't show anything else useful, try to find similar impls.
let impl_candidates = self.find_similar_impl_candidates(trait_ref);
self.report_similar_impl_candidates(impl_candidates, &mut err);
self.report_similar_impl_candidates(&impl_candidates, &mut err);

self.maybe_suggest_convert_to_slice(
&mut err,
trait_ref,
&impl_candidates,
span,
);
}

// Changing mutability doesn't make a difference to whether we have
Expand Down Expand Up @@ -1091,7 +1098,7 @@ trait InferCtxtPrivExt<'tcx> {

fn report_similar_impl_candidates(
&self,
impl_candidates: Vec<ty::TraitRef<'tcx>>,
impl_candidates: &[ty::TraitRef<'tcx>],
err: &mut DiagnosticBuilder<'_>,
);

Expand Down Expand Up @@ -1432,7 +1439,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {

fn report_similar_impl_candidates(
&self,
impl_candidates: Vec<ty::TraitRef<'tcx>>,
impl_candidates: &[ty::TraitRef<'tcx>],
err: &mut DiagnosticBuilder<'_>,
) {
if impl_candidates.is_empty() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,14 @@ pub trait InferCtxtExt<'tcx> {
trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
span: Span,
);

fn maybe_suggest_convert_to_slice(
&self,
err: &mut DiagnosticBuilder<'_>,
trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
candidate_impls: &[ty::TraitRef<'tcx>],
span: Span,
);
}

fn predicate_constraint(generics: &hir::Generics<'_>, pred: String) -> (Span, String) {
Expand Down Expand Up @@ -2499,6 +2507,72 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}
}
}

/// If the type that failed selection is an array or a reference to an array,
/// but the trait is implemented for slices, suggest that the user converts
/// the array into a slice.
fn maybe_suggest_convert_to_slice(
&self,
err: &mut DiagnosticBuilder<'_>,
trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
candidate_impls: &[ty::TraitRef<'tcx>],
span: Span,
) {
// Three cases where we can make a suggestion:
// 1. `[T; _]` (array of T)
// 2. `&[T; _]` (reference to array of T)
// 3. `&mut [T; _]` (mutable reference to array of T)
let (element_ty, mut mutability) = match *trait_ref.skip_binder().self_ty().kind() {
ty::Array(element_ty, _) => (element_ty, None),

ty::Ref(_, pointee_ty, mutability) => match *pointee_ty.kind() {
ty::Array(element_ty, _) => (element_ty, Some(mutability)),
_ => return,
},

_ => return,
};

// Go through all the candidate impls to see if any of them is for
// slices of `element_ty` with `mutability`.
let mut is_slice = |candidate: Ty<'tcx>| match *candidate.kind() {
ty::RawPtr(ty::TypeAndMut { ty: t, mutbl: m }) | ty::Ref(_, t, m) => {
if matches!(*t.kind(), ty::Slice(e) if e == element_ty)
&& m == mutability.unwrap_or(m)
{
// Use the candidate's mutability going forward.
mutability = Some(m);
true
} else {
false
}
}
_ => false,
};

// Grab the first candidate that matches, if any, and make a suggestion.
if let Some(slice_ty) = candidate_impls
.iter()
.map(|trait_ref| trait_ref.self_ty())
.filter(|t| is_slice(*t))
.next()
{
let msg = &format!("convert the array to a `{}` slice instead", slice_ty);

if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
let suggestion = if snippet.starts_with('&') {
format!("{}[..]", snippet)
} else if let Some(hir::Mutability::Mut) = mutability {
format!("&mut {}[..]", snippet)
} else {
format!("&{}[..]", snippet)
};
err.span_suggestion(span, msg, suggestion, Applicability::MaybeIncorrect);
} else {
err.span_help(span, msg);
}
}
}
}

/// Collect all the returned expressions within the input expression.
Expand Down
28 changes: 28 additions & 0 deletions src/test/ui/dst/issue-90528-unsizing-suggestion-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Issue #90528: provide helpful suggestions when a trait bound is unsatisfied
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you could probably collapse these tests down into fewer files - perhaps cases where a suggestion is made and where a suggestion isn't made - and then you can add // run-rustfix to the test to confirm the suggestion works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. What should I do about the scenarios where the suggestion is wrong (see anywhere I put bad suggestion)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could split these out into another test, as long as the suggestion is MaybeIncorrect then it's fine that it's sometimes wrong, it'll just need to be in a different test if we want // run-rustfix to work on the cases that do have correct suggestions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still new to rustfix. Will it apply MaybeIncorrect changes if the test is annotated with // run-rustfix? As written, my code does not know when its suggestion is MachineApplicable and when it is MaybeIncorrect, so I just always give MaybeIncorrect.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually can't remember, if it isn't doing it when you're trying this locally then we can just approve this - apologies for the runaround.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that in tests MaybeIncorrect ones do get applied. If they don't what we normally do is separate all the cases that can be fixed into its own file and another for the rest.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@estebank looks like you're right: https://rustc-dev-guide.rust-lang.org/tests/adding.html#other-header-commands

rustfix-only-machine-applicable is equivalent to run-rustfix except it will only apply MachineApplicable suggestions. run-rustfix will apply all suggestions.

// due to a missed unsizing coercion.
//
// This test exercises array literals and a trait implemented on immutable slices.

trait Read {}

impl Read for &[u8] {}

fn wants_read(_: impl Read) {}

fn main() {
wants_read([0u8]);
//~^ ERROR the trait bound `[u8; 1]: Read` is not satisfied
//~| HELP the following implementations were found
//~| HELP convert the array
//~| SUGGESTION &[0u8][..]
wants_read(&[0u8]);
//~^ ERROR the trait bound `&[u8; 1]: Read` is not satisfied
//~| HELP the following implementations were found
//~| HELP convert the array
//~| SUGGESTION &[0u8][..]
wants_read(&[0u8][..]);

wants_read(&mut [0u8]);
//~^ ERROR the trait bound `&mut [u8; 1]: Read` is not satisfied
//~| HELP the following implementations were found
}
55 changes: 55 additions & 0 deletions src/test/ui/dst/issue-90528-unsizing-suggestion-1.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
error[E0277]: the trait bound `[u8; 1]: Read` is not satisfied
--> $DIR/issue-90528-unsizing-suggestion-1.rs:13:16
|
LL | wants_read([0u8]);
| ---------- ^^^^^
| | |
| | the trait `Read` is not implemented for `[u8; 1]`
| | help: convert the array to a `&[u8]` slice instead: `&[0u8][..]`
| required by a bound introduced by this call
|
= help: the following implementations were found:
<&[u8] as Read>
note: required by a bound in `wants_read`
--> $DIR/issue-90528-unsizing-suggestion-1.rs:10:23
|
LL | fn wants_read(_: impl Read) {}
| ^^^^ required by this bound in `wants_read`

error[E0277]: the trait bound `&[u8; 1]: Read` is not satisfied
--> $DIR/issue-90528-unsizing-suggestion-1.rs:18:16
|
LL | wants_read(&[0u8]);
| ---------- ^^^^^^
| | |
| | the trait `Read` is not implemented for `&[u8; 1]`
| | help: convert the array to a `&[u8]` slice instead: `&[0u8][..]`
| required by a bound introduced by this call
|
= help: the following implementations were found:
<&[u8] as Read>
note: required by a bound in `wants_read`
--> $DIR/issue-90528-unsizing-suggestion-1.rs:10:23
|
LL | fn wants_read(_: impl Read) {}
| ^^^^ required by this bound in `wants_read`

error[E0277]: the trait bound `&mut [u8; 1]: Read` is not satisfied
--> $DIR/issue-90528-unsizing-suggestion-1.rs:25:16
|
LL | wants_read(&mut [0u8]);
| ---------- ^^^^^^^^^^ the trait `Read` is not implemented for `&mut [u8; 1]`
| |
| required by a bound introduced by this call
|
= help: the following implementations were found:
<&[u8] as Read>
note: required by a bound in `wants_read`
--> $DIR/issue-90528-unsizing-suggestion-1.rs:10:23
|
LL | fn wants_read(_: impl Read) {}
| ^^^^ required by this bound in `wants_read`

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0277`.
42 changes: 42 additions & 0 deletions src/test/ui/dst/issue-90528-unsizing-suggestion-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Issue #90528: provide helpful suggestions when a trait bound is unsatisfied
// due to a missed unsizing coercion.
//
// This test exercises array variables and a trait implemented on immmutable slices.

trait Read {}

impl Read for &[u8] {}

fn wants_read(_: impl Read) {}

fn main() {
let x = [0u8];
wants_read(x);
//~^ ERROR the trait bound `[u8; 1]: Read` is not satisfied
//~| HELP the following implementations were found
//~| HELP convert the array
//~| SUGGESTION &x[..]
wants_read(&x);
//~^ ERROR the trait bound `&[u8; 1]: Read` is not satisfied
//~| HELP the following implementations were found
//~| HELP convert the array
//~| SUGGESTION &x[..]
wants_read(&x[..]);

let x = &[0u8];
wants_read(x);
//~^ ERROR the trait bound `&[u8; 1]: Read` is not satisfied
//~| HELP the following implementations were found
//~| HELP convert the array
//~| SUGGESTION &x[..]
wants_read(&x);
//~^ ERROR the trait bound `&&[u8; 1]: Read` is not satisfied
//~| HELP the following implementations were found
wants_read(*x);
//~^ ERROR the trait bound `[u8; 1]: Read` is not satisfied
//~| HELP the following implementations were found
//~| HELP convert the array
//~| SUGGESTION &*x[..]
// ^^^^^^^ bad suggestion
wants_read(&x[..]);
}
91 changes: 91 additions & 0 deletions src/test/ui/dst/issue-90528-unsizing-suggestion-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
error[E0277]: the trait bound `[u8; 1]: Read` is not satisfied
--> $DIR/issue-90528-unsizing-suggestion-2.rs:14:16
|
LL | wants_read(x);
| ---------- ^
| | |
| | the trait `Read` is not implemented for `[u8; 1]`
| | help: convert the array to a `&[u8]` slice instead: `&x[..]`
| required by a bound introduced by this call
|
= help: the following implementations were found:
<&[u8] as Read>
note: required by a bound in `wants_read`
--> $DIR/issue-90528-unsizing-suggestion-2.rs:10:23
|
LL | fn wants_read(_: impl Read) {}
| ^^^^ required by this bound in `wants_read`

error[E0277]: the trait bound `&[u8; 1]: Read` is not satisfied
--> $DIR/issue-90528-unsizing-suggestion-2.rs:19:16
|
LL | wants_read(&x);
| ---------- ^^
| | |
| | the trait `Read` is not implemented for `&[u8; 1]`
| | help: convert the array to a `&[u8]` slice instead: `&x[..]`
| required by a bound introduced by this call
|
= help: the following implementations were found:
<&[u8] as Read>
note: required by a bound in `wants_read`
--> $DIR/issue-90528-unsizing-suggestion-2.rs:10:23
|
LL | fn wants_read(_: impl Read) {}
| ^^^^ required by this bound in `wants_read`

error[E0277]: the trait bound `&[u8; 1]: Read` is not satisfied
--> $DIR/issue-90528-unsizing-suggestion-2.rs:27:16
|
LL | wants_read(x);
| ---------- ^
| | |
| | the trait `Read` is not implemented for `&[u8; 1]`
| | help: convert the array to a `&[u8]` slice instead: `&x[..]`
| required by a bound introduced by this call
|
= help: the following implementations were found:
<&[u8] as Read>
note: required by a bound in `wants_read`
--> $DIR/issue-90528-unsizing-suggestion-2.rs:10:23
|
LL | fn wants_read(_: impl Read) {}
| ^^^^ required by this bound in `wants_read`

error[E0277]: the trait bound `&&[u8; 1]: Read` is not satisfied
--> $DIR/issue-90528-unsizing-suggestion-2.rs:32:16
|
LL | wants_read(&x);
| ---------- ^^ the trait `Read` is not implemented for `&&[u8; 1]`
| |
| required by a bound introduced by this call
|
= help: the following implementations were found:
<&[u8] as Read>
note: required by a bound in `wants_read`
--> $DIR/issue-90528-unsizing-suggestion-2.rs:10:23
|
LL | fn wants_read(_: impl Read) {}
| ^^^^ required by this bound in `wants_read`

error[E0277]: the trait bound `[u8; 1]: Read` is not satisfied
--> $DIR/issue-90528-unsizing-suggestion-2.rs:35:16
|
LL | wants_read(*x);
| ---------- ^^
| | |
| | the trait `Read` is not implemented for `[u8; 1]`
| | help: convert the array to a `&[u8]` slice instead: `&*x[..]`
| required by a bound introduced by this call
|
= help: the following implementations were found:
<&[u8] as Read>
note: required by a bound in `wants_read`
--> $DIR/issue-90528-unsizing-suggestion-2.rs:10:23
|
LL | fn wants_read(_: impl Read) {}
| ^^^^ required by this bound in `wants_read`

error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0277`.
33 changes: 33 additions & 0 deletions src/test/ui/dst/issue-90528-unsizing-suggestion-3.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Issue #90528: provide helpful suggestions when a trait bound is unsatisfied
// due to a missed unsizing coercion.
//
// This test exercises array literals and a trait implemented on mutable slices.

trait Write {}

impl Write for & mut [u8] {}

fn wants_write(_: impl Write) {}

fn main() {
wants_write([0u8]);
//~^ ERROR the trait bound `[u8; 1]: Write` is not satisfied
//~| HELP the following implementations were found
//~| HELP convert the array
//~| SUGGESTION &mut [0u8][..]
wants_write(&mut [0u8]);
//~^ ERROR the trait bound `&mut [u8; 1]: Write` is not satisfied
//~| HELP the following implementations were found
//~| HELP convert the array
//~| SUGGESTION &mut [0u8][..]
wants_write(&mut [0u8][..]);

wants_write(&[0u8]);
//~^ ERROR the trait bound `&[u8; 1]: Write` is not satisfied
//~| HELP the following implementations were found

wants_write(&[0u8][..]);
//~^ ERROR the trait bound `&[u8]: Write` is not satisfied
//~| HELP the following implementations were found
//~| HELP consider changing this borrow's mutability
}
Loading