Skip to content

Commit 804bcf7

Browse files
committed
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 rust-lang#26721 that's looks out of context now.
1 parent e6db79f commit 804bcf7

File tree

8 files changed

+94
-60
lines changed

8 files changed

+94
-60
lines changed

src/librustc/middle/stability.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -764,7 +764,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> {
764764
"unions with `Drop` implementations are unstable");
765765
} else {
766766
let param_env = self.tcx.param_env(def_id);
767-
if !param_env.can_type_implement_copy(self.tcx, ty, item.span).is_ok() {
767+
if !param_env.can_type_implement_copy(self.tcx, ty).is_ok() {
768768
emit_feature_err(&self.tcx.sess.parse_sess,
769769
"untagged_unions", item.span, GateIssue::Language,
770770
"unions with non-`Copy` fields are unstable");

src/librustc/traits/mod.rs

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -674,7 +674,7 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
674674
// we move over to lazy normalization *anyway*.
675675
let fulfill_cx = FulfillmentContext::new_ignoring_regions();
676676

677-
let predicates = match fully_normalize_with_fulfillcx(
677+
let predicates = match fully_normalize(
678678
&infcx,
679679
fulfill_cx,
680680
cause,
@@ -734,31 +734,7 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
734734
})
735735
}
736736

737-
pub fn fully_normalize<'a, 'gcx, 'tcx, T>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
738-
cause: ObligationCause<'tcx>,
739-
param_env: ty::ParamEnv<'tcx>,
740-
value: &T)
741-
-> Result<T, Vec<FulfillmentError<'tcx>>>
742-
where T : TypeFoldable<'tcx>
743-
{
744-
// FIXME (@jroesch) ISSUE 26721
745-
// I'm not sure if this is a bug or not, needs further investigation.
746-
// It appears that by reusing the fulfillment_cx here we incur more
747-
// obligations and later trip an assertion on regionck.rs line 337.
748-
//
749-
// The two possibilities I see is:
750-
// - normalization is not actually fully happening and we
751-
// have a bug else where
752-
// - we are adding a duplicate bound into the list causing
753-
// its size to change.
754-
//
755-
// I think we should probably land this refactor and then come
756-
// back to this is a follow-up patch.
757-
let fulfillcx = FulfillmentContext::new();
758-
fully_normalize_with_fulfillcx(infcx, fulfillcx, cause, param_env, value)
759-
}
760-
761-
pub fn fully_normalize_with_fulfillcx<'a, 'gcx, 'tcx, T>(
737+
pub fn fully_normalize<'a, 'gcx, 'tcx, T>(
762738
infcx: &InferCtxt<'a, 'gcx, 'tcx>,
763739
mut fulfill_cx: FulfillmentContext<'tcx>,
764740
cause: ObligationCause<'tcx>,
@@ -779,13 +755,7 @@ pub fn fully_normalize_with_fulfillcx<'a, 'gcx, 'tcx, T>(
779755
}
780756

781757
debug!("fully_normalize: select_all_or_error start");
782-
match fulfill_cx.select_all_or_error(infcx) {
783-
Ok(()) => { }
784-
Err(e) => {
785-
debug!("fully_normalize: error={:?}", e);
786-
return Err(e);
787-
}
788-
}
758+
fulfill_cx.select_all_or_error(infcx)?;
789759
debug!("fully_normalize: select_all_or_error complete");
790760
let resolved_value = infcx.resolve_type_vars_if_possible(&normalized_value);
791761
debug!("fully_normalize: resolved_value={:?}", resolved_value);

