From 8cf7f40a895f476ecd3216b11ff673389135b652 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 30 Nov 2022 20:41:02 +0000 Subject: [PATCH 1/5] Check ADT fields for copy implementations considering regions --- .../src/coherence/builtin.rs | 6 ++- compiler/rustc_lint/src/builtin.rs | 18 ++++--- .../rustc_trait_selection/src/traits/misc.rs | 50 +++++++++++++------ ...py-is-not-modulo-regions.not_static.stderr | 12 +++++ .../ui/traits/copy-is-not-modulo-regions.rs | 19 +++++++ .../src/needless_pass_by_value.rs | 4 +- 6 files changed, 83 insertions(+), 26 deletions(-) create mode 100644 src/test/ui/traits/copy-is-not-modulo-regions.not_static.stderr create mode 100644 src/test/ui/traits/copy-is-not-modulo-regions.rs diff --git a/compiler/rustc_hir_analysis/src/coherence/builtin.rs b/compiler/rustc_hir_analysis/src/coherence/builtin.rs index 2e2c1591e9b44..0926d5ccf2d57 100644 --- a/compiler/rustc_hir_analysis/src/coherence/builtin.rs +++ b/compiler/rustc_hir_analysis/src/coherence/builtin.rs @@ -13,7 +13,9 @@ use rustc_infer::infer::TyCtxtInferExt; 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, +}; use rustc_trait_selection::traits::predicate_for_trait_def; use rustc_trait_selection::traits::{self, ObligationCause}; use std::collections::BTreeMap; @@ -82,7 +84,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!( diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 6f445426df70e..fe188162cf85b 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 b6ded4ce5a396..b87412f7de160 100644 --- a/compiler/rustc_trait_selection/src/traits/misc.rs +++ b/compiler/rustc_trait_selection/src/traits/misc.rs @@ -1,9 +1,9 @@ //! 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_hir as hir; +use rustc_infer::infer::outlives::env::OutlivesEnvironment; use rustc_infer::infer::TyCtxtInferExt; use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitable}; @@ -16,14 +16,16 @@ pub enum CopyImplementationError<'tcx> { HasDestructor, } -pub fn can_type_implement_copy<'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,9 +44,14 @@ 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 { + // Do this per-field to get better error messages. + let infcx = tcx.infer_ctxt().build(); + let ocx = traits::ObligationCtxt::new(&infcx); + let ty = field.ty(tcx, substs); if ty.references_error() { continue; @@ -63,21 +70,36 @@ pub fn can_type_implement_copy<'tcx>( } 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); - } - }; + + let ty = ocx.normalize(&cause, param_env, ty); + let normalization_errors = ocx.select_where_possible(); + if !normalization_errors.is_empty() { + // Don't report this as a field that doesn't implement Copy, + // but instead just implement this as a field that isn't WF. + infcx.err_ctxt().report_fulfillment_errors(&normalization_errors, None); + continue; + } + + ocx.register_bound(cause, param_env, ty, copy_def_id); + if !ocx.select_all_or_error().is_empty() { + infringing.push((field, ty)); + } + + let outlives_env = OutlivesEnvironment::new(param_env); + infcx.process_registered_region_obligations( + outlives_env.region_bound_pairs(), + param_env, + ); + if !infcx.resolve_regions(&outlives_env).is_empty() { + infringing.push((field, ty)); + } } } + if !infringing.is_empty() { return Err(CopyImplementationError::InfrigingFields(infringing)); } + if adt.has_dtor(tcx) { return Err(CopyImplementationError::HasDestructor); } diff --git a/src/test/ui/traits/copy-is-not-modulo-regions.not_static.stderr b/src/test/ui/traits/copy-is-not-modulo-regions.not_static.stderr new file mode 100644 index 0000000000000..1c4d2136677d0 --- /dev/null +++ b/src/test/ui/traits/copy-is-not-modulo-regions.not_static.stderr @@ -0,0 +1,12 @@ +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> {} + | ^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0204`. diff --git a/src/test/ui/traits/copy-is-not-modulo-regions.rs b/src/test/ui/traits/copy-is-not-modulo-regions.rs new file mode 100644 index 0000000000000..adb8702376977 --- /dev/null +++ b/src/test/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/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs index 1249db5dc4792..8c9d4c5cfe66f 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, From 333c6bf523019fd1565a5236d3c727172ec844f2 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 9 Dec 2022 19:58:46 +0000 Subject: [PATCH 2/5] copy self type is implied wf --- compiler/rustc_trait_selection/src/traits/misc.rs | 15 ++++++++++++++- src/test/ui/traits/copy-requires-self-wf.rs | 14 ++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/traits/copy-requires-self-wf.rs diff --git a/compiler/rustc_trait_selection/src/traits/misc.rs b/compiler/rustc_trait_selection/src/traits/misc.rs index b87412f7de160..46eea628a3441 100644 --- a/compiler/rustc_trait_selection/src/traits/misc.rs +++ b/compiler/rustc_trait_selection/src/traits/misc.rs @@ -2,6 +2,7 @@ use crate::traits::{self, ObligationCause}; +use rustc_data_structures::fx::FxIndexSet; use rustc_hir as hir; use rustc_infer::infer::outlives::env::OutlivesEnvironment; use rustc_infer::infer::TyCtxtInferExt; @@ -9,6 +10,8 @@ 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>)>), @@ -45,6 +48,7 @@ pub fn type_allowed_to_implement_copy<'tcx>( }; 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 { @@ -85,7 +89,16 @@ pub fn type_allowed_to_implement_copy<'tcx>( infringing.push((field, ty)); } - let outlives_env = OutlivesEnvironment::new(param_env); + // 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, diff --git a/src/test/ui/traits/copy-requires-self-wf.rs b/src/test/ui/traits/copy-requires-self-wf.rs new file mode 100644 index 0000000000000..9abfdfab9d06d --- /dev/null +++ b/src/test/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 Copy for B<'_, T> {} + +fn main() {} From 16cfadbfe835cadd24dc65481ab3ad0b5b627c5a Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 9 Dec 2022 20:59:26 +0000 Subject: [PATCH 3/5] Suggest lifetime bound in illegal Copy impl --- .../src/coherence/builtin.rs | 106 +++++++++++------- .../rustc_trait_selection/src/traits/misc.rs | 44 +++++--- ...py-is-not-modulo-regions.not_static.stderr | 10 ++ 3 files changed, 103 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/coherence/builtin.rs b/compiler/rustc_hir_analysis/src/coherence/builtin.rs index 0926d5ccf2d57..e642572e3dab2 100644 --- a/compiler/rustc_hir_analysis/src/coherence/builtin.rs +++ b/compiler/rustc_hir_analysis/src/coherence/builtin.rs @@ -7,14 +7,14 @@ 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::{ - type_allowed_to_implement_copy, CopyImplementationError, + type_allowed_to_implement_copy, CopyImplementationError, InfringingFieldsReason, }; use rustc_trait_selection::traits::predicate_for_trait_def; use rustc_trait_selection::traits::{self, ObligationCause}; @@ -99,50 +99,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_trait_selection/src/traits/misc.rs b/compiler/rustc_trait_selection/src/traits/misc.rs index 46eea628a3441..0de44dba0ddd0 100644 --- a/compiler/rustc_trait_selection/src/traits/misc.rs +++ b/compiler/rustc_trait_selection/src/traits/misc.rs @@ -4,21 +4,25 @@ use crate::traits::{self, ObligationCause}; use rustc_data_structures::fx::FxIndexSet; use rustc_hir as hir; -use rustc_infer::infer::outlives::env::OutlivesEnvironment; -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 enum InfringingFieldsReason<'tcx> { + Fulfill(Vec>), + Regions(Vec>), +} + /// 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 @@ -60,22 +64,27 @@ pub fn type_allowed_to_implement_copy<'tcx>( if 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) + ObligationCause::dummy_with_span(field_ty_span) }; - - let ty = ocx.normalize(&cause, param_env, ty); + let ty = ocx.normalize(&normalization_cause, param_env, ty); let normalization_errors = ocx.select_where_possible(); if !normalization_errors.is_empty() { // Don't report this as a field that doesn't implement Copy, @@ -84,9 +93,15 @@ pub fn type_allowed_to_implement_copy<'tcx>( continue; } - ocx.register_bound(cause, param_env, ty, copy_def_id); - if !ocx.select_all_or_error().is_empty() { - infringing.push((field, ty)); + 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 @@ -103,8 +118,9 @@ pub fn type_allowed_to_implement_copy<'tcx>( outlives_env.region_bound_pairs(), param_env, ); - if !infcx.resolve_regions(&outlives_env).is_empty() { - infringing.push((field, ty)); + let errors = infcx.resolve_regions(&outlives_env); + if !errors.is_empty() { + infringing.push((field, ty, InfringingFieldsReason::Regions(errors))); } } } diff --git a/src/test/ui/traits/copy-is-not-modulo-regions.not_static.stderr b/src/test/ui/traits/copy-is-not-modulo-regions.not_static.stderr index 1c4d2136677d0..edd94d2010b96 100644 --- a/src/test/ui/traits/copy-is-not-modulo-regions.not_static.stderr +++ b/src/test/ui/traits/copy-is-not-modulo-regions.not_static.stderr @@ -6,6 +6,16 @@ LL | struct Bar<'lt>(Foo<'lt>); ... 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 From 6ec8c13e15824d5b0dbdca5ab404d15af3e68f48 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 11 Jan 2023 20:16:12 +0000 Subject: [PATCH 4/5] Rebase and move UI tests --- .../ui/traits/copy-is-not-modulo-regions.not_static.stderr | 0 {src/test => tests}/ui/traits/copy-is-not-modulo-regions.rs | 0 {src/test => tests}/ui/traits/copy-requires-self-wf.rs | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename {src/test => tests}/ui/traits/copy-is-not-modulo-regions.not_static.stderr (100%) rename {src/test => tests}/ui/traits/copy-is-not-modulo-regions.rs (100%) rename {src/test => tests}/ui/traits/copy-requires-self-wf.rs (100%) diff --git a/src/test/ui/traits/copy-is-not-modulo-regions.not_static.stderr b/tests/ui/traits/copy-is-not-modulo-regions.not_static.stderr similarity index 100% rename from src/test/ui/traits/copy-is-not-modulo-regions.not_static.stderr rename to tests/ui/traits/copy-is-not-modulo-regions.not_static.stderr diff --git a/src/test/ui/traits/copy-is-not-modulo-regions.rs b/tests/ui/traits/copy-is-not-modulo-regions.rs similarity index 100% rename from src/test/ui/traits/copy-is-not-modulo-regions.rs rename to tests/ui/traits/copy-is-not-modulo-regions.rs diff --git a/src/test/ui/traits/copy-requires-self-wf.rs b/tests/ui/traits/copy-requires-self-wf.rs similarity index 100% rename from src/test/ui/traits/copy-requires-self-wf.rs rename to tests/ui/traits/copy-requires-self-wf.rs From 75074e0e528cf8a50310bc0de19f73b60e8c8304 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 11 Jan 2023 20:26:12 +0000 Subject: [PATCH 5/5] Delay normalization bugs instead of reporting them --- .../rustc_trait_selection/src/traits/misc.rs | 12 +++----- .../traits/copy-impl-cannot-normalize.stderr | 10 +++++++ tests/ui/traits/issue-50480.rs | 2 -- tests/ui/traits/issue-50480.stderr | 28 ++++--------------- 4 files changed, 19 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/misc.rs b/compiler/rustc_trait_selection/src/traits/misc.rs index 0de44dba0ddd0..a41a601f2db07 100644 --- a/compiler/rustc_trait_selection/src/traits/misc.rs +++ b/compiler/rustc_trait_selection/src/traits/misc.rs @@ -8,8 +8,6 @@ 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; pub enum CopyImplementationError<'tcx> { @@ -60,8 +58,8 @@ pub fn type_allowed_to_implement_copy<'tcx>( let infcx = tcx.infer_ctxt().build(); let ocx = traits::ObligationCtxt::new(&infcx); - let ty = field.ty(tcx, substs); - if ty.references_error() { + let unnormalized_ty = field.ty(tcx, substs); + if unnormalized_ty.references_error() { continue; } @@ -84,12 +82,10 @@ pub fn type_allowed_to_implement_copy<'tcx>( } else { ObligationCause::dummy_with_span(field_ty_span) }; - let ty = ocx.normalize(&normalization_cause, param_env, ty); + let ty = ocx.normalize(&normalization_cause, param_env, unnormalized_ty); let normalization_errors = ocx.select_where_possible(); if !normalization_errors.is_empty() { - // Don't report this as a field that doesn't implement Copy, - // but instead just implement this as a field that isn't WF. - infcx.err_ctxt().report_fulfillment_errors(&normalization_errors, None); + tcx.sess.delay_span_bug(field_span, format!("couldn't normalize struct field `{unnormalized_ty}` when checking Copy implementation")); continue; } diff --git a/tests/ui/traits/copy-impl-cannot-normalize.stderr b/tests/ui/traits/copy-impl-cannot-normalize.stderr index 68b95b42b3463..86c511c089567 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 Copy for Foo {} | ^^^^^^ the trait `TraitFoo` is not implemented for `T` | +note: required for `Foo` to implement `Clone` + --> $DIR/copy-impl-cannot-normalize.rs:12:9 + | +LL | impl Clone for Foo + | ^^^^^ ^^^^^^ +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 Copy for Foo {} diff --git a/tests/ui/traits/issue-50480.rs b/tests/ui/traits/issue-50480.rs index 10597caf5b2dc..005939e0c46e4 100644 --- a/tests/ui/traits/issue-50480.rs +++ b/tests/ui/traits/issue-50480.rs @@ -5,13 +5,11 @@ struct Foo(N, NotDefined, ::Item, Vec, 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, N, NotDefined, ::Item, Vec, 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 aa8384e980539..5063fdca09273 100644 --- a/tests/ui/traits/issue-50480.stderr +++ b/tests/ui/traits/issue-50480.stderr @@ -38,7 +38,7 @@ LL | struct Foo(N, NotDefined, ::Item, Vec, 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, N, NotDefined, ::Item, Vec, String); | - ^ @@ -55,20 +55,11 @@ LL | struct Bar(T, N, NotDefined, ::Item, Vec, 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, N, NotDefined, ::Item, Vec, String); | ^^^^^^^^^^ not found in this scope -error[E0277]: `i32` is not an iterator - --> $DIR/issue-50480.rs:3:27 - | -LL | struct Foo(N, NotDefined, ::Item, Vec, 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, ::Item, Vec, 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, N, NotDefined, ::Item, Vec, 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, N, NotDefined, ::Item, Vec, 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`.