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

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

Merged
merged 1 commit into from
Mar 24, 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
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