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

Use impl's generics when suggesting fix on bad impl Copy #99741

Merged
merged 1 commit into from
Jul 31, 2022
Merged
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
26 changes: 8 additions & 18 deletions compiler/rustc_typeck/src/coherence/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,6 @@ fn visit_implementation_of_copy(tcx: TyCtxt<'_>, impl_did: LocalDefId) {

// We'll try to suggest constraining type parameters to fulfill the requirements of
// their `Copy` implementation.
let mut generics = None;
if let ty::Adt(def, _substs) = self_type.kind() {
let self_def_id = def.did();
if let Some(local) = self_def_id.as_local() {
let self_item = tcx.hir().expect_item(local);
generics = self_item.kind.generics();
}
}
let mut errors: BTreeMap<_, Vec<_>> = Default::default();
let mut bounds = vec![];

Expand Down Expand Up @@ -163,16 +155,14 @@ fn visit_implementation_of_copy(tcx: TyCtxt<'_>, impl_did: LocalDefId) {
&format!("the `Copy` impl for `{}` requires that `{}`", ty, error_predicate),
);
}
if let Some(generics) = generics {
suggest_constraining_type_params(
tcx,
generics,
&mut err,
bounds.iter().map(|(param, constraint, def_id)| {
(param.as_str(), constraint.as_str(), *def_id)
}),
);
}
suggest_constraining_type_params(
tcx,
tcx.hir().get_generics(impl_did).expect("impls always have generics"),
&mut err,
bounds.iter().map(|(param, constraint, def_id)| {
(param.as_str(), constraint.as_str(), *def_id)
}),
);
err.emit();
}
Err(CopyImplementationError::NotAnAdt) => {
Expand Down
19 changes: 19 additions & 0 deletions src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// run-rustfix

#[derive(Clone)]
struct Wrapper<T>(T);

struct OnlyCopyIfDisplay<T>(std::marker::PhantomData<T>);

impl<T: std::fmt::Display> Clone for OnlyCopyIfDisplay<T> {
fn clone(&self) -> Self {
OnlyCopyIfDisplay(std::marker::PhantomData)
}
}

impl<T: std::fmt::Display> Copy for OnlyCopyIfDisplay<T> {}

impl<S: std::fmt::Display> Copy for Wrapper<OnlyCopyIfDisplay<S>> {}
//~^ ERROR the trait `Copy` may not be implemented for this type

fn main() {}
19 changes: 19 additions & 0 deletions src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// run-rustfix

#[derive(Clone)]
struct Wrapper<T>(T);

struct OnlyCopyIfDisplay<T>(std::marker::PhantomData<T>);

impl<T: std::fmt::Display> Clone for OnlyCopyIfDisplay<T> {
fn clone(&self) -> Self {
OnlyCopyIfDisplay(std::marker::PhantomData)
}
}

impl<T: std::fmt::Display> Copy for OnlyCopyIfDisplay<T> {}

impl<S> Copy for Wrapper<OnlyCopyIfDisplay<S>> {}
//~^ ERROR the trait `Copy` may not be implemented for this type

fn main() {}
22 changes: 22 additions & 0 deletions src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E0204]: the trait `Copy` may not be implemented for this type
--> $DIR/missing-bound-in-manual-copy-impl-2.rs:16:9
|
LL | struct Wrapper<T>(T);
| - this field does not implement `Copy`
...
LL | impl<S> Copy for Wrapper<OnlyCopyIfDisplay<S>> {}
| ^^^^
|
note: the `Copy` impl for `OnlyCopyIfDisplay<S>` requires that `S: std::fmt::Display`
--> $DIR/missing-bound-in-manual-copy-impl-2.rs:4:19
|
LL | struct Wrapper<T>(T);
| ^
Comment on lines +10 to +14
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the late review. This span could be better but we can leave it as followup.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. You're right, this is super misleading. I'll fix it.

help: consider restricting type parameter `S`
|
LL | impl<S: std::fmt::Display> Copy for Wrapper<OnlyCopyIfDisplay<S>> {}
| +++++++++++++++++++

error: aborting due to previous error

For more information about this error, try `rustc --explain E0204`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// run-rustfix

#[derive(Clone)]
struct Wrapper<T>(T);

impl<S: Copy> Copy for Wrapper<S> {}
//~^ ERROR the trait `Copy` may not be implemented for this type

fn main() {}
9 changes: 9 additions & 0 deletions src/test/ui/suggestions/missing-bound-in-manual-copy-impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// run-rustfix

#[derive(Clone)]
struct Wrapper<T>(T);

impl<S> Copy for Wrapper<S> {}
//~^ ERROR the trait `Copy` may not be implemented for this type

fn main() {}
17 changes: 17 additions & 0 deletions src/test/ui/suggestions/missing-bound-in-manual-copy-impl.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error[E0204]: the trait `Copy` may not be implemented for this type
--> $DIR/missing-bound-in-manual-copy-impl.rs:6:9
|
LL | struct Wrapper<T>(T);
| - this field does not implement `Copy`
LL |
LL | impl<S> Copy for Wrapper<S> {}
| ^^^^
|
help: consider restricting type parameter `S`
|
LL | impl<S: Copy> Copy for Wrapper<S> {}
| ++++++

error: aborting due to previous error

For more information about this error, try `rustc --explain E0204`.