Skip to content

Commit

Permalink
Auto merge of #50536 - leodasvacas:report-fullfilment-errors-in-copy-…
Browse files Browse the repository at this point in the history
…derive, r=estebank

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 looks out of context now.

Fixes #50480

r? @estebank
  • Loading branch information
bors committed May 12, 2018
2 parents ff2ac35 + 6389f35 commit 6fb34bd
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 60 deletions.
2 changes: 1 addition & 1 deletion src/librustc/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
36 changes: 3 additions & 33 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<T, Vec<FulfillmentError<'tcx>>>
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>,
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/librustc/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
37 changes: 22 additions & 15 deletions src/librustc/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
}
Expand All @@ -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| {
Expand All @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
20 changes: 10 additions & 10 deletions src/librustc_typeck/coherence/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,24 +111,24 @@ 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
} else {
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()
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);
Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/issue-50480.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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
struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
//~^ ERROR cannot find type `NotDefined` in this scope
//~| ERROR the trait bound `i32: std::iter::Iterator` is not satisfied

fn main() {}
29 changes: 29 additions & 0 deletions src/test/ui/issue-50480.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
error[E0412]: cannot find type `NotDefined` in this scope
--> $DIR/issue-50480.rs:13:12
|
LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| ^^^^^^^^^^ not found in this scope

error[E0277]: the trait bound `i32: std::iter::Iterator` is not satisfied
--> $DIR/issue-50480.rs:13:24
|
LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, 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 | //~^ ERROR the trait `Copy` may not be implemented for this type
LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| -------- ------ this field does not implement `Copy`
| |
| this field does not implement `Copy`

error: aborting due to 3 previous errors

Some errors occurred: E0204, E0277, E0412.
For more information about an error, try `rustc --explain E0204`.

0 comments on commit 6fb34bd

Please sign in to comment.