From 871eb6233ed4b5c3a3c5fe96e88e4dacc046e45f Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Wed, 1 Sep 2021 00:49:14 +0100 Subject: [PATCH 1/2] Stop allocating vtable entries for non-object-safe methods --- .../rustc_trait_selection/src/traits/mod.rs | 8 ++++---- .../rustc_trait_selection/src/traits/util.rs | 13 +++++++++---- .../ui/traits/vtable/vtable-non-object-safe.rs | 18 ++++++++++++++++++ .../vtable/vtable-non-object-safe.stderr | 16 ++++++++++++++++ src/test/ui/traits/vtable/vtable-vacant.rs | 7 +++++-- src/test/ui/traits/vtable/vtable-vacant.stderr | 4 ++-- 6 files changed, 54 insertions(+), 12 deletions(-) create mode 100644 src/test/ui/traits/vtable/vtable-non-object-safe.rs create mode 100644 src/test/ui/traits/vtable/vtable-non-object-safe.stderr diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs index 17a4184c3c9ef..f5a02ee7ad205 100644 --- a/compiler/rustc_trait_selection/src/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/mod.rs @@ -647,14 +647,14 @@ fn vtable_entries<'tcx>( .filter(|item| item.kind == ty::AssocKind::Fn); // Now list each method's DefId and InternalSubsts (for within its trait). // If the method can never be called from this object, produce `Vacant`. - let own_entries = trait_methods.map(move |trait_method| { + let own_entries = trait_methods.filter_map(move |trait_method| { debug!("vtable_entries: trait_method={:?}", trait_method); let def_id = trait_method.def_id; // Some methods cannot be called on an object; skip those. if !is_vtable_safe_method(tcx, trait_ref.def_id(), &trait_method) { debug!("vtable_entries: not vtable safe"); - return VtblEntry::Vacant; + return None; } // The method may have some early-bound lifetimes; add regions for those. @@ -681,7 +681,7 @@ fn vtable_entries<'tcx>( let predicates = tcx.predicates_of(def_id).instantiate_own(tcx, substs); if impossible_predicates(tcx, predicates.predicates) { debug!("vtable_entries: predicates do not hold"); - return VtblEntry::Vacant; + return Some(VtblEntry::Vacant); } let instance = ty::Instance::resolve_for_vtable( @@ -691,7 +691,7 @@ fn vtable_entries<'tcx>( substs, ) .expect("resolution failed during building vtable representation"); - VtblEntry::Method(instance) + Some(VtblEntry::Method(instance)) }); entries.extend(own_entries); diff --git a/compiler/rustc_trait_selection/src/traits/util.rs b/compiler/rustc_trait_selection/src/traits/util.rs index fd94f9f799847..3b98fe48c8c1b 100644 --- a/compiler/rustc_trait_selection/src/traits/util.rs +++ b/compiler/rustc_trait_selection/src/traits/util.rs @@ -289,7 +289,9 @@ pub fn count_own_vtable_entries(tcx: TyCtxt<'tcx>, trait_ref: ty::PolyTraitRef<' // Count number of methods and add them to the total offset. // Skip over associated types and constants. for trait_item in tcx.associated_items(trait_ref.def_id()).in_definition_order() { - if trait_item.kind == ty::AssocKind::Fn { + let is_vtable_safe_method = trait_item.kind == ty::AssocKind::Fn + && super::is_vtable_safe_method(tcx, trait_ref.def_id(), trait_item); + if is_vtable_safe_method { entries += 1; } } @@ -308,13 +310,16 @@ pub fn get_vtable_index_of_object_method( // add them to the total offset. // Skip over associated types and constants, as those aren't stored in the vtable. let mut entries = object.vtable_base; - for trait_item in tcx.associated_items(object.upcast_trait_ref.def_id()).in_definition_order() { + let trait_def_id = object.upcast_trait_ref.def_id(); + for trait_item in tcx.associated_items(trait_def_id).in_definition_order() { + let is_vtable_safe_method = trait_item.kind == ty::AssocKind::Fn + && super::is_vtable_safe_method(tcx, trait_def_id, trait_item); if trait_item.def_id == method_def_id { // The item with the ID we were given really ought to be a method. - assert_eq!(trait_item.kind, ty::AssocKind::Fn); + assert!(is_vtable_safe_method); return entries; } - if trait_item.kind == ty::AssocKind::Fn { + if is_vtable_safe_method { entries += 1; } } diff --git a/src/test/ui/traits/vtable/vtable-non-object-safe.rs b/src/test/ui/traits/vtable/vtable-non-object-safe.rs new file mode 100644 index 0000000000000..45b6a8a98a79f --- /dev/null +++ b/src/test/ui/traits/vtable/vtable-non-object-safe.rs @@ -0,0 +1,18 @@ +// build-fail +#![feature(rustc_attrs)] + +// Ensure that non-object-safe methods in Iterator does not generate +// vtable entries. + +#[rustc_dump_vtable] +trait A: Iterator {} +//~^ error Vtable + +impl A for T where T: Iterator {} + +fn foo(_a: &mut dyn A) { +} + +fn main() { + foo(&mut vec![0, 1, 2, 3].into_iter()); +} diff --git a/src/test/ui/traits/vtable/vtable-non-object-safe.stderr b/src/test/ui/traits/vtable/vtable-non-object-safe.stderr new file mode 100644 index 0000000000000..f3175b805d1b6 --- /dev/null +++ b/src/test/ui/traits/vtable/vtable-non-object-safe.stderr @@ -0,0 +1,16 @@ +error: Vtable entries for ` as A>`: [ + MetadataDropInPlace, + MetadataSize, + MetadataAlign, + Method( as Iterator>::next), + Method( as Iterator>::size_hint), + Method( as Iterator>::advance_by), + Method( as Iterator>::nth), +] + --> $DIR/vtable-non-object-safe.rs:8:1 + | +LL | trait A: Iterator {} + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/traits/vtable/vtable-vacant.rs b/src/test/ui/traits/vtable/vtable-vacant.rs index ebea94171f2ab..429ce523799f3 100644 --- a/src/test/ui/traits/vtable/vtable-vacant.rs +++ b/src/test/ui/traits/vtable/vtable-vacant.rs @@ -1,22 +1,25 @@ // build-fail #![feature(rustc_attrs)] +#![feature(negative_impls)] +#![allow(where_clauses_object_safety)] // B --> A #[rustc_dump_vtable] trait A { fn foo_a1(&self) {} - fn foo_a2(&self) where Self: Sized {} + fn foo_a2(&self) where Self: Send {} } #[rustc_dump_vtable] trait B: A { //~^ error Vtable fn foo_b1(&self) {} - fn foo_b2() where Self: Sized {} + fn foo_b2(&self) where Self: Send {} } struct S; +impl !Send for S {} impl A for S {} impl B for S {} diff --git a/src/test/ui/traits/vtable/vtable-vacant.stderr b/src/test/ui/traits/vtable/vtable-vacant.stderr index 768cca526894a..f5cd36264fcff 100644 --- a/src/test/ui/traits/vtable/vtable-vacant.stderr +++ b/src/test/ui/traits/vtable/vtable-vacant.stderr @@ -7,12 +7,12 @@ error: Vtable entries for ``: [ Method(::foo_b1), Vacant, ] - --> $DIR/vtable-vacant.rs:13:1 + --> $DIR/vtable-vacant.rs:15:1 | LL | / trait B: A { LL | | LL | | fn foo_b1(&self) {} -LL | | fn foo_b2() where Self: Sized {} +LL | | fn foo_b2(&self) where Self: Send {} LL | | } | |_^ From 97214eecc5a6c35c6cd8d9798207175cc2e15812 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Wed, 1 Sep 2021 23:04:28 +0100 Subject: [PATCH 2/2] Add query `own_existential_vtable_entries` --- compiler/rustc_middle/src/query/mod.rs | 6 +++ compiler/rustc_query_impl/src/keys.rs | 10 ++++ .../rustc_trait_selection/src/traits/mod.rs | 54 +++++++++++++------ .../rustc_trait_selection/src/traits/util.rs | 45 ++++++---------- 4 files changed, 70 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index dada94edc95ff..c93996162e3e3 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -996,6 +996,12 @@ rustc_queries! { desc { |tcx| "checking if item has mir available: `{}`", tcx.def_path_str(key) } } + query own_existential_vtable_entries( + key: ty::PolyExistentialTraitRef<'tcx> + ) -> &'tcx [DefId] { + desc { |tcx| "finding all existential vtable entries for trait {}", tcx.def_path_str(key.def_id()) } + } + query vtable_entries(key: ty::PolyTraitRef<'tcx>) -> &'tcx [ty::VtblEntry<'tcx>] { desc { |tcx| "finding all vtable entries for trait {}", tcx.def_path_str(key.def_id()) } diff --git a/compiler/rustc_query_impl/src/keys.rs b/compiler/rustc_query_impl/src/keys.rs index c973eae6b0665..42e8b4023cfad 100644 --- a/compiler/rustc_query_impl/src/keys.rs +++ b/compiler/rustc_query_impl/src/keys.rs @@ -294,6 +294,16 @@ impl<'tcx> Key for ty::PolyTraitRef<'tcx> { } } +impl<'tcx> Key for ty::PolyExistentialTraitRef<'tcx> { + #[inline(always)] + fn query_crate_is_local(&self) -> bool { + self.def_id().krate == LOCAL_CRATE + } + fn default_span(&self, tcx: TyCtxt<'_>) -> Span { + tcx.def_span(self.def_id()) + } +} + impl<'tcx> Key for (ty::PolyTraitRef<'tcx>, ty::PolyTraitRef<'tcx>) { #[inline(always)] fn query_crate_is_local(&self) -> bool { diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs index f5a02ee7ad205..44c675243838a 100644 --- a/compiler/rustc_trait_selection/src/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/mod.rs @@ -625,6 +625,31 @@ fn dump_vtable_entries<'tcx>( tcx.sess.struct_span_err(sp, &msg).emit(); } +fn own_existential_vtable_entries<'tcx>( + tcx: TyCtxt<'tcx>, + trait_ref: ty::PolyExistentialTraitRef<'tcx>, +) -> &'tcx [DefId] { + let trait_methods = tcx + .associated_items(trait_ref.def_id()) + .in_definition_order() + .filter(|item| item.kind == ty::AssocKind::Fn); + // Now list each method's DefId (for within its trait). + let own_entries = trait_methods.filter_map(move |trait_method| { + debug!("own_existential_vtable_entry: trait_method={:?}", trait_method); + let def_id = trait_method.def_id; + + // Some methods cannot be called on an object; skip those. + if !is_vtable_safe_method(tcx, trait_ref.def_id(), &trait_method) { + debug!("own_existential_vtable_entry: not vtable safe"); + return None; + } + + Some(def_id) + }); + + tcx.arena.alloc_from_iter(own_entries.into_iter()) +} + /// Given a trait `trait_ref`, iterates the vtable entries /// that come from `trait_ref`, including its supertraits. fn vtable_entries<'tcx>( @@ -641,21 +666,15 @@ fn vtable_entries<'tcx>( entries.extend(COMMON_VTABLE_ENTRIES); } VtblSegment::TraitOwnEntries { trait_ref, emit_vptr } => { - let trait_methods = tcx - .associated_items(trait_ref.def_id()) - .in_definition_order() - .filter(|item| item.kind == ty::AssocKind::Fn); - // Now list each method's DefId and InternalSubsts (for within its trait). - // If the method can never be called from this object, produce `Vacant`. - let own_entries = trait_methods.filter_map(move |trait_method| { - debug!("vtable_entries: trait_method={:?}", trait_method); - let def_id = trait_method.def_id; - - // Some methods cannot be called on an object; skip those. - if !is_vtable_safe_method(tcx, trait_ref.def_id(), &trait_method) { - debug!("vtable_entries: not vtable safe"); - return None; - } + let existential_trait_ref = trait_ref + .map_bound(|trait_ref| ty::ExistentialTraitRef::erase_self_ty(tcx, trait_ref)); + + // Lookup the shape of vtable for the trait. + let own_existential_entries = + tcx.own_existential_vtable_entries(existential_trait_ref); + + let own_entries = own_existential_entries.iter().copied().map(|def_id| { + debug!("vtable_entries: trait_method={:?}", def_id); // The method may have some early-bound lifetimes; add regions for those. let substs = trait_ref.map_bound(|trait_ref| { @@ -681,7 +700,7 @@ fn vtable_entries<'tcx>( let predicates = tcx.predicates_of(def_id).instantiate_own(tcx, substs); if impossible_predicates(tcx, predicates.predicates) { debug!("vtable_entries: predicates do not hold"); - return Some(VtblEntry::Vacant); + return VtblEntry::Vacant; } let instance = ty::Instance::resolve_for_vtable( @@ -691,7 +710,7 @@ fn vtable_entries<'tcx>( substs, ) .expect("resolution failed during building vtable representation"); - Some(VtblEntry::Method(instance)) + VtblEntry::Method(instance) }); entries.extend(own_entries); @@ -804,6 +823,7 @@ pub fn provide(providers: &mut ty::query::Providers) { specialization_graph_of: specialize::specialization_graph_provider, specializes: specialize::specializes, codegen_fulfill_obligation: codegen::codegen_fulfill_obligation, + own_existential_vtable_entries, vtable_entries, vtable_trait_upcasting_coercion_new_vptr_slot, subst_and_check_impossible_predicates, diff --git a/compiler/rustc_trait_selection/src/traits/util.rs b/compiler/rustc_trait_selection/src/traits/util.rs index 3b98fe48c8c1b..b108d85bb20c9 100644 --- a/compiler/rustc_trait_selection/src/traits/util.rs +++ b/compiler/rustc_trait_selection/src/traits/util.rs @@ -285,17 +285,10 @@ pub fn upcast_choices( /// that come from `trait_ref`, excluding its supertraits. Used in /// computing the vtable base for an upcast trait of a trait object. pub fn count_own_vtable_entries(tcx: TyCtxt<'tcx>, trait_ref: ty::PolyTraitRef<'tcx>) -> usize { - let mut entries = 0; - // Count number of methods and add them to the total offset. - // Skip over associated types and constants. - for trait_item in tcx.associated_items(trait_ref.def_id()).in_definition_order() { - let is_vtable_safe_method = trait_item.kind == ty::AssocKind::Fn - && super::is_vtable_safe_method(tcx, trait_ref.def_id(), trait_item); - if is_vtable_safe_method { - entries += 1; - } - } - entries + let existential_trait_ref = + trait_ref.map_bound(|trait_ref| ty::ExistentialTraitRef::erase_self_ty(tcx, trait_ref)); + let existential_trait_ref = tcx.erase_regions(existential_trait_ref); + tcx.own_existential_vtable_entries(existential_trait_ref).len() } /// Given an upcast trait object described by `object`, returns the @@ -306,25 +299,21 @@ pub fn get_vtable_index_of_object_method( object: &super::ImplSourceObjectData<'tcx, N>, method_def_id: DefId, ) -> usize { + let existential_trait_ref = object + .upcast_trait_ref + .map_bound(|trait_ref| ty::ExistentialTraitRef::erase_self_ty(tcx, trait_ref)); + let existential_trait_ref = tcx.erase_regions(existential_trait_ref); // Count number of methods preceding the one we are selecting and // add them to the total offset. - // Skip over associated types and constants, as those aren't stored in the vtable. - let mut entries = object.vtable_base; - let trait_def_id = object.upcast_trait_ref.def_id(); - for trait_item in tcx.associated_items(trait_def_id).in_definition_order() { - let is_vtable_safe_method = trait_item.kind == ty::AssocKind::Fn - && super::is_vtable_safe_method(tcx, trait_def_id, trait_item); - if trait_item.def_id == method_def_id { - // The item with the ID we were given really ought to be a method. - assert!(is_vtable_safe_method); - return entries; - } - if is_vtable_safe_method { - entries += 1; - } - } - - bug!("get_vtable_index_of_object_method: {:?} was not found", method_def_id); + let index = tcx + .own_existential_vtable_entries(existential_trait_ref) + .iter() + .copied() + .position(|def_id| def_id == method_def_id) + .unwrap_or_else(|| { + bug!("get_vtable_index_of_object_method: {:?} was not found", method_def_id); + }); + object.vtable_base + index } pub fn closure_trait_ref_and_return_type(