src/librustc/traits/specialize/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ pub(super) fn specializes<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
196196
// that this always succeeds.
197197
let impl1_trait_ref =
198198
match traits::fully_normalize(&infcx,
199+
FulfillmentContext::new(),
199200
ObligationCause::dummy(),
200201
penv,
201202
&impl1_trait_ref) {

src/librustc/ty/util.rs

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use hir::map::{DefPathData, Node};
1616
use hir;
1717
use ich::NodeIdHashingMode;
1818
use middle::const_val::ConstVal;
19-
use traits;
19+
use traits::{self, ObligationCause};
2020
use ty::{self, Ty, TyCtxt, TypeFoldable};
2121
use ty::fold::TypeVisitor;
2222
use ty::subst::UnpackedKind;
@@ -166,9 +166,9 @@ impl IntTypeExt for attr::IntType {
166166
}
167167

168168

169-
#[derive(Copy, Clone)]
169+
#[derive(Clone)]
170170
pub enum CopyImplementationError<'tcx> {
171-
InfrigingField(&'tcx ty::FieldDef),
171+
InfrigingFields(Vec<&'tcx ty::FieldDef>),
172172
NotAnAdt,
173173
HasDestructor,
174174
}
@@ -191,7 +191,7 @@ pub enum Representability {
191191
impl<'tcx> ty::ParamEnv<'tcx> {
192192
pub fn can_type_implement_copy<'a>(self,
193193
tcx: TyCtxt<'a, 'tcx, 'tcx>,
194-
self_type: Ty<'tcx>, span: Span)
194+
self_type: Ty<'tcx>)
195195
-> Result<(), CopyImplementationError<'tcx>> {
196196
// FIXME: (@jroesch) float this code up
197197
tcx.infer_ctxt().enter(|infcx| {
@@ -207,22 +207,29 @@ impl<'tcx> ty::ParamEnv<'tcx> {
207207
_ => return Err(CopyImplementationError::NotAnAdt),
208208
};
209209

210-
let field_implements_copy = |field: &ty::FieldDef| {
211-
let cause = traits::ObligationCause::dummy();
212-
match traits::fully_normalize(&infcx, cause, self, &field.ty(tcx, substs)) {
213-
Ok(ty) => !infcx.type_moves_by_default(self, ty, span),
214-
Err(..) => false,
215-
}
216-
};
217-
210+
let mut infringing = Vec::new();
218211
for variant in &adt.variants {
219212
for field in &variant.fields {
220-
if !field_implements_copy(field) {
221-
return Err(CopyImplementationError::InfrigingField(field));
213+
let span = tcx.def_span(field.did);
214+
let ty = field.ty(tcx, substs);
215+
if ty.references_error() {
216+
continue;
222217
}
218+
let cause = ObligationCause { span, ..ObligationCause::dummy() };
219+
let ctx = traits::FulfillmentContext::new();
220+
match traits::fully_normalize(&infcx, ctx, cause, self, &ty) {
221+
Ok(ty) => if infcx.type_moves_by_default(self, ty, span) {
222+
infringing.push(field);
223+
}
224+
Err(errors) => {
225+
infcx.report_fulfillment_errors(&errors, None, false);
226+
}
227+
};
223228
}
224229
}
225-
230+
if !infringing.is_empty() {
231+
return Err(CopyImplementationError::InfrigingFields(infringing));
232+
}
226233
if adt.has_dtor(tcx) {
227234
return Err(CopyImplementationError::HasDestructor);
228235
}

src/librustc_lint/builtin.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingCopyImplementations {
541541
if !ty.moves_by_default(cx.tcx, param_env, item.span) {
542542
return;
543543
}
544-
if param_env.can_type_implement_copy(cx.tcx, ty, item.span).is_ok() {
544+
if param_env.can_type_implement_copy(cx.tcx, ty).is_ok() {
545545
cx.span_lint(MISSING_COPY_IMPLEMENTATIONS,
546546
item.span,
547547
"type could implement `Copy`; consider adding `impl \

src/librustc_typeck/coherence/builtin.rs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -111,24 +111,26 @@ fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
111111
debug!("visit_implementation_of_copy: self_type={:?} (free)",
112112
self_type);
113113

114-
match param_env.can_type_implement_copy(tcx, self_type, span) {
114+
match param_env.can_type_implement_copy(tcx, self_type) {
115115
Ok(()) => {}
116-
Err(CopyImplementationError::InfrigingField(field)) => {
116+
Err(CopyImplementationError::InfrigingFields(fields)) => {
117117
let item = tcx.hir.expect_item(impl_node_id);
118118
let span = if let ItemImpl(.., Some(ref tr), _, _) = item.node {
119119
tr.path.span
120120
} else {
121121
span
122122
};
123123

124-
struct_span_err!(tcx.sess,
125-
span,
126-
E0204,
127-
"the trait `Copy` may not be implemented for this type")
128-
.span_label(
129-
tcx.def_span(field.did),
130-
"this field does not implement `Copy`")
131-
.emit()
124+
for field in fields {
125+
struct_span_err!(tcx.sess,
126+
span,
127+
E0204,
128+
"the trait `Copy` may not be implemented for this type")
129+
.span_label(
130+
tcx.def_span(field.did),
131+
"this field does not implement `Copy`")
132+
.emit()
133+
}
132134
}
133135
Err(CopyImplementationError::NotAnAdt) => {
134136
let item = tcx.hir.expect_item(impl_node_id);

src/test/ui/issue-50480.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#[derive(Clone, Copy)]
12+
//~^ ERROR the trait `Copy` may not be implemented for this type
13+
//~| ERROR the trait `Copy` may not be implemented for this type
14+
struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
15+
//~^ ERROR cannot find type `NotDefined` in this scope
16+
//~| ERROR the trait bound `i32: std::iter::Iterator` is not satisfied
17+
18+
fn main() {}

src/test/ui/issue-50480.stderr

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
error[E0412]: cannot find type `NotDefined` in this scope
2+
--> $DIR/issue-50480.rs:14:12
3+
|
4+
LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
5+
| ^^^^^^^^^^ not found in this scope
6+
7+
error[E0277]: the trait bound `i32: std::iter::Iterator` is not satisfied
8+
--> $DIR/issue-50480.rs:14:24
9+
|
10+
LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator; maybe try calling `.iter()` or a similar method
12+
|
13+
= help: the trait `std::iter::Iterator` is not implemented for `i32`
14+
15+
error[E0204]: the trait `Copy` may not be implemented for this type
16+
--> $DIR/issue-50480.rs:11:17
17+
|
18+
LL | #[derive(Clone, Copy)]
19+
| ^^^^
20+
...
21+
LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
22+
| --------- this field does not implement `Copy`
23+
24+
error[E0204]: the trait `Copy` may not be implemented for this type
25+
--> $DIR/issue-50480.rs:11:17
26+
|
27+
LL | #[derive(Clone, Copy)]
28+
| ^^^^
29+
...
30+
LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
31+
| ------- this field does not implement `Copy`
32+
33+
error: aborting due to 4 previous errors
34+
35+
Some errors occurred: E0204, E0277, E0412.
36+
For more information about an error, try `rustc --explain E0204`.

0 commit comments

Comments
 (0)