From 604ffab063e0b488f43b6f8faf60421a4533dbc0 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 5 Jun 2023 14:22:45 +0000 Subject: [PATCH 1/2] Avoid going through queries if a value of type `AssocItem` is already available --- compiler/rustc_hir_analysis/src/astconv/mod.rs | 2 +- compiler/rustc_hir_analysis/src/check/compare_impl_item.rs | 4 ++-- compiler/rustc_hir_analysis/src/check/mod.rs | 2 +- compiler/rustc_trait_selection/src/traits/object_safety.rs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index 3d78ea9aa9b..9e78d9858a6 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -1601,7 +1601,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { tcx.associated_items(pred.def_id()) .in_definition_order() .filter(|item| item.kind == ty::AssocKind::Type) - .filter(|item| tcx.opt_rpitit_info(item.def_id).is_none()) + .filter(|item| item.opt_rpitit_info.is_none()) .map(|item| item.def_id), ); } diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index 31b89525f15..d95862420da 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -1216,7 +1216,7 @@ fn compare_number_of_generics<'tcx>( // has mismatched type or const generic arguments, then the method that it's // inheriting the generics from will also have mismatched arguments, and // we'll report an error for that instead. Delay a bug for safety, though. - if tcx.opt_rpitit_info(trait_.def_id).is_some() { + if trait_.opt_rpitit_info.is_some() { return Err(tcx.sess.delay_span_bug( rustc_span::DUMMY_SP, "errors comparing numbers of generics of trait/impl functions were not emitted", @@ -2006,7 +2006,7 @@ pub(super) fn check_type_bounds<'tcx>( // A synthetic impl Trait for RPITIT desugaring has no HIR, which we currently use to get the // span for an impl's associated type. Instead, for these, use the def_span for the synthesized // associated type. - let impl_ty_span = if tcx.opt_rpitit_info(impl_ty.def_id).is_some() { + let impl_ty_span = if impl_ty.opt_rpitit_info.is_some() { tcx.def_span(impl_ty_def_id) } else { match tcx.hir().get_by_def_id(impl_ty_def_id) { diff --git a/compiler/rustc_hir_analysis/src/check/mod.rs b/compiler/rustc_hir_analysis/src/check/mod.rs index 3971a4c01d6..c9e74896ac0 100644 --- a/compiler/rustc_hir_analysis/src/check/mod.rs +++ b/compiler/rustc_hir_analysis/src/check/mod.rs @@ -188,7 +188,7 @@ fn missing_items_err( full_impl_span: Span, ) { let missing_items = - missing_items.iter().filter(|trait_item| tcx.opt_rpitit_info(trait_item.def_id).is_none()); + missing_items.iter().filter(|trait_item| trait_item.opt_rpitit_info.is_none()); let missing_items_msg = missing_items .clone() diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index 048302187cf..e61cc19354e 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs @@ -161,7 +161,7 @@ fn object_safety_violations_for_trait( .in_definition_order() .filter(|item| item.kind == ty::AssocKind::Type) .filter(|item| !tcx.generics_of(item.def_id).params.is_empty()) - .filter(|item| tcx.opt_rpitit_info(item.def_id).is_none()) + .filter(|item| item.opt_rpitit_info.is_none()) .map(|item| { let ident = item.ident(tcx); ObjectSafetyViolation::GAT(ident.name, ident.span) From 58972d19e7ba591321d6e67d32080a5e5a78e19a Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 5 Jun 2023 15:32:23 +0000 Subject: [PATCH 2/2] Merge method, type and const object safety checks --- .../rustc_hir_analysis/src/astconv/mod.rs | 4 + .../src/traits/object_safety.rs | 94 +++++++++---------- 2 files changed, 47 insertions(+), 51 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index 9e78d9858a6..95517f01414 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -1643,6 +1643,10 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { } } + // `dyn Trait` desugars to (not Rust syntax) `dyn Trait where ::Assoc = Foo`. + // So every `Projection` clause is an `Assoc = Foo` bound. `associated_types` contains all associated + // types's `DefId`, so the following loop removes all the `DefIds` of the associated types that have a + // corresponding `Projection` clause for (projection_bound, _) in &projection_bounds { for def_ids in associated_types.values_mut() { def_ids.remove(&projection_bound.projection_def_id()); diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index e61cc19354e..b2771915eef 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs @@ -115,15 +115,11 @@ fn object_safety_violations_for_trait( tcx: TyCtxt<'_>, trait_def_id: DefId, ) -> Vec { - // Check methods for violations. + // Check assoc items for violations. let mut violations: Vec<_> = tcx .associated_items(trait_def_id) .in_definition_order() - .filter(|item| item.kind == ty::AssocKind::Fn) - .filter_map(|&item| { - object_safety_violation_for_method(tcx, trait_def_id, item) - .map(|(code, span)| ObjectSafetyViolation::Method(item.name, code, span)) - }) + .filter_map(|&item| object_safety_violation_for_assoc_item(tcx, trait_def_id, item)) .collect(); // Check the trait itself. @@ -145,30 +141,6 @@ fn object_safety_violations_for_trait( violations.push(ObjectSafetyViolation::SupertraitNonLifetimeBinder(spans)); } - violations.extend( - tcx.associated_items(trait_def_id) - .in_definition_order() - .filter(|item| item.kind == ty::AssocKind::Const) - .map(|item| { - let ident = item.ident(tcx); - ObjectSafetyViolation::AssocConst(ident.name, ident.span) - }), - ); - - if !tcx.features().generic_associated_types_extended { - violations.extend( - tcx.associated_items(trait_def_id) - .in_definition_order() - .filter(|item| item.kind == ty::AssocKind::Type) - .filter(|item| !tcx.generics_of(item.def_id).params.is_empty()) - .filter(|item| item.opt_rpitit_info.is_none()) - .map(|item| { - let ident = item.ident(tcx); - ObjectSafetyViolation::GAT(ident.name, ident.span) - }), - ); - } - debug!( "object_safety_violations_for_trait(trait_def_id={:?}) = {:?}", trait_def_id, violations @@ -401,34 +373,54 @@ fn generics_require_sized_self(tcx: TyCtxt<'_>, def_id: DefId) -> bool { }) } -/// Returns `Some(_)` if this method makes the containing trait not object safe. -fn object_safety_violation_for_method( +/// Returns `Some(_)` if this item makes the containing trait not object safe. +#[instrument(level = "debug", skip(tcx), ret)] +fn object_safety_violation_for_assoc_item( tcx: TyCtxt<'_>, trait_def_id: DefId, - method: ty::AssocItem, -) -> Option<(MethodViolationCode, Span)> { - debug!("object_safety_violation_for_method({:?}, {:?})", trait_def_id, method); - // Any method that has a `Self : Sized` requisite is otherwise + item: ty::AssocItem, +) -> Option { + // Any item that has a `Self : Sized` requisite is otherwise // exempt from the regulations. - if generics_require_sized_self(tcx, method.def_id) { + if generics_require_sized_self(tcx, item.def_id) { return None; } - let violation = virtual_call_violation_for_method(tcx, trait_def_id, method); - // Get an accurate span depending on the violation. - violation.map(|v| { - let node = tcx.hir().get_if_local(method.def_id); - let span = match (&v, node) { - (MethodViolationCode::ReferencesSelfInput(Some(span)), _) => *span, - (MethodViolationCode::UndispatchableReceiver(Some(span)), _) => *span, - (MethodViolationCode::ReferencesImplTraitInTrait(span), _) => *span, - (MethodViolationCode::ReferencesSelfOutput, Some(node)) => { - node.fn_decl().map_or(method.ident(tcx).span, |decl| decl.output.span()) + match item.kind { + // Associated consts are never object safe, as they can't have `where` bounds yet at all, + // and associated const bounds in trait objects aren't a thing yet either. + ty::AssocKind::Const => { + Some(ObjectSafetyViolation::AssocConst(item.name, item.ident(tcx).span)) + } + ty::AssocKind::Fn => virtual_call_violation_for_method(tcx, trait_def_id, item).map(|v| { + let node = tcx.hir().get_if_local(item.def_id); + // Get an accurate span depending on the violation. + let span = match (&v, node) { + (MethodViolationCode::ReferencesSelfInput(Some(span)), _) => *span, + (MethodViolationCode::UndispatchableReceiver(Some(span)), _) => *span, + (MethodViolationCode::ReferencesImplTraitInTrait(span), _) => *span, + (MethodViolationCode::ReferencesSelfOutput, Some(node)) => { + node.fn_decl().map_or(item.ident(tcx).span, |decl| decl.output.span()) + } + _ => item.ident(tcx).span, + }; + + ObjectSafetyViolation::Method(item.name, v, span) + }), + // Associated types can only be object safe if they have `Self: Sized` bounds. + ty::AssocKind::Type => { + if !tcx.features().generic_associated_types_extended + && !tcx.generics_of(item.def_id).params.is_empty() + && item.opt_rpitit_info.is_none() + { + Some(ObjectSafetyViolation::GAT(item.name, item.ident(tcx).span)) + } else { + // We will permit associated types if they are explicitly mentioned in the trait object. + // We can't check this here, as here we only check if it is guaranteed to not be possible. + None } - _ => method.ident(tcx).span, - }; - (v, span) - }) + } + } } /// Returns `Some(_)` if this method cannot be called on a trait