diff options
| -rw-r--r-- | compiler/rustc_hir_analysis/src/coherence/builtin.rs | 110 | ||||
| -rw-r--r-- | compiler/rustc_lint/src/builtin.rs | 18 | ||||
| -rw-r--r-- | compiler/rustc_trait_selection/src/traits/misc.rs | 93 | ||||
| -rw-r--r-- | src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs | 4 | ||||
| -rw-r--r-- | tests/ui/traits/copy-impl-cannot-normalize.stderr | 10 | ||||
| -rw-r--r-- | tests/ui/traits/copy-is-not-modulo-regions.not_static.stderr | 22 | ||||
| -rw-r--r-- | tests/ui/traits/copy-is-not-modulo-regions.rs | 19 | ||||
| -rw-r--r-- | tests/ui/traits/copy-requires-self-wf.rs | 14 | ||||
| -rw-r--r-- | tests/ui/traits/issue-50480.rs | 2 | ||||
| -rw-r--r-- | tests/ui/traits/issue-50480.stderr | 28 |
10 files changed, 218 insertions, 102 deletions
diff --git a/compiler/rustc_hir_analysis/src/coherence/builtin.rs b/compiler/rustc_hir_analysis/src/coherence/builtin.rs index 55de68f83f2..28c04087868 100644 --- a/compiler/rustc_hir_analysis/src/coherence/builtin.rs +++ b/compiler/rustc_hir_analysis/src/coherence/builtin.rs @@ -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, + } } } } diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 6f445426df7..fe188162cf8 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -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, diff --git a/compiler/rustc_trait_selection/src/traits/misc.rs b/compiler/rustc_trait_selection/src/traits/misc.rs index b6ded4ce5a3..a41a601f2db 100644 --- a/compiler/rustc_trait_selection/src/traits/misc.rs +++ b/compiler/rustc_trait_selection/src/traits/misc.rs @@ -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); } diff --git a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs index 1249db5dc47..8c9d4c5cfe6 100644 --- a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs +++ b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs @@ -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, diff --git a/tests/ui/traits/copy-impl-cannot-normalize.stderr b/tests/ui/traits/copy-impl-cannot-normalize.stderr index 68b95b42b34..86c511c0895 100644 --- a/tests/ui/traits/copy-impl-cannot-normalize.stderr +++ b/tests/ui/traits/copy-impl-cannot-normalize.stderr @@ -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> {} diff --git a/tests/ui/traits/copy-is-not-modulo-regions.not_static.stderr b/tests/ui/traits/copy-is-not-modulo-regions.not_static.stderr new file mode 100644 index 00000000000..edd94d2010b --- /dev/null +++ b/tests/ui/traits/copy-is-not-modulo-regions.not_static.stderr @@ -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`. diff --git a/tests/ui/traits/copy-is-not-modulo-regions.rs b/tests/ui/traits/copy-is-not-modulo-regions.rs new file mode 100644 index 00000000000..adb87023769 --- /dev/null +++ b/tests/ui/traits/copy-is-not-modulo-regions.rs @@ -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() {} diff --git a/tests/ui/traits/copy-requires-self-wf.rs b/tests/ui/traits/copy-requires-self-wf.rs new file mode 100644 index 00000000000..9abfdfab9d0 --- /dev/null +++ b/tests/ui/traits/copy-requires-self-wf.rs @@ -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() {} diff --git a/tests/ui/traits/issue-50480.rs b/tests/ui/traits/issue-50480.rs index 10597caf5b2..005939e0c46 100644 --- a/tests/ui/traits/issue-50480.rs +++ b/tests/ui/traits/issue-50480.rs @@ -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() {} diff --git a/tests/ui/traits/issue-50480.stderr b/tests/ui/traits/issue-50480.stderr index aa8384e9805..5063fdca092 100644 --- a/tests/ui/traits/issue-50480.stderr +++ b/tests/ui/traits/issue-50480.stderr @@ -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`. |