Skip to content

Commit 94a300b

Browse files
borsbors
committed
Auto merge of #105102 - compiler-errors:copy-impl-considering-regions, r=lcnr
Check ADT fields for copy implementations considering regions Fixes #88901 r? `@ghost`
2 parents 5ce39f4 + 75074e0 commit 94a300b

File tree

10 files changed

+218
-102
lines changed

10 files changed

+218
-102
lines changed

‎compiler/rustc_hir_analysis/src/coherence/builtin.rs

+66-44
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,15 @@ 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;
16-
use rustc_trait_selection::traits::misc::{can_type_implement_copy, CopyImplementationError};
16+
use rustc_trait_selection::traits::misc::{
17+
type_allowed_to_implement_copy, CopyImplementationError, InfringingFieldsReason,
18+
};
1719
use rustc_trait_selection::traits::predicate_for_trait_def;
1820
use rustc_trait_selection::traits::{self, ObligationCause};
1921
use std::collections::BTreeMap;
@@ -79,7 +81,7 @@ fn visit_implementation_of_copy(tcx: TyCtxt<'_>, impl_did: LocalDefId) {
7981
};
8082

8183
let cause = traits::ObligationCause::misc(span, impl_hir_id);
82-
match can_type_implement_copy(tcx, param_env, self_type, cause) {
84+
match type_allowed_to_implement_copy(tcx, param_env, self_type, cause) {
8385
Ok(()) => {}
8486
Err(CopyImplementationError::InfrigingFields(fields)) => {
8587
let mut err = struct_span_err!(
@@ -94,50 +96,70 @@ fn visit_implementation_of_copy(tcx: TyCtxt<'_>, impl_did: LocalDefId) {
9496
let mut errors: BTreeMap<_, Vec<_>> = Default::default();
9597
let mut bounds = vec![];
9698

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

‎compiler/rustc_lint/src/builtin.rs

+10-8
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol};
7272
use rustc_span::{BytePos, InnerSpan, Span};
7373
use rustc_target::abi::{Abi, VariantIdx};
7474
use rustc_trait_selection::infer::{InferCtxtExt, TyCtxtInferExt};
75-
use rustc_trait_selection::traits::{self, misc::can_type_implement_copy, EvaluationResult};
75+
use rustc_trait_selection::traits::{self, misc::type_allowed_to_implement_copy};
7676

7777
use crate::nonstandard_style::{method_context, MethodLateContext};
7878

@@ -709,12 +709,14 @@ impl<'tcx> LateLintPass<'tcx> for MissingCopyImplementations {
709709

710710
// We shouldn't recommend implementing `Copy` on stateful things,
711711
// such as iterators.
712-
if let Some(iter_trait) = cx.tcx.get_diagnostic_item(sym::Iterator) {
713-
if cx.tcx.infer_ctxt().build().type_implements_trait(iter_trait, [ty], param_env)
714-
== EvaluationResult::EvaluatedToOk
715-
{
716-
return;
717-
}
712+
if let Some(iter_trait) = cx.tcx.get_diagnostic_item(sym::Iterator)
713+
&& cx.tcx
714+
.infer_ctxt()
715+
.build()
716+
.type_implements_trait(iter_trait, [ty], param_env)
717+
.must_apply_modulo_regions()
718+
{
719+
return;
718720
}
719721

720722
// Default value of clippy::trivially_copy_pass_by_ref
@@ -726,7 +728,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingCopyImplementations {
726728
}
727729
}
728730

729-
if can_type_implement_copy(
731+
if type_allowed_to_implement_copy(
730732
cx.tcx,
731733
param_env,
732734
ty,

‎compiler/rustc_trait_selection/src/traits/misc.rs

+70-23
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,36 @@
11
//! Miscellaneous type-system utilities that are too small to deserve their own modules.
22
3-
use crate::infer::InferCtxtExt as _;
43
use crate::traits::{self, ObligationCause};
54

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

10-
use crate::traits::error_reporting::TypeErrCtxtExt;
11+
use super::outlives_bounds::InferCtxtExt;
1112

12-
#[derive(Clone)]
1313
pub enum CopyImplementationError<'tcx> {
14-
InfrigingFields(Vec<(&'tcx ty::FieldDef, Ty<'tcx>)>),
14+
InfrigingFields(Vec<(&'tcx ty::FieldDef, Ty<'tcx>, InfringingFieldsReason<'tcx>)>),
1515
NotAnAdt,
1616
HasDestructor,
1717
}
1818

19-
pub fn can_type_implement_copy<'tcx>(
19+
pub enum InfringingFieldsReason<'tcx> {
20+
Fulfill(Vec<FulfillmentError<'tcx>>),
21+
Regions(Vec<RegionResolutionError<'tcx>>),
22+
}
23+
24+
/// Checks that the fields of the type (an ADT) all implement copy.
25+
///
26+
/// If fields don't implement copy, return an error containing a list of
27+
/// those violating fields. If it's not an ADT, returns `Err(NotAnAdt)`.
28+
pub fn type_allowed_to_implement_copy<'tcx>(
2029
tcx: TyCtxt<'tcx>,
2130
param_env: ty::ParamEnv<'tcx>,
2231
self_type: Ty<'tcx>,
2332
parent_cause: ObligationCause<'tcx>,
2433
) -> Result<(), CopyImplementationError<'tcx>> {
25-
// FIXME: (@jroesch) float this code up
26-
let infcx = tcx.infer_ctxt().build();
2734
let (adt, substs) = match self_type.kind() {
2835
// These types used to have a builtin impl.
2936
// Now libcore provides that impl.
@@ -42,42 +49,82 @@ pub fn can_type_implement_copy<'tcx>(
4249
_ => return Err(CopyImplementationError::NotAnAdt),
4350
};
4451

52+
let copy_def_id = tcx.require_lang_item(hir::LangItem::Copy, Some(parent_cause.span));
53+
4554
let mut infringing = Vec::new();
4655
for variant in adt.variants() {
4756
for field in &variant.fields {
48-
let ty = field.ty(tcx, substs);
49-
if ty.references_error() {
57+
// Do this per-field to get better error messages.
58+
let infcx = tcx.infer_ctxt().build();
59+
let ocx = traits::ObligationCtxt::new(&infcx);
60+
61+
let unnormalized_ty = field.ty(tcx, substs);
62+
if unnormalized_ty.references_error() {
5063
continue;
5164
}
52-
let span = tcx.def_span(field.did);
65+
66+
let field_span = tcx.def_span(field.did);
67+
let field_ty_span = match tcx.hir().get_if_local(field.did) {
68+
Some(hir::Node::Field(field_def)) => field_def.ty.span,
69+
_ => field_span,
70+
};
71+
5372
// FIXME(compiler-errors): This gives us better spans for bad
5473
// projection types like in issue-50480.
5574
// If the ADT has substs, point to the cause we are given.
5675
// If it does not, then this field probably doesn't normalize
5776
// to begin with, and point to the bad field's span instead.
58-
let cause = if field
77+
let normalization_cause = if field
5978
.ty(tcx, traits::InternalSubsts::identity_for_item(tcx, adt.did()))
6079
.has_non_region_param()
6180
{
6281
parent_cause.clone()
6382
} else {
64-
ObligationCause::dummy_with_span(span)
65-
};
66-
match traits::fully_normalize(&infcx, cause, param_env, ty) {
67-
Ok(ty) => {
68-
if !infcx.type_is_copy_modulo_regions(param_env, ty, span) {
69-
infringing.push((field, ty));
70-
}
71-
}
72-
Err(errors) => {
73-
infcx.err_ctxt().report_fulfillment_errors(&errors, None);
74-
}
83+
ObligationCause::dummy_with_span(field_ty_span)
7584
};
85+
let ty = ocx.normalize(&normalization_cause, param_env, unnormalized_ty);
86+
let normalization_errors = ocx.select_where_possible();
87+
if !normalization_errors.is_empty() {
88+
tcx.sess.delay_span_bug(field_span, format!("couldn't normalize struct field `{unnormalized_ty}` when checking Copy implementation"));
89+
continue;
90+
}
91+
92+
ocx.register_bound(
93+
ObligationCause::dummy_with_span(field_ty_span),
94+
param_env,
95+
ty,
96+
copy_def_id,
97+
);
98+
let errors = ocx.select_all_or_error();
99+
if !errors.is_empty() {
100+
infringing.push((field, ty, InfringingFieldsReason::Fulfill(errors)));
101+
}
102+
103+
// Check regions assuming the self type of the impl is WF
104+
let outlives_env = OutlivesEnvironment::with_bounds(
105+
param_env,
106+
Some(&infcx),
107+
infcx.implied_bounds_tys(
108+
param_env,
109+
parent_cause.body_id,
110+
FxIndexSet::from_iter([self_type]),
111+
),
112+
);
113+
infcx.process_registered_region_obligations(
114+
outlives_env.region_bound_pairs(),
115+
param_env,
116+
);
117+
let errors = infcx.resolve_regions(&outlives_env);
118+
if !errors.is_empty() {
119+
infringing.push((field, ty, InfringingFieldsReason::Regions(errors)));
120+
}
76121
}
77122
}
123+
78124
if !infringing.is_empty() {
79125
return Err(CopyImplementationError::InfrigingFields(infringing));
80126
}
127+
81128
if adt.has_dtor(tcx) {
82129
return Err(CopyImplementationError::HasDestructor);
83130
}

‎src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use rustc_span::symbol::kw;
2424
use rustc_span::{sym, Span};
2525
use rustc_target::spec::abi::Abi;
2626
use rustc_trait_selection::traits;
27-
use rustc_trait_selection::traits::misc::can_type_implement_copy;
27+
use rustc_trait_selection::traits::misc::type_allowed_to_implement_copy;
2828
use std::borrow::Cow;
2929

3030
declare_clippy_lint! {
@@ -200,7 +200,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue {
200200
let sugg = |diag: &mut Diagnostic| {
201201
if let ty::Adt(def, ..) = ty.kind() {
202202
if let Some(span) = cx.tcx.hir().span_if_local(def.did()) {
203-
if can_type_implement_copy(
203+
if type_allowed_to_implement_copy(
204204
cx.tcx,
205205
cx.param_env,
206206
ty,

‎tests/ui/traits/copy-impl-cannot-normalize.stderr

+10
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,16 @@ error[E0277]: the trait bound `T: TraitFoo` is not satisfied
44
LL | impl<T> Copy for Foo<T> {}
55
| ^^^^^^ the trait `TraitFoo` is not implemented for `T`
66
|
7+
note: required for `Foo<T>` to implement `Clone`
8+
--> $DIR/copy-impl-cannot-normalize.rs:12:9
9+
|
10+
LL | impl<T> Clone for Foo<T>
11+
| ^^^^^ ^^^^^^
12+
LL | where
13+
LL | T: TraitFoo,
14+
| -------- unsatisfied trait bound introduced here
15+
note: required by a bound in `Copy`
16+
--> $SRC_DIR/core/src/marker.rs:LL:COL
717
help: consider restricting type parameter `T`
818
|
919
LL | impl<T: TraitFoo> Copy for Foo<T> {}

‎tests/ui/traits/copy-is-not-modulo-regions.not_static.stderr

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
error[E0204]: the trait `Copy` may not be implemented for this type
2+
--> $DIR/copy-is-not-modulo-regions.rs:13:21
3+
|
4+
LL | struct Bar<'lt>(Foo<'lt>);
5+
| -------- this field does not implement `Copy`
6+
...
7+
LL | impl<'any> Copy for Bar<'any> {}
8+
| ^^^^^^^^^
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+
| +++++++++
19+
20+
error: aborting due to previous error
21+
22+
For more information about this error, try `rustc --explain E0204`.

‎tests/ui/traits/copy-is-not-modulo-regions.rs

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// revisions: not_static yes_static
2+
//[yes_static] check-pass
3+
4+
#[derive(Clone)]
5+
struct Foo<'lt>(&'lt ());
6+
7+
impl Copy for Foo<'static> {}
8+
9+
#[derive(Clone)]
10+
struct Bar<'lt>(Foo<'lt>);
11+
12+
#[cfg(not_static)]
13+
impl<'any> Copy for Bar<'any> {}
14+
//[not_static]~^ the trait `Copy` may not be implemented for this type
15+
16+
#[cfg(yes_static)]
17+
impl<'any> Copy for Bar<'static> {}
18+
19+
fn main() {}

0 commit comments

Comments
 (0)