Skip to content

Commit

Permalink
Rollup merge of rust-lang#94249 - compiler-errors:better-copy-errors,…
Browse files Browse the repository at this point in the history
… r=davidtwco

Better errors when a Copy impl on a Struct is not self-consistent

As discovered in a Zulip thread with `@nnethercote` and `@Mark-Simulacrum,` it's not immediately obvious why a field on an ADT doesn't implement `Copy`.  This PR attempts to give slightly more detailed information by spinning up a fulfillment context to try to dig down and discover transitive fulfillment errors that cause `is_copy_modulo_regions` to fail on a ADT field.

The error message still kinda sucks, but should only show up in the case that an existing error message was totally missing... so I think it's a good compromise for now?
  • Loading branch information
matthiaskrgr authored Mar 23, 2022
2 parents 547369d + c8cbd3d commit af19a50
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 4 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::traits::error_reporting::InferCtxtExt;

#[derive(Clone)]
pub enum CopyImplementationError<'tcx> {
InfrigingFields(Vec<&'tcx ty::FieldDef>),
InfrigingFields(Vec<(&'tcx ty::FieldDef, Ty<'tcx>)>),
NotAnAdt,
HasDestructor,
}
Expand Down Expand Up @@ -67,7 +67,7 @@ pub fn can_type_implement_copy<'tcx>(
match traits::fully_normalize(&infcx, ctx, cause, param_env, ty) {
Ok(ty) => {
if !infcx.type_is_copy_modulo_regions(param_env, ty, span) {
infringing.push(field);
infringing.push((field, ty));
}
}
Err(errors) => {
Expand Down
36 changes: 34 additions & 2 deletions compiler/rustc_typeck/src/coherence/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,40 @@ fn visit_implementation_of_copy(tcx: TyCtxt<'_>, impl_did: LocalDefId) {
E0204,
"the trait `Copy` may not be implemented for this type"
);
for span in fields.iter().map(|f| tcx.def_span(f.did)) {
err.span_label(span, "this field does not implement `Copy`");
for (field, ty) in fields {
let field_span = tcx.def_span(field.did);
err.span_label(field_span, "this field does not implement `Copy`");
// Spin up a new FulfillmentContext, so we can get the _precise_ reason
// why this field does not implement Copy. This is useful because sometimes
// it is not immediately clear why Copy is not implemented for a field, since
// all we point at is the field itself.
tcx.infer_ctxt().enter(|infcx| {
let mut fulfill_cx = traits::FulfillmentContext::new_ignoring_regions();
fulfill_cx.register_bound(
&infcx,
param_env,
ty,
tcx.lang_items().copy_trait().unwrap(),
traits::ObligationCause::dummy_with_span(field_span),
);
for error in fulfill_cx.select_all_or_error(&infcx) {
let error_predicate = error.obligation.predicate;
// Only note if it's not the root obligation, otherwise it's trivial and
// should be self-explanatory (i.e. a field literally doesn't implement Copy).

// FIXME: This error could be more descriptive, especially if the error_predicate
// contains a foreign type or if it's a deeply nested type...
if error_predicate != error.root_obligation.predicate {
err.span_note(
error.obligation.cause.span,
&format!(
"the `Copy` impl for `{}` requires that `{}`",
ty, error_predicate
),
);
}
}
});
}
err.emit();
}
Expand Down
40 changes: 40 additions & 0 deletions src/test/ui/coherence/deep-bad-copy-reason.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#![feature(extern_types)]

extern "Rust" {
type OpaqueListContents;
}

pub struct ListS<T> {
len: usize,
data: [T; 0],
opaque: OpaqueListContents,
}

pub struct Interned<'a, T>(&'a T);

impl<'a, T> Clone for Interned<'a, T> {
fn clone(&self) -> Self {
*self
}
}

impl<'a, T> Copy for Interned<'a, T> {}

pub struct List<'tcx, T>(Interned<'tcx, ListS<T>>);
//~^ NOTE this field does not implement `Copy`
//~| NOTE the `Copy` impl for `Interned<'tcx, ListS<T>>` requires that `OpaqueListContents: Sized`

impl<'tcx, T> Clone for List<'tcx, T> {
fn clone(&self) -> Self {
*self
}
}

impl<'tcx, T> Copy for List<'tcx, T> {}
//~^ ERROR the trait `Copy` may not be implemented for this type

fn assert_is_copy<T: Copy>() {}

fn main() {
assert_is_copy::<List<'static, ()>>();
}
18 changes: 18 additions & 0 deletions src/test/ui/coherence/deep-bad-copy-reason.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error[E0204]: the trait `Copy` may not be implemented for this type
--> $DIR/deep-bad-copy-reason.rs:33:15
|
LL | pub struct List<'tcx, T>(Interned<'tcx, ListS<T>>);
| ------------------------ this field does not implement `Copy`
...
LL | impl<'tcx, T> Copy for List<'tcx, T> {}
| ^^^^
|
note: the `Copy` impl for `Interned<'tcx, ListS<T>>` requires that `OpaqueListContents: Sized`
--> $DIR/deep-bad-copy-reason.rs:23:26
|
LL | pub struct List<'tcx, T>(Interned<'tcx, ListS<T>>);
| ^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0204`.
6 changes: 6 additions & 0 deletions src/test/ui/union/union-copy.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ LL | a: std::mem::ManuallyDrop<String>
...
LL | impl Copy for W {}
| ^^^^
|
note: the `Copy` impl for `ManuallyDrop<String>` requires that `String: Copy`
--> $DIR/union-copy.rs:8:5
|
LL | a: std::mem::ManuallyDrop<String>
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Expand Down

0 comments on commit af19a50

Please sign in to comment.