From 804bcf7716f14a55e85c599733a58348b5c05c82 Mon Sep 17 00:00:00 2001 From: "leonardo.yvens" Date: Tue, 8 May 2018 11:38:35 -0300 Subject: [PATCH 1/3] Better error reporting in Copy derive In Copy derive, report all fulfillment erros when present and do not report errors for types tainted with `TyErr`. Also report all fields which are not Copy rather than just the first. Also refactored `fn fully_normalize`, removing the not very useful helper function along with a FIXME to the closed issue #26721 that's looks out of context now. --- src/librustc/middle/stability.rs | 2 +- src/librustc/traits/mod.rs | 36 ++--------------------- src/librustc/traits/specialize/mod.rs | 1 + src/librustc/ty/util.rs | 37 ++++++++++++++---------- src/librustc_lint/builtin.rs | 2 +- src/librustc_typeck/coherence/builtin.rs | 22 +++++++------- src/test/ui/issue-50480.rs | 18 ++++++++++++ src/test/ui/issue-50480.stderr | 36 +++++++++++++++++++++++ 8 files changed, 94 insertions(+), 60 deletions(-) create mode 100644 src/test/ui/issue-50480.rs create mode 100644 src/test/ui/issue-50480.stderr diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 279908d2b675f..d6a7d5e8472ac 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -764,7 +764,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { "unions with `Drop` implementations are unstable"); } else { let param_env = self.tcx.param_env(def_id); - if !param_env.can_type_implement_copy(self.tcx, ty, item.span).is_ok() { + if !param_env.can_type_implement_copy(self.tcx, ty).is_ok() { emit_feature_err(&self.tcx.sess.parse_sess, "untagged_unions", item.span, GateIssue::Language, "unions with non-`Copy` fields are unstable"); diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 10d88063ac35f..a765ffe2396bf 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -674,7 +674,7 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // we move over to lazy normalization *anyway*. let fulfill_cx = FulfillmentContext::new_ignoring_regions(); - let predicates = match fully_normalize_with_fulfillcx( + let predicates = match fully_normalize( &infcx, fulfill_cx, cause, @@ -734,31 +734,7 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }) } -pub fn fully_normalize<'a, 'gcx, 'tcx, T>(infcx: &InferCtxt<'a, 'gcx, 'tcx>, - cause: ObligationCause<'tcx>, - param_env: ty::ParamEnv<'tcx>, - value: &T) - -> Result>> - where T : TypeFoldable<'tcx> -{ - // FIXME (@jroesch) ISSUE 26721 - // I'm not sure if this is a bug or not, needs further investigation. - // It appears that by reusing the fulfillment_cx here we incur more - // obligations and later trip an assertion on regionck.rs line 337. - // - // The two possibilities I see is: - // - normalization is not actually fully happening and we - // have a bug else where - // - we are adding a duplicate bound into the list causing - // its size to change. - // - // I think we should probably land this refactor and then come - // back to this is a follow-up patch. - let fulfillcx = FulfillmentContext::new(); - fully_normalize_with_fulfillcx(infcx, fulfillcx, cause, param_env, value) -} - -pub fn fully_normalize_with_fulfillcx<'a, 'gcx, 'tcx, T>( +pub fn fully_normalize<'a, 'gcx, 'tcx, T>( infcx: &InferCtxt<'a, 'gcx, 'tcx>, mut fulfill_cx: FulfillmentContext<'tcx>, cause: ObligationCause<'tcx>, @@ -779,13 +755,7 @@ pub fn fully_normalize_with_fulfillcx<'a, 'gcx, 'tcx, T>( } debug!("fully_normalize: select_all_or_error start"); - match fulfill_cx.select_all_or_error(infcx) { - Ok(()) => { } - Err(e) => { - debug!("fully_normalize: error={:?}", e); - return Err(e); - } - } + fulfill_cx.select_all_or_error(infcx)?; debug!("fully_normalize: select_all_or_error complete"); let resolved_value = infcx.resolve_type_vars_if_possible(&normalized_value); debug!("fully_normalize: resolved_value={:?}", resolved_value); diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index 30b2c55afa194..d33806285142e 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/src/librustc/traits/specialize/mod.rs @@ -196,6 +196,7 @@ pub(super) fn specializes<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // that this always succeeds. let impl1_trait_ref = match traits::fully_normalize(&infcx, + FulfillmentContext::new(), ObligationCause::dummy(), penv, &impl1_trait_ref) { diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index 76803f4503129..fdd0754730feb 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -16,7 +16,7 @@ use hir::map::{DefPathData, Node}; use hir; use ich::NodeIdHashingMode; use middle::const_val::ConstVal; -use traits; +use traits::{self, ObligationCause}; use ty::{self, Ty, TyCtxt, TypeFoldable}; use ty::fold::TypeVisitor; use ty::subst::UnpackedKind; @@ -166,9 +166,9 @@ impl IntTypeExt for attr::IntType { } -#[derive(Copy, Clone)] +#[derive(Clone)] pub enum CopyImplementationError<'tcx> { - InfrigingField(&'tcx ty::FieldDef), + InfrigingFields(Vec<&'tcx ty::FieldDef>), NotAnAdt, HasDestructor, } @@ -191,7 +191,7 @@ pub enum Representability { impl<'tcx> ty::ParamEnv<'tcx> { pub fn can_type_implement_copy<'a>(self, tcx: TyCtxt<'a, 'tcx, 'tcx>, - self_type: Ty<'tcx>, span: Span) + self_type: Ty<'tcx>) -> Result<(), CopyImplementationError<'tcx>> { // FIXME: (@jroesch) float this code up tcx.infer_ctxt().enter(|infcx| { @@ -207,22 +207,29 @@ impl<'tcx> ty::ParamEnv<'tcx> { _ => return Err(CopyImplementationError::NotAnAdt), }; - let field_implements_copy = |field: &ty::FieldDef| { - let cause = traits::ObligationCause::dummy(); - match traits::fully_normalize(&infcx, cause, self, &field.ty(tcx, substs)) { - Ok(ty) => !infcx.type_moves_by_default(self, ty, span), - Err(..) => false, - } - }; - + let mut infringing = Vec::new(); for variant in &adt.variants { for field in &variant.fields { - if !field_implements_copy(field) { - return Err(CopyImplementationError::InfrigingField(field)); + let span = tcx.def_span(field.did); + let ty = field.ty(tcx, substs); + if ty.references_error() { + continue; } + let cause = ObligationCause { span, ..ObligationCause::dummy() }; + let ctx = traits::FulfillmentContext::new(); + match traits::fully_normalize(&infcx, ctx, cause, self, &ty) { + Ok(ty) => if infcx.type_moves_by_default(self, ty, span) { + infringing.push(field); + } + Err(errors) => { + infcx.report_fulfillment_errors(&errors, None, false); + } + }; } } - + if !infringing.is_empty() { + return Err(CopyImplementationError::InfrigingFields(infringing)); + } if adt.has_dtor(tcx) { return Err(CopyImplementationError::HasDestructor); } diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 251b95a6fcb79..1d8016fb0f979 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -541,7 +541,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingCopyImplementations { if !ty.moves_by_default(cx.tcx, param_env, item.span) { return; } - if param_env.can_type_implement_copy(cx.tcx, ty, item.span).is_ok() { + if param_env.can_type_implement_copy(cx.tcx, ty).is_ok() { cx.span_lint(MISSING_COPY_IMPLEMENTATIONS, item.span, "type could implement `Copy`; consider adding `impl \ diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs index 3424a31e09df0..7627d9071dc0c 100644 --- a/src/librustc_typeck/coherence/builtin.rs +++ b/src/librustc_typeck/coherence/builtin.rs @@ -111,9 +111,9 @@ fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, debug!("visit_implementation_of_copy: self_type={:?} (free)", self_type); - match param_env.can_type_implement_copy(tcx, self_type, span) { + match param_env.can_type_implement_copy(tcx, self_type) { Ok(()) => {} - Err(CopyImplementationError::InfrigingField(field)) => { + Err(CopyImplementationError::InfrigingFields(fields)) => { let item = tcx.hir.expect_item(impl_node_id); let span = if let ItemImpl(.., Some(ref tr), _, _) = item.node { tr.path.span @@ -121,14 +121,16 @@ fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, span }; - struct_span_err!(tcx.sess, - span, - E0204, - "the trait `Copy` may not be implemented for this type") - .span_label( - tcx.def_span(field.did), - "this field does not implement `Copy`") - .emit() + for field in fields { + struct_span_err!(tcx.sess, + span, + E0204, + "the trait `Copy` may not be implemented for this type") + .span_label( + tcx.def_span(field.did), + "this field does not implement `Copy`") + .emit() + } } Err(CopyImplementationError::NotAnAdt) => { let item = tcx.hir.expect_item(impl_node_id); diff --git a/src/test/ui/issue-50480.rs b/src/test/ui/issue-50480.rs new file mode 100644 index 0000000000000..20af6d4cf83d5 --- /dev/null +++ b/src/test/ui/issue-50480.rs @@ -0,0 +1,18 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[derive(Clone, Copy)] +//~^ ERROR the trait `Copy` may not be implemented for this type +//~| ERROR the trait `Copy` may not be implemented for this type +struct Foo(NotDefined, ::Item, Vec, String); +//~^ ERROR cannot find type `NotDefined` in this scope +//~| ERROR the trait bound `i32: std::iter::Iterator` is not satisfied + +fn main() {} diff --git a/src/test/ui/issue-50480.stderr b/src/test/ui/issue-50480.stderr new file mode 100644 index 0000000000000..7aeecfaabb312 --- /dev/null +++ b/src/test/ui/issue-50480.stderr @@ -0,0 +1,36 @@ +error[E0412]: cannot find type `NotDefined` in this scope + --> $DIR/issue-50480.rs:14:12 + | +LL | struct Foo(NotDefined, ::Item, Vec, String); + | ^^^^^^^^^^ not found in this scope + +error[E0277]: the trait bound `i32: std::iter::Iterator` is not satisfied + --> $DIR/issue-50480.rs:14:24 + | +LL | struct Foo(NotDefined, ::Item, Vec, String); + | ^^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator; maybe try calling `.iter()` or a similar method + | + = help: the trait `std::iter::Iterator` is not implemented for `i32` + +error[E0204]: the trait `Copy` may not be implemented for this type + --> $DIR/issue-50480.rs:11:17 + | +LL | #[derive(Clone, Copy)] + | ^^^^ +... +LL | struct Foo(NotDefined, ::Item, Vec, String); + | --------- this field does not implement `Copy` + +error[E0204]: the trait `Copy` may not be implemented for this type + --> $DIR/issue-50480.rs:11:17 + | +LL | #[derive(Clone, Copy)] + | ^^^^ +... +LL | struct Foo(NotDefined, ::Item, Vec, String); + | ------- this field does not implement `Copy` + +error: aborting due to 4 previous errors + +Some errors occurred: E0204, E0277, E0412. +For more information about an error, try `rustc --explain E0204`. From 3deb75729eb08d930da90f2e9407b2f79f0025b7 Mon Sep 17 00:00:00 2001 From: "leonardo.yvens" Date: Wed, 9 May 2018 10:05:59 -0300 Subject: [PATCH 2/3] Merge all "Copy not implemented" errors --- src/librustc_typeck/coherence/builtin.rs | 16 +++++++--------- src/test/ui/issue-50480.rs | 1 - src/test/ui/issue-50480.stderr | 21 +++++++-------------- 3 files changed, 14 insertions(+), 24 deletions(-) diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs index 7627d9071dc0c..2f08a54e10f08 100644 --- a/src/librustc_typeck/coherence/builtin.rs +++ b/src/librustc_typeck/coherence/builtin.rs @@ -121,16 +121,14 @@ fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, span }; - for field in fields { - struct_span_err!(tcx.sess, - span, - E0204, - "the trait `Copy` may not be implemented for this type") - .span_label( - tcx.def_span(field.did), - "this field does not implement `Copy`") - .emit() + let mut err = struct_span_err!(tcx.sess, + span, + 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`"); } + err.emit() } Err(CopyImplementationError::NotAnAdt) => { let item = tcx.hir.expect_item(impl_node_id); diff --git a/src/test/ui/issue-50480.rs b/src/test/ui/issue-50480.rs index 20af6d4cf83d5..3427cf6bf9ca2 100644 --- a/src/test/ui/issue-50480.rs +++ b/src/test/ui/issue-50480.rs @@ -10,7 +10,6 @@ #[derive(Clone, Copy)] //~^ ERROR the trait `Copy` may not be implemented for this type -//~| ERROR the trait `Copy` may not be implemented for this type struct Foo(NotDefined, ::Item, Vec, String); //~^ ERROR cannot find type `NotDefined` in this scope //~| ERROR the trait bound `i32: std::iter::Iterator` is not satisfied diff --git a/src/test/ui/issue-50480.stderr b/src/test/ui/issue-50480.stderr index 7aeecfaabb312..3714a0b72b7b8 100644 --- a/src/test/ui/issue-50480.stderr +++ b/src/test/ui/issue-50480.stderr @@ -1,11 +1,11 @@ error[E0412]: cannot find type `NotDefined` in this scope - --> $DIR/issue-50480.rs:14:12 + --> $DIR/issue-50480.rs:13:12 | LL | struct Foo(NotDefined, ::Item, Vec, String); | ^^^^^^^^^^ not found in this scope error[E0277]: the trait bound `i32: std::iter::Iterator` is not satisfied - --> $DIR/issue-50480.rs:14:24 + --> $DIR/issue-50480.rs:13:24 | LL | struct Foo(NotDefined, ::Item, Vec, String); | ^^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator; maybe try calling `.iter()` or a similar method @@ -17,20 +17,13 @@ error[E0204]: the trait `Copy` may not be implemented for this type | LL | #[derive(Clone, Copy)] | ^^^^ -... +LL | //~^ ERROR the trait `Copy` may not be implemented for this type LL | struct Foo(NotDefined, ::Item, Vec, String); - | --------- this field does not implement `Copy` + | --------- ------- this field does not implement `Copy` + | | + | this field does not implement `Copy` -error[E0204]: the trait `Copy` may not be implemented for this type - --> $DIR/issue-50480.rs:11:17 - | -LL | #[derive(Clone, Copy)] - | ^^^^ -... -LL | struct Foo(NotDefined, ::Item, Vec, String); - | ------- this field does not implement `Copy` - -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors Some errors occurred: E0204, E0277, E0412. For more information about an error, try `rustc --explain E0204`. From 6389f35ef985f6bf354b7cd848c94e446d08d2bc Mon Sep 17 00:00:00 2001 From: "leonardo.yvens" Date: Sat, 12 May 2018 15:07:15 -0300 Subject: [PATCH 3/3] Fix rebase --- src/test/ui/issue-50480.stderr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/issue-50480.stderr b/src/test/ui/issue-50480.stderr index 3714a0b72b7b8..f5281fec4d1ea 100644 --- a/src/test/ui/issue-50480.stderr +++ b/src/test/ui/issue-50480.stderr @@ -8,7 +8,7 @@ error[E0277]: the trait bound `i32: std::iter::Iterator` is not satisfied --> $DIR/issue-50480.rs:13:24 | LL | struct Foo(NotDefined, ::Item, Vec, String); - | ^^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator; maybe try calling `.iter()` or a similar method + | ^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator; maybe try calling `.iter()` or a similar method | = help: the trait `std::iter::Iterator` is not implemented for `i32` @@ -19,7 +19,7 @@ LL | #[derive(Clone, Copy)] | ^^^^ LL | //~^ ERROR the trait `Copy` may not be implemented for this type LL | struct Foo(NotDefined, ::Item, Vec, String); - | --------- ------- this field does not implement `Copy` + | -------- ------ this field does not implement `Copy` | | | this field does not implement `Copy`