From a1e7495a41bac25797a7b82aee18f350e406bc2a Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 28 Mar 2020 21:15:45 +0100 Subject: [PATCH 1/7] Simplify `NodeItem` The generic parameter is unused, and so is `map` --- .../traits/specialization_graph.rs | 18 +++++++----------- src/librustc_trait_selection/traits/project.rs | 2 +- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/librustc_middle/traits/specialization_graph.rs b/src/librustc_middle/traits/specialization_graph.rs index 1847326a742eb..b09f1e66f1774 100644 --- a/src/librustc_middle/traits/specialization_graph.rs +++ b/src/librustc_middle/traits/specialization_graph.rs @@ -154,15 +154,9 @@ impl Iterator for Ancestors<'_> { } } -pub struct NodeItem { +pub struct NodeItem { pub node: Node, - pub item: T, -} - -impl NodeItem { - pub fn map U>(self, f: F) -> NodeItem { - NodeItem { node: self.node, item: f(self.item) } - } + pub item: ty::AssocItem, } impl<'tcx> Ancestors<'tcx> { @@ -173,7 +167,7 @@ impl<'tcx> Ancestors<'tcx> { tcx: TyCtxt<'tcx>, trait_item_name: Ident, trait_item_kind: ty::AssocKind, - ) -> Option> { + ) -> Option { let trait_def_id = self.trait_def_id; self.find_map(|node| { node.item(tcx, trait_item_name, trait_item_kind, trait_def_id) @@ -183,8 +177,10 @@ impl<'tcx> Ancestors<'tcx> { } /// Walk up the specialization ancestors of a given impl, starting with that -/// impl itself. Returns `None` if an error was reported while building the -/// specialization graph. +/// impl itself. +/// +/// Returns `Err` if an error was reported while building the specialization +/// graph. pub fn ancestors( tcx: TyCtxt<'tcx>, trait_def_id: DefId, diff --git a/src/librustc_trait_selection/traits/project.rs b/src/librustc_trait_selection/traits/project.rs index 2eb63b8f59030..1e26b62c75951 100644 --- a/src/librustc_trait_selection/traits/project.rs +++ b/src/librustc_trait_selection/traits/project.rs @@ -1447,7 +1447,7 @@ fn assoc_ty_def( selcx: &SelectionContext<'_, '_>, impl_def_id: DefId, assoc_ty_def_id: DefId, -) -> Result, ErrorReported> { +) -> Result { let tcx = selcx.tcx(); let assoc_ty_name = tcx.associated_item(assoc_ty_def_id).ident; let trait_def_id = tcx.impl_trait_ref(impl_def_id).unwrap().def_id; From 49ba323c8db6fd5fbe3d72f623c9d89cb09c508d Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sun, 29 Mar 2020 00:57:49 +0100 Subject: [PATCH 2/7] spec. graph: track defining and finalizing impls --- src/librustc_middle/traits/mod.rs | 1 + .../traits/specialization_graph.rs | 62 +++++++++++++++++-- src/librustc_middle/traits/util.rs | 17 +++++ src/librustc_trait_selection/traits/mod.rs | 3 +- .../traits/project.rs | 48 ++++---------- .../traits/specialize/mod.rs | 2 +- src/librustc_trait_selection/traits/util.rs | 16 +---- src/librustc_typeck/check/mod.rs | 7 ++- 8 files changed, 93 insertions(+), 63 deletions(-) create mode 100644 src/librustc_middle/traits/util.rs diff --git a/src/librustc_middle/traits/mod.rs b/src/librustc_middle/traits/mod.rs index c129b574fd38a..d47e2d75fba35 100644 --- a/src/librustc_middle/traits/mod.rs +++ b/src/librustc_middle/traits/mod.rs @@ -6,6 +6,7 @@ pub mod query; pub mod select; pub mod specialization_graph; mod structural_impls; +pub mod util; use crate::mir::interpret::ErrorHandled; use crate::ty::subst::SubstsRef; diff --git a/src/librustc_middle/traits/specialization_graph.rs b/src/librustc_middle/traits/specialization_graph.rs index b09f1e66f1774..f66f94fe8c168 100644 --- a/src/librustc_middle/traits/specialization_graph.rs +++ b/src/librustc_middle/traits/specialization_graph.rs @@ -154,9 +154,45 @@ impl Iterator for Ancestors<'_> { } } -pub struct NodeItem { - pub node: Node, +/// Information about the most specialized definition of an associated item. +pub struct LeafDef { + /// The associated item described by this `LeafDef`. pub item: ty::AssocItem, + + /// The node in the specialization graph containing the definition of `item`. + pub defining_node: Node, + + /// The "top-most" (ie. least specialized) specialization graph node that finalized the + /// definition of `item`. + /// + /// Example: + /// + /// ``` + /// trait Tr { + /// fn assoc(&self); + /// } + /// + /// impl Tr for T { + /// default fn assoc(&self) {} + /// } + /// + /// impl Tr for u8 {} + /// ``` + /// + /// If we start the leaf definition search at `impl Tr for u8`, that impl will be the + /// `finalizing_node`, while `defining_node` will be the generic impl. + /// + /// If the leaf definition search is started at the generic impl, `finalizing_node` will be + /// `None`, since the most specialized impl we found still allows overriding the method + /// (doesn't finalize it). + pub finalizing_node: Option, +} + +impl LeafDef { + /// Returns whether this definition is known to not be further specializable. + pub fn is_final(&self) -> bool { + self.finalizing_node.is_some() + } } impl<'tcx> Ancestors<'tcx> { @@ -167,11 +203,27 @@ impl<'tcx> Ancestors<'tcx> { tcx: TyCtxt<'tcx>, trait_item_name: Ident, trait_item_kind: ty::AssocKind, - ) -> Option { + ) -> Option { let trait_def_id = self.trait_def_id; + let mut finalizing_node = None; + self.find_map(|node| { - node.item(tcx, trait_item_name, trait_item_kind, trait_def_id) - .map(|item| NodeItem { node, item }) + if let Some(item) = node.item(tcx, trait_item_name, trait_item_kind, trait_def_id) { + if finalizing_node.is_none() { + let is_specializable = item.defaultness.is_default() + || super::util::impl_is_default(tcx, node.def_id()); + + if !is_specializable { + finalizing_node = Some(node); + } + } + + Some(LeafDef { item, defining_node: node, finalizing_node }) + } else { + // Item not mentioned. This "finalizes" any defaulted item provided by an ancestor. + finalizing_node = Some(node); + None + } }) } } diff --git a/src/librustc_middle/traits/util.rs b/src/librustc_middle/traits/util.rs new file mode 100644 index 0000000000000..cb29cf0760ebc --- /dev/null +++ b/src/librustc_middle/traits/util.rs @@ -0,0 +1,17 @@ +use crate::ty::TyCtxt; +use rustc_hir as hir; +use rustc_hir::def_id::DefId; + +pub fn impl_is_default(tcx: TyCtxt<'_>, node_item_def_id: DefId) -> bool { + match tcx.hir().as_local_hir_id(node_item_def_id) { + Some(hir_id) => { + let item = tcx.hir().expect_item(hir_id); + if let hir::ItemKind::Impl { defaultness, .. } = item.kind { + defaultness.is_default() + } else { + false + } + } + None => tcx.impl_defaultness(node_item_def_id).is_default(), + } +} diff --git a/src/librustc_trait_selection/traits/mod.rs b/src/librustc_trait_selection/traits/mod.rs index 3ef44a198417a..d6204975a7bf2 100644 --- a/src/librustc_trait_selection/traits/mod.rs +++ b/src/librustc_trait_selection/traits/mod.rs @@ -64,8 +64,7 @@ pub use self::structural_match::NonStructuralMatchTy; pub use self::util::{elaborate_predicates, elaborate_trait_ref, elaborate_trait_refs}; pub use self::util::{expand_trait_aliases, TraitAliasExpander}; pub use self::util::{ - get_vtable_index_of_object_method, impl_is_default, impl_item_is_final, - predicate_for_trait_def, upcast_choices, + get_vtable_index_of_object_method, impl_item_is_final, predicate_for_trait_def, upcast_choices, }; pub use self::util::{ supertrait_def_ids, supertraits, transitive_bounds, SupertraitDefIds, Supertraits, diff --git a/src/librustc_trait_selection/traits/project.rs b/src/librustc_trait_selection/traits/project.rs index 1e26b62c75951..cd89111a17712 100644 --- a/src/librustc_trait_selection/traits/project.rs +++ b/src/librustc_trait_selection/traits/project.rs @@ -1015,49 +1015,21 @@ fn assemble_candidates_from_impls<'cx, 'tcx>( assoc_ty_def(selcx, impl_data.impl_def_id, obligation.predicate.item_def_id) .map_err(|ErrorReported| ())?; - let is_default = if node_item.node.is_from_trait() { - // If true, the impl inherited a `type Foo = Bar` - // given in the trait, which is implicitly default. - // Otherwise, the impl did not specify `type` and - // neither did the trait: - // - // ```rust - // trait Foo { type T; } - // impl Foo for Bar { } - // ``` - // - // This is an error, but it will be - // reported in `check_impl_items_against_trait`. - // We accept it here but will flag it as - // an error when we confirm the candidate - // (which will ultimately lead to `normalize_to_error` - // being invoked). - false - } else { - // If we're looking at a trait *impl*, the item is - // specializable if the impl or the item are marked - // `default`. - node_item.item.defaultness.is_default() - || super::util::impl_is_default(selcx.tcx(), node_item.node.def_id()) - }; - - match is_default { + if node_item.is_final() { // Non-specializable items are always projectable - false => true, - + true + } else { // Only reveal a specializable default if we're past type-checking // and the obligation is monomorphic, otherwise passes such as // transmute checking and polymorphic MIR optimizations could // get a result which isn't correct for all monomorphizations. - true if obligation.param_env.reveal == Reveal::All => { + if obligation.param_env.reveal == Reveal::All { // NOTE(eddyb) inference variables can resolve to parameters, so // assume `poly_trait_ref` isn't monomorphic, if it contains any. let poly_trait_ref = selcx.infcx().resolve_vars_if_possible(&poly_trait_ref); !poly_trait_ref.needs_infer() && !poly_trait_ref.needs_subst() - } - - true => { + } else { debug!( "assemble_candidates_from_impls: not eligible due to default: \ assoc_ty={} predicate={}", @@ -1422,7 +1394,8 @@ fn confirm_impl_candidate<'cx, 'tcx>( return Progress { ty: tcx.types.err, obligations: nested }; } let substs = obligation.predicate.substs.rebase_onto(tcx, trait_def_id, substs); - let substs = translate_substs(selcx.infcx(), param_env, impl_def_id, substs, assoc_ty.node); + let substs = + translate_substs(selcx.infcx(), param_env, impl_def_id, substs, assoc_ty.defining_node); let ty = if let ty::AssocKind::OpaqueTy = assoc_ty.item.kind { let item_substs = InternalSubsts::identity_for_item(tcx, assoc_ty.item.def_id); tcx.mk_opaque(assoc_ty.item.def_id, item_substs) @@ -1447,7 +1420,7 @@ fn assoc_ty_def( selcx: &SelectionContext<'_, '_>, impl_def_id: DefId, assoc_ty_def_id: DefId, -) -> Result { +) -> Result { let tcx = selcx.tcx(); let assoc_ty_name = tcx.associated_item(assoc_ty_def_id).ident; let trait_def_id = tcx.impl_trait_ref(impl_def_id).unwrap().def_id; @@ -1464,9 +1437,10 @@ fn assoc_ty_def( if matches!(item.kind, ty::AssocKind::Type | ty::AssocKind::OpaqueTy) && tcx.hygienic_eq(item.ident, assoc_ty_name, trait_def_id) { - return Ok(specialization_graph::NodeItem { - node: specialization_graph::Node::Impl(impl_def_id), + return Ok(specialization_graph::LeafDef { item: *item, + defining_node: impl_node, + finalizing_node: if item.defaultness.is_default() { None } else { Some(impl_node) }, }); } } diff --git a/src/librustc_trait_selection/traits/specialize/mod.rs b/src/librustc_trait_selection/traits/specialize/mod.rs index edf02d75ee60f..f98d8f2fabf6a 100644 --- a/src/librustc_trait_selection/traits/specialize/mod.rs +++ b/src/librustc_trait_selection/traits/specialize/mod.rs @@ -141,7 +141,7 @@ pub fn find_associated_item<'tcx>( param_env, impl_data.impl_def_id, substs, - node_item.node, + node_item.defining_node, ); infcx.tcx.erase_regions(&substs) }); diff --git a/src/librustc_trait_selection/traits/util.rs b/src/librustc_trait_selection/traits/util.rs index 6eeac2f676c09..c28628678a935 100644 --- a/src/librustc_trait_selection/traits/util.rs +++ b/src/librustc_trait_selection/traits/util.rs @@ -4,11 +4,11 @@ use smallvec::smallvec; use smallvec::SmallVec; use rustc_data_structures::fx::FxHashSet; -use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_middle::ty::outlives::Component; use rustc_middle::ty::subst::{GenericArg, Subst, SubstsRef}; use rustc_middle::ty::{self, ToPolyTraitRef, ToPredicate, Ty, TyCtxt, WithConstness}; +use rustc_middle::traits::util::impl_is_default; use super::{Normalized, Obligation, ObligationCause, PredicateObligation, SelectionContext}; @@ -651,20 +651,6 @@ pub fn generator_trait_ref_and_outputs( ty::Binder::bind((trait_ref, sig.skip_binder().yield_ty, sig.skip_binder().return_ty)) } -pub fn impl_is_default(tcx: TyCtxt<'_>, node_item_def_id: DefId) -> bool { - match tcx.hir().as_local_hir_id(node_item_def_id) { - Some(hir_id) => { - let item = tcx.hir().expect_item(hir_id); - if let hir::ItemKind::Impl { defaultness, .. } = item.kind { - defaultness.is_default() - } else { - false - } - } - None => tcx.impl_defaultness(node_item_def_id).is_default(), - } -} - pub fn impl_item_is_final(tcx: TyCtxt<'_>, assoc_item: &ty::AssocItem) -> bool { assoc_item.defaultness.is_final() && !impl_is_default(tcx, assoc_item.container.id()) } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 6249e1d49779e..c85b5a4f2a258 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -110,6 +110,7 @@ use rustc_infer::infer::{self, InferCtxt, InferOk, InferResult, TyCtxtInferExt}; use rustc_middle::hir::map::blocks::FnLikeNode; use rustc_middle::middle::region; use rustc_middle::mir::interpret::ConstValue; +use rustc_middle::traits::util::impl_is_default; use rustc_middle::ty::adjustment::{ Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, PointerCast, }; @@ -1942,7 +1943,7 @@ fn check_specialization_validity<'tcx>( // grandparent. In that case, if parent is a `default impl`, inherited items use the // "defaultness" from the grandparent, else they are final. None => { - if traits::impl_is_default(tcx, parent_impl.def_id()) { + if impl_is_default(tcx, parent_impl.def_id()) { None } else { Some(Err(parent_impl.def_id())) @@ -2114,10 +2115,10 @@ fn check_impl_items_against_trait<'tcx>( for trait_item in tcx.associated_items(impl_trait_ref.def_id).in_definition_order() { let is_implemented = ancestors .leaf_def(tcx, trait_item.ident, trait_item.kind) - .map(|node_item| !node_item.node.is_from_trait()) + .map(|node_item| !node_item.defining_node.is_from_trait()) .unwrap_or(false); - if !is_implemented && !traits::impl_is_default(tcx, impl_id) { + if !is_implemented && !impl_is_default(tcx, impl_id) { if !trait_item.defaultness.has_value() { missing_items.push(*trait_item); } From 103771ce57acc240bc01abf4f7365172935bc7fb Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sun, 29 Mar 2020 01:01:32 +0100 Subject: [PATCH 3/7] Add a test --- src/test/ui/specialization/issue-70442.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 src/test/ui/specialization/issue-70442.rs diff --git a/src/test/ui/specialization/issue-70442.rs b/src/test/ui/specialization/issue-70442.rs new file mode 100644 index 0000000000000..4371dd2e16747 --- /dev/null +++ b/src/test/ui/specialization/issue-70442.rs @@ -0,0 +1,23 @@ +#![feature(specialization)] + +// check-pass + +trait Trait { + type Assoc; +} + +impl Trait for T { + default type Assoc = bool; +} + +// This impl inherits the `Assoc` definition from above and "locks it in", or finalizes it, making +// child impls unable to further specialize it. However, since the specialization graph didn't +// correctly track this, we would refuse to project `Assoc` from this impl, even though that should +// happen for items that are final. +impl Trait for () {} + +fn foo>() {} + +fn main() { + foo::<()>(); // `<() as Trait>::Assoc` is normalized to `bool` correctly +} From e8910f51d804438b6ed161a27e21dd272d9e969a Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sun, 29 Mar 2020 16:40:30 +0200 Subject: [PATCH 4/7] Sync `Instance::resolve` with the projection code --- src/librustc_trait_selection/traits/mod.rs | 1 - .../traits/project.rs | 2 +- .../traits/specialize/mod.rs | 44 +----------------- src/librustc_ty/instance.rs | 46 +++++++++++++++---- 4 files changed, 40 insertions(+), 53 deletions(-) diff --git a/src/librustc_trait_selection/traits/mod.rs b/src/librustc_trait_selection/traits/mod.rs index d6204975a7bf2..5f98850633075 100644 --- a/src/librustc_trait_selection/traits/mod.rs +++ b/src/librustc_trait_selection/traits/mod.rs @@ -54,7 +54,6 @@ pub use self::project::{ }; pub use self::select::{EvaluationCache, SelectionCache, SelectionContext}; pub use self::select::{EvaluationResult, IntercrateAmbiguityCause, OverflowError}; -pub use self::specialize::find_associated_item; pub use self::specialize::specialization_graph::FutureCompatOverlapError; pub use self::specialize::specialization_graph::FutureCompatOverlapErrorKind; pub use self::specialize::{specialization_graph, translate_substs, OverlapError}; diff --git a/src/librustc_trait_selection/traits/project.rs b/src/librustc_trait_selection/traits/project.rs index cd89111a17712..aae0d46756331 100644 --- a/src/librustc_trait_selection/traits/project.rs +++ b/src/librustc_trait_selection/traits/project.rs @@ -1016,7 +1016,7 @@ fn assemble_candidates_from_impls<'cx, 'tcx>( .map_err(|ErrorReported| ())?; if node_item.is_final() { - // Non-specializable items are always projectable + // Non-specializable items are always projectable. true } else { // Only reveal a specializable default if we're past type-checking diff --git a/src/librustc_trait_selection/traits/specialize/mod.rs b/src/librustc_trait_selection/traits/specialize/mod.rs index f98d8f2fabf6a..fabd8c89b72af 100644 --- a/src/librustc_trait_selection/traits/specialize/mod.rs +++ b/src/librustc_trait_selection/traits/specialize/mod.rs @@ -20,7 +20,7 @@ use rustc_errors::struct_span_err; use rustc_hir::def_id::DefId; use rustc_middle::lint::LintDiagnosticBuilder; use rustc_middle::ty::subst::{InternalSubsts, Subst, SubstsRef}; -use rustc_middle::ty::{self, TyCtxt, TypeFoldable}; +use rustc_middle::ty::{self, TyCtxt}; use rustc_session::lint::builtin::COHERENCE_LEAK_CHECK; use rustc_session::lint::builtin::ORDER_DEPENDENT_TRAIT_OBJECTS; use rustc_span::DUMMY_SP; @@ -112,48 +112,6 @@ pub fn translate_substs<'a, 'tcx>( source_substs.rebase_onto(infcx.tcx, source_impl, target_substs) } -/// Given a selected impl described by `impl_data`, returns the -/// definition and substitutions for the method with the name `name` -/// the kind `kind`, and trait method substitutions `substs`, in -/// that impl, a less specialized impl, or the trait default, -/// whichever applies. -pub fn find_associated_item<'tcx>( - tcx: TyCtxt<'tcx>, - param_env: ty::ParamEnv<'tcx>, - item: &ty::AssocItem, - substs: SubstsRef<'tcx>, - impl_data: &super::VtableImplData<'tcx, ()>, -) -> (DefId, SubstsRef<'tcx>) { - debug!("find_associated_item({:?}, {:?}, {:?}, {:?})", param_env, item, substs, impl_data); - assert!(!substs.needs_infer()); - - let trait_def_id = tcx.trait_id_of_impl(impl_data.impl_def_id).unwrap(); - let trait_def = tcx.trait_def(trait_def_id); - - if let Ok(ancestors) = trait_def.ancestors(tcx, impl_data.impl_def_id) { - match ancestors.leaf_def(tcx, item.ident, item.kind) { - Some(node_item) => { - let substs = tcx.infer_ctxt().enter(|infcx| { - let param_env = param_env.with_reveal_all(); - let substs = substs.rebase_onto(tcx, trait_def_id, impl_data.substs); - let substs = translate_substs( - &infcx, - param_env, - impl_data.impl_def_id, - substs, - node_item.defining_node, - ); - infcx.tcx.erase_regions(&substs) - }); - (node_item.item.def_id, substs) - } - None => bug!("{:?} not found in {:?}", item, impl_data.impl_def_id), - } - } else { - (item.def_id, substs) - } -} - /// Is `impl1` a specialization of `impl2`? /// /// Specialization is determined by the sets of types to which the impls apply; diff --git a/src/librustc_ty/instance.rs b/src/librustc_ty/instance.rs index 447c49e40fb5a..2b7d7e7e63771 100644 --- a/src/librustc_ty/instance.rs +++ b/src/librustc_ty/instance.rs @@ -1,9 +1,11 @@ use rustc_hir::def_id::DefId; use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::{self, Instance, TyCtxt, TypeFoldable}; +use rustc_infer::infer::TyCtxtInferExt; use rustc_span::sym; use rustc_target::spec::abi::Abi; use rustc_trait_selection::traits; +use traits::{Reveal, translate_substs}; use log::debug; @@ -82,21 +84,49 @@ fn resolve_associated_item<'tcx>( // the actual function: match vtbl { traits::VtableImpl(impl_data) => { - let (def_id, substs) = - traits::find_associated_item(tcx, param_env, trait_item, rcvr_substs, &impl_data); - - let resolved_item = tcx.associated_item(def_id); + debug!( + "resolving VtableImpl: {:?}, {:?}, {:?}, {:?}", + param_env, trait_item, rcvr_substs, impl_data + ); + assert!(!rcvr_substs.needs_infer()); + + let trait_def_id = tcx.trait_id_of_impl(impl_data.impl_def_id).unwrap(); + let trait_def = tcx.trait_def(trait_def_id); + let leaf_def = trait_def + .ancestors(tcx, impl_data.impl_def_id) + .ok()? + .leaf_def(tcx, trait_item.ident, trait_item.kind) + .unwrap_or_else(|| { + bug!("{:?} not found in {:?}", trait_item, impl_data.impl_def_id); + }); + let def_id = leaf_def.item.def_id; + + let substs = tcx.infer_ctxt().enter(|infcx| { + let param_env = param_env.with_reveal_all(); + let substs = rcvr_substs.rebase_onto(tcx, trait_def_id, impl_data.substs); + let substs = translate_substs( + &infcx, + param_env, + impl_data.impl_def_id, + substs, + leaf_def.defining_node, + ); + infcx.tcx.erase_regions(&substs) + }); // Since this is a trait item, we need to see if the item is either a trait default item // or a specialization because we can't resolve those unless we can `Reveal::All`. // NOTE: This should be kept in sync with the similar code in // `rustc_middle::traits::project::assemble_candidates_from_impls()`. - let eligible = if !resolved_item.defaultness.is_default() { + let eligible = if leaf_def.is_final() { + // Non-specializable items are always projectable. true - } else if param_env.reveal == traits::Reveal::All { - !trait_ref.needs_subst() } else { - false + // Only reveal a specializable default if we're past type-checking + // and the obligation is monomorphic, otherwise passes such as + // transmute checking and polymorphic MIR optimizations could + // get a result which isn't correct for all monomorphizations. + if param_env.reveal == Reveal::All { !trait_ref.needs_subst() } else { false } }; if !eligible { From b00ba382e06aff8aae4ef6ba927ce53a4efd6f25 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sun, 29 Mar 2020 20:01:14 +0200 Subject: [PATCH 5/7] Use query instead of `impl_is_default` fn --- src/librustc_middle/traits/mod.rs | 1 - .../traits/specialization_graph.rs | 2 +- src/librustc_middle/traits/util.rs | 17 ----------------- src/librustc_trait_selection/traits/util.rs | 3 +-- src/librustc_ty/ty.rs | 11 +++++++++++ src/librustc_typeck/check/mod.rs | 5 ++--- 6 files changed, 15 insertions(+), 24 deletions(-) delete mode 100644 src/librustc_middle/traits/util.rs diff --git a/src/librustc_middle/traits/mod.rs b/src/librustc_middle/traits/mod.rs index d47e2d75fba35..c129b574fd38a 100644 --- a/src/librustc_middle/traits/mod.rs +++ b/src/librustc_middle/traits/mod.rs @@ -6,7 +6,6 @@ pub mod query; pub mod select; pub mod specialization_graph; mod structural_impls; -pub mod util; use crate::mir::interpret::ErrorHandled; use crate::ty::subst::SubstsRef; diff --git a/src/librustc_middle/traits/specialization_graph.rs b/src/librustc_middle/traits/specialization_graph.rs index f66f94fe8c168..a2793f9805004 100644 --- a/src/librustc_middle/traits/specialization_graph.rs +++ b/src/librustc_middle/traits/specialization_graph.rs @@ -211,7 +211,7 @@ impl<'tcx> Ancestors<'tcx> { if let Some(item) = node.item(tcx, trait_item_name, trait_item_kind, trait_def_id) { if finalizing_node.is_none() { let is_specializable = item.defaultness.is_default() - || super::util::impl_is_default(tcx, node.def_id()); + || tcx.impl_defaultness(node.def_id()).is_default(); if !is_specializable { finalizing_node = Some(node); diff --git a/src/librustc_middle/traits/util.rs b/src/librustc_middle/traits/util.rs deleted file mode 100644 index cb29cf0760ebc..0000000000000 --- a/src/librustc_middle/traits/util.rs +++ /dev/null @@ -1,17 +0,0 @@ -use crate::ty::TyCtxt; -use rustc_hir as hir; -use rustc_hir::def_id::DefId; - -pub fn impl_is_default(tcx: TyCtxt<'_>, node_item_def_id: DefId) -> bool { - match tcx.hir().as_local_hir_id(node_item_def_id) { - Some(hir_id) => { - let item = tcx.hir().expect_item(hir_id); - if let hir::ItemKind::Impl { defaultness, .. } = item.kind { - defaultness.is_default() - } else { - false - } - } - None => tcx.impl_defaultness(node_item_def_id).is_default(), - } -} diff --git a/src/librustc_trait_selection/traits/util.rs b/src/librustc_trait_selection/traits/util.rs index c28628678a935..6348673dab8b5 100644 --- a/src/librustc_trait_selection/traits/util.rs +++ b/src/librustc_trait_selection/traits/util.rs @@ -8,7 +8,6 @@ use rustc_hir::def_id::DefId; use rustc_middle::ty::outlives::Component; use rustc_middle::ty::subst::{GenericArg, Subst, SubstsRef}; use rustc_middle::ty::{self, ToPolyTraitRef, ToPredicate, Ty, TyCtxt, WithConstness}; -use rustc_middle::traits::util::impl_is_default; use super::{Normalized, Obligation, ObligationCause, PredicateObligation, SelectionContext}; @@ -652,7 +651,7 @@ pub fn generator_trait_ref_and_outputs( } pub fn impl_item_is_final(tcx: TyCtxt<'_>, assoc_item: &ty::AssocItem) -> bool { - assoc_item.defaultness.is_final() && !impl_is_default(tcx, assoc_item.container.id()) + assoc_item.defaultness.is_final() && tcx.impl_defaultness(assoc_item.container.id()).is_final() } pub enum TupleArgumentsFlag { diff --git a/src/librustc_ty/ty.rs b/src/librustc_ty/ty.rs index 9d28447e212a6..aefe61f60b87a 100644 --- a/src/librustc_ty/ty.rs +++ b/src/librustc_ty/ty.rs @@ -165,6 +165,16 @@ fn associated_item(tcx: TyCtxt<'_>, def_id: DefId) -> ty::AssocItem { ) } +fn impl_defaultness(tcx: TyCtxt<'_>, def_id: DefId) -> hir::Defaultness { + let hir_id = tcx.hir().as_local_hir_id(def_id).unwrap(); + let item = tcx.hir().expect_item(hir_id); + if let hir::ItemKind::Impl { defaultness, .. } = item.kind { + defaultness + } else { + bug!("`impl_defaultness` called on {:?}", item); + } +} + /// Calculates the `Sized` constraint. /// /// In fact, there are only a few options for the types in the constraint: @@ -371,6 +381,7 @@ pub fn provide(providers: &mut ty::query::Providers<'_>) { crate_hash, instance_def_size_estimate, issue33140_self_ty, + impl_defaultness, ..*providers }; } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index c85b5a4f2a258..3823efe9d927e 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -110,7 +110,6 @@ use rustc_infer::infer::{self, InferCtxt, InferOk, InferResult, TyCtxtInferExt}; use rustc_middle::hir::map::blocks::FnLikeNode; use rustc_middle::middle::region; use rustc_middle::mir::interpret::ConstValue; -use rustc_middle::traits::util::impl_is_default; use rustc_middle::ty::adjustment::{ Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, PointerCast, }; @@ -1943,7 +1942,7 @@ fn check_specialization_validity<'tcx>( // grandparent. In that case, if parent is a `default impl`, inherited items use the // "defaultness" from the grandparent, else they are final. None => { - if impl_is_default(tcx, parent_impl.def_id()) { + if tcx.impl_defaultness(parent_impl.def_id()).is_default() { None } else { Some(Err(parent_impl.def_id())) @@ -2118,7 +2117,7 @@ fn check_impl_items_against_trait<'tcx>( .map(|node_item| !node_item.defining_node.is_from_trait()) .unwrap_or(false); - if !is_implemented && !impl_is_default(tcx, impl_id) { + if !is_implemented && tcx.impl_defaultness(impl_id).is_final() { if !trait_item.defaultness.has_value() { missing_items.push(*trait_item); } From 12d9f4efaf972c4da1192dda4f43c19c8983bc79 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sun, 29 Mar 2020 20:01:41 +0200 Subject: [PATCH 6/7] Assert that the trait ref does not need inference --- src/librustc_ty/instance.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc_ty/instance.rs b/src/librustc_ty/instance.rs index 2b7d7e7e63771..2d16f1f1d3a2c 100644 --- a/src/librustc_ty/instance.rs +++ b/src/librustc_ty/instance.rs @@ -89,6 +89,7 @@ fn resolve_associated_item<'tcx>( param_env, trait_item, rcvr_substs, impl_data ); assert!(!rcvr_substs.needs_infer()); + assert!(!trait_ref.needs_infer()); let trait_def_id = tcx.trait_id_of_impl(impl_data.impl_def_id).unwrap(); let trait_def = tcx.trait_def(trait_def_id); From fd8f8189d3323737a06285c9a4b926fefc9411fa Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 30 Mar 2020 21:40:53 +0200 Subject: [PATCH 7/7] Format --- src/librustc_ty/instance.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_ty/instance.rs b/src/librustc_ty/instance.rs index 2d16f1f1d3a2c..47c4b1c41cdbd 100644 --- a/src/librustc_ty/instance.rs +++ b/src/librustc_ty/instance.rs @@ -1,11 +1,11 @@ use rustc_hir::def_id::DefId; +use rustc_infer::infer::TyCtxtInferExt; use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::{self, Instance, TyCtxt, TypeFoldable}; -use rustc_infer::infer::TyCtxtInferExt; use rustc_span::sym; use rustc_target::spec::abi::Abi; use rustc_trait_selection::traits; -use traits::{Reveal, translate_substs}; +use traits::{translate_substs, Reveal}; use log::debug;