Auto merge of #105102 - compiler-errors:copy-impl-considering-regions, r=lcnr

Check ADT fields for copy implementations considering regions

Fixes #88901
r? `@ghost`
This commit is contained in:
bors 2023-01-20 21:29:52 +00:00
commit 94a300b9b8
10 changed files with 217 additions and 101 deletions

View File

@ -7,13 +7,15 @@ use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::lang_items::LangItem;
use rustc_hir::ItemKind;
use rustc_infer::infer;
use rustc_infer::infer::outlives::env::OutlivesEnvironment;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_infer::infer::{self, RegionResolutionError};
use rustc_middle::ty::adjustment::CoerceUnsizedInfo;
use rustc_middle::ty::{self, suggest_constraining_type_params, Ty, TyCtxt, TypeVisitable};
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt;
use rustc_trait_selection::traits::misc::{can_type_implement_copy, CopyImplementationError};
use rustc_trait_selection::traits::misc::{
type_allowed_to_implement_copy, CopyImplementationError, InfringingFieldsReason,
};
use rustc_trait_selection::traits::predicate_for_trait_def;
use rustc_trait_selection::traits::{self, ObligationCause};
use std::collections::BTreeMap;
@ -79,7 +81,7 @@ fn visit_implementation_of_copy(tcx: TyCtxt<'_>, impl_did: LocalDefId) {
};
let cause = traits::ObligationCause::misc(span, impl_hir_id);
match can_type_implement_copy(tcx, param_env, self_type, cause) {
match type_allowed_to_implement_copy(tcx, param_env, self_type, cause) {
Ok(()) => {}
Err(CopyImplementationError::InfrigingFields(fields)) => {
let mut err = struct_span_err!(
@ -94,50 +96,70 @@ fn visit_implementation_of_copy(tcx: TyCtxt<'_>, impl_did: LocalDefId) {
let mut errors: BTreeMap<_, Vec<_>> = Default::default();
let mut bounds = vec![];
for (field, ty) in fields {
for (field, ty, reason) in fields {
let field_span = tcx.def_span(field.did);
let field_ty_span = match tcx.hir().get_if_local(field.did) {
Some(hir::Node::Field(field_def)) => field_def.ty.span,
_ => field_span,
};
err.span_label(field_span, "this field does not implement `Copy`");
// Spin up a new FulfillmentContext, so we can get the _precise_ reason
// why this field does not implement Copy. This is useful because sometimes
// it is not immediately clear why Copy is not implemented for a field, since
// all we point at is the field itself.
let infcx = tcx.infer_ctxt().ignoring_regions().build();
for error in traits::fully_solve_bound(
&infcx,
traits::ObligationCause::dummy_with_span(field_ty_span),
param_env,
ty,
tcx.require_lang_item(LangItem::Copy, Some(span)),
) {
let error_predicate = error.obligation.predicate;
// Only note if it's not the root obligation, otherwise it's trivial and
// should be self-explanatory (i.e. a field literally doesn't implement Copy).
// FIXME: This error could be more descriptive, especially if the error_predicate
// contains a foreign type or if it's a deeply nested type...
if error_predicate != error.root_obligation.predicate {
errors
.entry((ty.to_string(), error_predicate.to_string()))
.or_default()
.push(error.obligation.cause.span);
match reason {
InfringingFieldsReason::Fulfill(fulfillment_errors) => {
for error in fulfillment_errors {
let error_predicate = error.obligation.predicate;
// Only note if it's not the root obligation, otherwise it's trivial and
// should be self-explanatory (i.e. a field literally doesn't implement Copy).
// FIXME: This error could be more descriptive, especially if the error_predicate
// contains a foreign type or if it's a deeply nested type...
if error_predicate != error.root_obligation.predicate {
errors
.entry((ty.to_string(), error_predicate.to_string()))
.or_default()
.push(error.obligation.cause.span);
}
if let ty::PredicateKind::Clause(ty::Clause::Trait(
ty::TraitPredicate {
trait_ref,
polarity: ty::ImplPolarity::Positive,
..
},
)) = error_predicate.kind().skip_binder()
{
let ty = trait_ref.self_ty();
if let ty::Param(_) = ty.kind() {
bounds.push((
format!("{ty}"),
trait_ref.print_only_trait_path().to_string(),
Some(trait_ref.def_id),
));
}
}
}
}
if let ty::PredicateKind::Clause(ty::Clause::Trait(ty::TraitPredicate {
trait_ref,
polarity: ty::ImplPolarity::Positive,
..
})) = error_predicate.kind().skip_binder()
{
let ty = trait_ref.self_ty();
if let ty::Param(_) = ty.kind() {
bounds.push((
format!("{ty}"),
trait_ref.print_only_trait_path().to_string(),
Some(trait_ref.def_id),
));
InfringingFieldsReason::Regions(region_errors) => {
for error in region_errors {
let ty = ty.to_string();
match error {
RegionResolutionError::ConcreteFailure(origin, a, b) => {
let predicate = format!("{b}: {a}");
errors
.entry((ty.clone(), predicate.clone()))
.or_default()
.push(origin.span());
if let ty::RegionKind::ReEarlyBound(ebr) = *b && ebr.has_name() {
bounds.push((b.to_string(), a.to_string(), None));
}
}
RegionResolutionError::GenericBoundFailure(origin, a, b) => {
let predicate = format!("{a}: {b}");
errors
.entry((ty.clone(), predicate.clone()))
.or_default()
.push(origin.span());
if let infer::region_constraints::GenericKind::Param(_) = a {
bounds.push((a.to_string(), b.to_string(), None));
}
}
_ => continue,
}
}
}
}

View File

@ -72,7 +72,7 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{BytePos, InnerSpan, Span};
use rustc_target::abi::{Abi, VariantIdx};
use rustc_trait_selection::infer::{InferCtxtExt, TyCtxtInferExt};
use rustc_trait_selection::traits::{self, misc::can_type_implement_copy, EvaluationResult};
use rustc_trait_selection::traits::{self, misc::type_allowed_to_implement_copy};
use crate::nonstandard_style::{method_context, MethodLateContext};
@ -709,12 +709,14 @@ impl<'tcx> LateLintPass<'tcx> for MissingCopyImplementations {
// We shouldn't recommend implementing `Copy` on stateful things,
// such as iterators.
if let Some(iter_trait) = cx.tcx.get_diagnostic_item(sym::Iterator) {
if cx.tcx.infer_ctxt().build().type_implements_trait(iter_trait, [ty], param_env)
== EvaluationResult::EvaluatedToOk
{
return;
}
if let Some(iter_trait) = cx.tcx.get_diagnostic_item(sym::Iterator)
&& cx.tcx
.infer_ctxt()
.build()
.type_implements_trait(iter_trait, [ty], param_env)
.must_apply_modulo_regions()
{
return;
}
// Default value of clippy::trivially_copy_pass_by_ref
@ -726,7 +728,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingCopyImplementations {
}
}
if can_type_implement_copy(
if type_allowed_to_implement_copy(
cx.tcx,
param_env,
ty,

View File

@ -1,29 +1,36 @@
//! Miscellaneous type-system utilities that are too small to deserve their own modules.
use crate::infer::InferCtxtExt as _;
use crate::traits::{self, ObligationCause};
use rustc_data_structures::fx::FxIndexSet;
use rustc_hir as hir;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_infer::infer::{RegionResolutionError, TyCtxtInferExt};
use rustc_infer::{infer::outlives::env::OutlivesEnvironment, traits::FulfillmentError};
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitable};
use crate::traits::error_reporting::TypeErrCtxtExt;
use super::outlives_bounds::InferCtxtExt;
#[derive(Clone)]
pub enum CopyImplementationError<'tcx> {
InfrigingFields(Vec<(&'tcx ty::FieldDef, Ty<'tcx>)>),
InfrigingFields(Vec<(&'tcx ty::FieldDef, Ty<'tcx>, InfringingFieldsReason<'tcx>)>),
NotAnAdt,
HasDestructor,
}
pub fn can_type_implement_copy<'tcx>(
pub enum InfringingFieldsReason<'tcx> {
Fulfill(Vec<FulfillmentError<'tcx>>),
Regions(Vec<RegionResolutionError<'tcx>>),
}
/// Checks that the fields of the type (an ADT) all implement copy.
///
/// If fields don't implement copy, return an error containing a list of
/// those violating fields. If it's not an ADT, returns `Err(NotAnAdt)`.
pub fn type_allowed_to_implement_copy<'tcx>(
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
self_type: Ty<'tcx>,
parent_cause: ObligationCause<'tcx>,
) -> Result<(), CopyImplementationError<'tcx>> {
// FIXME: (@jroesch) float this code up
let infcx = tcx.infer_ctxt().build();
let (adt, substs) = match self_type.kind() {
// These types used to have a builtin impl.
// Now libcore provides that impl.
@ -42,42 +49,82 @@ pub fn can_type_implement_copy<'tcx>(
_ => return Err(CopyImplementationError::NotAnAdt),
};
let copy_def_id = tcx.require_lang_item(hir::LangItem::Copy, Some(parent_cause.span));
let mut infringing = Vec::new();
for variant in adt.variants() {
for field in &variant.fields {
let ty = field.ty(tcx, substs);
if ty.references_error() {
// Do this per-field to get better error messages.
let infcx = tcx.infer_ctxt().build();
let ocx = traits::ObligationCtxt::new(&infcx);
let unnormalized_ty = field.ty(tcx, substs);
if unnormalized_ty.references_error() {
continue;
}
let span = tcx.def_span(field.did);
let field_span = tcx.def_span(field.did);
let field_ty_span = match tcx.hir().get_if_local(field.did) {
Some(hir::Node::Field(field_def)) => field_def.ty.span,
_ => field_span,
};
// FIXME(compiler-errors): This gives us better spans for bad
// projection types like in issue-50480.
// If the ADT has substs, point to the cause we are given.
// If it does not, then this field probably doesn't normalize
// to begin with, and point to the bad field's span instead.
let cause = if field
let normalization_cause = if field
.ty(tcx, traits::InternalSubsts::identity_for_item(tcx, adt.did()))
.has_non_region_param()
{
parent_cause.clone()
} else {
ObligationCause::dummy_with_span(span)
};
match traits::fully_normalize(&infcx, cause, param_env, ty) {
Ok(ty) => {
if !infcx.type_is_copy_modulo_regions(param_env, ty, span) {
infringing.push((field, ty));
}
}
Err(errors) => {
infcx.err_ctxt().report_fulfillment_errors(&errors, None);
}
ObligationCause::dummy_with_span(field_ty_span)
};
let ty = ocx.normalize(&normalization_cause, param_env, unnormalized_ty);
let normalization_errors = ocx.select_where_possible();
if !normalization_errors.is_empty() {
tcx.sess.delay_span_bug(field_span, format!("couldn't normalize struct field `{unnormalized_ty}` when checking Copy implementation"));
continue;
}
ocx.register_bound(
ObligationCause::dummy_with_span(field_ty_span),
param_env,
ty,
copy_def_id,
);
let errors = ocx.select_all_or_error();
if !errors.is_empty() {
infringing.push((field, ty, InfringingFieldsReason::Fulfill(errors)));
}
// Check regions assuming the self type of the impl is WF
let outlives_env = OutlivesEnvironment::with_bounds(
param_env,
Some(&infcx),
infcx.implied_bounds_tys(
param_env,
parent_cause.body_id,
FxIndexSet::from_iter([self_type]),
),
);
infcx.process_registered_region_obligations(
outlives_env.region_bound_pairs(),
param_env,
);
let errors = infcx.resolve_regions(&outlives_env);
if !errors.is_empty() {
infringing.push((field, ty, InfringingFieldsReason::Regions(errors)));
}
}
}
if !infringing.is_empty() {
return Err(CopyImplementationError::InfrigingFields(infringing));
}
if adt.has_dtor(tcx) {
return Err(CopyImplementationError::HasDestructor);
}

View File

@ -24,7 +24,7 @@ use rustc_span::symbol::kw;
use rustc_span::{sym, Span};
use rustc_target::spec::abi::Abi;
use rustc_trait_selection::traits;
use rustc_trait_selection::traits::misc::can_type_implement_copy;
use rustc_trait_selection::traits::misc::type_allowed_to_implement_copy;
use std::borrow::Cow;
declare_clippy_lint! {
@ -200,7 +200,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue {
let sugg = |diag: &mut Diagnostic| {
if let ty::Adt(def, ..) = ty.kind() {
if let Some(span) = cx.tcx.hir().span_if_local(def.did()) {
if can_type_implement_copy(
if type_allowed_to_implement_copy(
cx.tcx,
cx.param_env,
ty,

View File

@ -4,6 +4,16 @@ error[E0277]: the trait bound `T: TraitFoo` is not satisfied
LL | impl<T> Copy for Foo<T> {}
| ^^^^^^ the trait `TraitFoo` is not implemented for `T`
|
note: required for `Foo<T>` to implement `Clone`
--> $DIR/copy-impl-cannot-normalize.rs:12:9
|
LL | impl<T> Clone for Foo<T>
| ^^^^^ ^^^^^^
LL | where
LL | T: TraitFoo,
| -------- unsatisfied trait bound introduced here
note: required by a bound in `Copy`
--> $SRC_DIR/core/src/marker.rs:LL:COL
help: consider restricting type parameter `T`
|
LL | impl<T: TraitFoo> Copy for Foo<T> {}

View File

@ -0,0 +1,22 @@
error[E0204]: the trait `Copy` may not be implemented for this type
--> $DIR/copy-is-not-modulo-regions.rs:13:21
|
LL | struct Bar<'lt>(Foo<'lt>);
| -------- this field does not implement `Copy`
...
LL | impl<'any> Copy for Bar<'any> {}
| ^^^^^^^^^
|
note: the `Copy` impl for `Foo<'any>` requires that `'any: 'static`
--> $DIR/copy-is-not-modulo-regions.rs:10:17
|
LL | struct Bar<'lt>(Foo<'lt>);
| ^^^^^^^^
help: consider restricting type parameter `'any`
|
LL | impl<'any: 'static> Copy for Bar<'any> {}
| +++++++++
error: aborting due to previous error
For more information about this error, try `rustc --explain E0204`.

View File

@ -0,0 +1,19 @@
// revisions: not_static yes_static
//[yes_static] check-pass
#[derive(Clone)]
struct Foo<'lt>(&'lt ());
impl Copy for Foo<'static> {}
#[derive(Clone)]
struct Bar<'lt>(Foo<'lt>);
#[cfg(not_static)]
impl<'any> Copy for Bar<'any> {}
//[not_static]~^ the trait `Copy` may not be implemented for this type
#[cfg(yes_static)]
impl<'any> Copy for Bar<'static> {}
fn main() {}

View File

@ -0,0 +1,14 @@
// check-pass
#[derive(Clone)]
struct A<'a, T>(&'a T);
impl<'a, T: Copy + 'a> Copy for A<'a, T> {}
#[derive(Clone)]
struct B<'a, T>(A<'a, T>);
// `T: '_` should be implied by `WF(B<'_, T>)`.
impl<T: Copy> Copy for B<'_, T> {}
fn main() {}

View File

@ -5,13 +5,11 @@ struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
//~| ERROR cannot find type `NotDefined` in this scope
//~| ERROR cannot find type `N` in this scope
//~| ERROR cannot find type `N` in this scope
//~| ERROR `i32` is not an iterator
#[derive(Clone, Copy)]
//~^ ERROR the trait `Copy` may not be implemented for this type
struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
//~^ ERROR cannot find type `NotDefined` in this scope
//~| ERROR cannot find type `N` in this scope
//~| ERROR `i32` is not an iterator
fn main() {}

View File

@ -38,7 +38,7 @@ LL | struct Foo<NotDefined>(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, St
| ++++++++++++
error[E0412]: cannot find type `N` in this scope
--> $DIR/issue-50480.rs:12:18
--> $DIR/issue-50480.rs:11:18
|
LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| - ^
@ -55,20 +55,11 @@ LL | struct Bar<T, N>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, Strin
| +++
error[E0412]: cannot find type `NotDefined` in this scope
--> $DIR/issue-50480.rs:12:21
--> $DIR/issue-50480.rs:11:21
|
LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| ^^^^^^^^^^ not found in this scope
error[E0277]: `i32` is not an iterator
--> $DIR/issue-50480.rs:3:27
|
LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| ^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator
|
= help: the trait `Iterator` is not implemented for `i32`
= note: if you want to iterate between `start` until a value `end`, use the exclusive range syntax `start..end` or the inclusive range syntax `start..=end`
error[E0204]: the trait `Copy` may not be implemented for this type
--> $DIR/issue-50480.rs:1:17
|
@ -82,17 +73,8 @@ LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
|
= note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0277]: `i32` is not an iterator
--> $DIR/issue-50480.rs:12:33
|
LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| ^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator
|
= help: the trait `Iterator` is not implemented for `i32`
= note: if you want to iterate between `start` until a value `end`, use the exclusive range syntax `start..end` or the inclusive range syntax `start..=end`
error[E0204]: the trait `Copy` may not be implemented for this type
--> $DIR/issue-50480.rs:10:17
--> $DIR/issue-50480.rs:9:17
|
LL | #[derive(Clone, Copy)]
| ^^^^
@ -104,7 +86,7 @@ LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
|
= note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info)
error: aborting due to 10 previous errors
error: aborting due to 8 previous errors
Some errors have detailed explanations: E0204, E0277, E0412.
Some errors have detailed explanations: E0204, E0412.
For more information about an error, try `rustc --explain E0204`.