Skip to content

Commit 16cfadb

Browse files
Suggest lifetime bound in illegal Copy impl
1 parent 333c6bf commit 16cfadb

File tree

3 files changed

+103
-57
lines changed

3 files changed

+103
-57
lines changed

compiler/rustc_hir_analysis/src/coherence/builtin.rs

+63-43
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ use rustc_hir as hir;
77
use rustc_hir::def_id::{DefId, LocalDefId};
88
use rustc_hir::lang_items::LangItem;
99
use rustc_hir::ItemKind;
10-
use rustc_infer::infer;
1110
use rustc_infer::infer::outlives::env::OutlivesEnvironment;
1211
use rustc_infer::infer::TyCtxtInferExt;
12+
use rustc_infer::infer::{self, RegionResolutionError};
1313
use rustc_middle::ty::adjustment::CoerceUnsizedInfo;
1414
use rustc_middle::ty::{self, suggest_constraining_type_params, Ty, TyCtxt, TypeVisitable};
1515
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt;
1616
use rustc_trait_selection::traits::misc::{
17-
type_allowed_to_implement_copy, CopyImplementationError,
17+
type_allowed_to_implement_copy, CopyImplementationError, InfringingFieldsReason,
1818
};
1919
use rustc_trait_selection::traits::predicate_for_trait_def;
2020
use rustc_trait_selection::traits::{self, ObligationCause};
@@ -99,50 +99,70 @@ fn visit_implementation_of_copy(tcx: TyCtxt<'_>, impl_did: LocalDefId) {
9999
let mut errors: BTreeMap<_, Vec<_>> = Default::default();
100100
let mut bounds = vec![];
101101

102-
for (field, ty) in fields {
102+
for (field, ty, reason) in fields {
103103
let field_span = tcx.def_span(field.did);
104-
let field_ty_span = match tcx.hir().get_if_local(field.did) {
105-
Some(hir::Node::Field(field_def)) => field_def.ty.span,
106-
_ => field_span,
107-
};
108104
err.span_label(field_span, "this field does not implement `Copy`");
109-
// Spin up a new FulfillmentContext, so we can get the _precise_ reason
110-
// why this field does not implement Copy. This is useful because sometimes
111-
// it is not immediately clear why Copy is not implemented for a field, since
112-
// all we point at is the field itself.
113-
let infcx = tcx.infer_ctxt().ignoring_regions().build();
114-
for error in traits::fully_solve_bound(
115-
&infcx,
116-
traits::ObligationCause::dummy_with_span(field_ty_span),
117-
param_env,
118-
ty,
119-
tcx.require_lang_item(LangItem::Copy, Some(span)),
120-
) {
121-
let error_predicate = error.obligation.predicate;
122-
// Only note if it's not the root obligation, otherwise it's trivial and
123-
// should be self-explanatory (i.e. a field literally doesn't implement Copy).
124-
125-
// FIXME: This error could be more descriptive, especially if the error_predicate
126-
// contains a foreign type or if it's a deeply nested type...
127-
if error_predicate != error.root_obligation.predicate {
128-
errors
129-
.entry((ty.to_string(), error_predicate.to_string()))
130-
.or_default()
131-
.push(error.obligation.cause.span);
105+
106+
match reason {
107+
InfringingFieldsReason::Fulfill(fulfillment_errors) => {
108+
for error in fulfillment_errors {
109+
let error_predicate = error.obligation.predicate;
110+
// Only note if it's not the root obligation, otherwise it's trivial and
111+
// should be self-explanatory (i.e. a field literally doesn't implement Copy).
112+
113+
// FIXME: This error could be more descriptive, especially if the error_predicate
114+
// contains a foreign type or if it's a deeply nested type...
115+
if error_predicate != error.root_obligation.predicate {
116+
errors
117+
.entry((ty.to_string(), error_predicate.to_string()))
118+
.or_default()
119+
.push(error.obligation.cause.span);
120+
}
121+
if let ty::PredicateKind::Clause(ty::Clause::Trait(
122+
ty::TraitPredicate {
123+
trait_ref,
124+
polarity: ty::ImplPolarity::Positive,
125+
..
126+
},
127+
)) = error_predicate.kind().skip_binder()
128+
{
129+
let ty = trait_ref.self_ty();
130+
if let ty::Param(_) = ty.kind() {
131+
bounds.push((
132+
format!("{ty}"),
133+
trait_ref.print_only_trait_path().to_string(),
134+
Some(trait_ref.def_id),
135+
));
136+
}
137+
}
138+
}
132139
}
133-
if let ty::PredicateKind::Clause(ty::Clause::Trait(ty::TraitPredicate {
134-
trait_ref,
135-
polarity: ty::ImplPolarity::Positive,
136-
..
137-
})) = error_predicate.kind().skip_binder()
138-
{
139-
let ty = trait_ref.self_ty();
140-
if let ty::Param(_) = ty.kind() {
141-
bounds.push((
142-
format!("{ty}"),
143-
trait_ref.print_only_trait_path().to_string(),
144-
Some(trait_ref.def_id),
145-
));
140+
InfringingFieldsReason::Regions(region_errors) => {
141+
for error in region_errors {
142+
let ty = ty.to_string();
143+
match error {
144+
RegionResolutionError::ConcreteFailure(origin, a, b) => {
145+
let predicate = format!("{b}: {a}");
146+
errors
147+
.entry((ty.clone(), predicate.clone()))
148+
.or_default()
149+
.push(origin.span());
150+
if let ty::RegionKind::ReEarlyBound(ebr) = *b && ebr.has_name() {
151+
bounds.push((b.to_string(), a.to_string(), None));
152+
}
153+
}
154+
RegionResolutionError::GenericBoundFailure(origin, a, b) => {
155+
let predicate = format!("{a}: {b}");
156+
errors
157+
.entry((ty.clone(), predicate.clone()))
158+
.or_default()
159+
.push(origin.span());
160+
if let infer::region_constraints::GenericKind::Param(_) = a {
161+
bounds.push((a.to_string(), b.to_string(), None));
162+
}
163+
}
164+
_ => continue,
165+
}
146166
}
147167
}
148168
}

compiler/rustc_trait_selection/src/traits/misc.rs

+30-14
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,25 @@ use crate::traits::{self, ObligationCause};
44

55
use rustc_data_structures::fx::FxIndexSet;
66
use rustc_hir as hir;
7-
use rustc_infer::infer::outlives::env::OutlivesEnvironment;
8-
use rustc_infer::infer::TyCtxtInferExt;
7+
use rustc_infer::infer::{RegionResolutionError, TyCtxtInferExt};
8+
use rustc_infer::{infer::outlives::env::OutlivesEnvironment, traits::FulfillmentError};
99
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitable};
1010

1111
use crate::traits::error_reporting::TypeErrCtxtExt;
1212

1313
use super::outlives_bounds::InferCtxtExt;
1414

15-
#[derive(Clone)]
1615
pub enum CopyImplementationError<'tcx> {
17-
InfrigingFields(Vec<(&'tcx ty::FieldDef, Ty<'tcx>)>),
16+
InfrigingFields(Vec<(&'tcx ty::FieldDef, Ty<'tcx>, InfringingFieldsReason<'tcx>)>),
1817
NotAnAdt,
1918
HasDestructor,
2019
}
2120

21+
pub enum InfringingFieldsReason<'tcx> {
22+
Fulfill(Vec<FulfillmentError<'tcx>>),
23+
Regions(Vec<RegionResolutionError<'tcx>>),
24+
}
25+
2226
/// Checks that the fields of the type (an ADT) all implement copy.
2327
///
2428
/// If fields don't implement copy, return an error containing a list of
@@ -60,22 +64,27 @@ pub fn type_allowed_to_implement_copy<'tcx>(
6064
if ty.references_error() {
6165
continue;
6266
}
63-
let span = tcx.def_span(field.did);
67+
68+
let field_span = tcx.def_span(field.did);
69+
let field_ty_span = match tcx.hir().get_if_local(field.did) {
70+
Some(hir::Node::Field(field_def)) => field_def.ty.span,
71+
_ => field_span,
72+
};
73+
6474
// FIXME(compiler-errors): This gives us better spans for bad
6575
// projection types like in issue-50480.
6676
// If the ADT has substs, point to the cause we are given.
6777
// If it does not, then this field probably doesn't normalize
6878
// to begin with, and point to the bad field's span instead.
69-
let cause = if field
79+
let normalization_cause = if field
7080
.ty(tcx, traits::InternalSubsts::identity_for_item(tcx, adt.did()))
7181
.has_non_region_param()
7282
{
7383
parent_cause.clone()
7484
} else {
75-
ObligationCause::dummy_with_span(span)
85+
ObligationCause::dummy_with_span(field_ty_span)
7686
};
77-
78-
let ty = ocx.normalize(&cause, param_env, ty);
87+
let ty = ocx.normalize(&normalization_cause, param_env, ty);
7988
let normalization_errors = ocx.select_where_possible();
8089
if !normalization_errors.is_empty() {
8190
// Don't report this as a field that doesn't implement Copy,
@@ -84,9 +93,15 @@ pub fn type_allowed_to_implement_copy<'tcx>(
8493
continue;
8594
}
8695

87-
ocx.register_bound(cause, param_env, ty, copy_def_id);
88-
if !ocx.select_all_or_error().is_empty() {
89-
infringing.push((field, ty));
96+
ocx.register_bound(
97+
ObligationCause::dummy_with_span(field_ty_span),
98+
param_env,
99+
ty,
100+
copy_def_id,
101+
);
102+
let errors = ocx.select_all_or_error();
103+
if !errors.is_empty() {
104+
infringing.push((field, ty, InfringingFieldsReason::Fulfill(errors)));
90105
}
91106

92107
// Check regions assuming the self type of the impl is WF
@@ -103,8 +118,9 @@ pub fn type_allowed_to_implement_copy<'tcx>(
103118
outlives_env.region_bound_pairs(),
104119
param_env,
105120
);
106-
if !infcx.resolve_regions(&outlives_env).is_empty() {
107-
infringing.push((field, ty));
121+
let errors = infcx.resolve_regions(&outlives_env);
122+
if !errors.is_empty() {
123+
infringing.push((field, ty, InfringingFieldsReason::Regions(errors)));
108124
}
109125
}
110126
}

src/test/ui/traits/copy-is-not-modulo-regions.not_static.stderr

+10
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@ LL | struct Bar<'lt>(Foo<'lt>);
66
...
77
LL | impl<'any> Copy for Bar<'any> {}
88
| ^^^^^^^^^
9+
|
10+
note: the `Copy` impl for `Foo<'any>` requires that `'any: 'static`
11+
--> $DIR/copy-is-not-modulo-regions.rs:10:17
12+
|
13+
LL | struct Bar<'lt>(Foo<'lt>);
14+
| ^^^^^^^^
15+
help: consider restricting type parameter `'any`
16+
|
17+
LL | impl<'any: 'static> Copy for Bar<'any> {}
18+
| +++++++++
919

1020
error: aborting due to previous error
1121

0 commit comments

Comments
 (0)