Skip to content

Commit

Permalink
Rollup merge of #99741 - compiler-errors:copy-impl-impl-generics, r=f…
Browse files Browse the repository at this point in the history
…ee1-dead

Use `impl`'s generics when suggesting fix on bad `impl Copy`

See the UI test for a more complicated example, but we weren't correctly suggesting to add bounds given a manual `impl` whose generics didn't match the struct generics.

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

impl<S> Copy for Wrapper<S> {}
```

Coincidentally this fix didn't cause any regressions for `derive(Copy)` impls, I think because those use the same spans in the impl generics as the struct generics, so the machinery still applies the same change.
  • Loading branch information
Dylan-DPC authored Jul 31, 2022
2 parents 403c1b3 + 1390220 commit 2c14bc3
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 18 deletions.
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);
| ^
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`.

0 comments on commit 2c14bc3

Please sign in to comment.