From 32d330df30c079c8d0bcdeff9049152a65c36b31 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 8 Feb 2020 20:10:06 +0000 Subject: [PATCH] Avoid ICEs when we emit errors constructing the specialization graph --- src/librustc/traits/specialization_graph.rs | 29 +++++++++----- src/librustc/ty/trait_def.rs | 3 +- src/librustc_metadata/rmeta/encoder.rs | 13 ++++--- .../traits/project.rs | 24 +++++++----- .../traits/specialize/mod.rs | 38 ++++++++++--------- src/librustc_typeck/check/mod.rs | 26 +++++++------ 6 files changed, 78 insertions(+), 55 deletions(-) diff --git a/src/librustc/traits/specialization_graph.rs b/src/librustc/traits/specialization_graph.rs index d481e578fc1..1847326a742 100644 --- a/src/librustc/traits/specialization_graph.rs +++ b/src/librustc/traits/specialization_graph.rs @@ -4,6 +4,7 @@ use crate::ty::{self, TyCtxt}; use rustc_ast::ast::Ident; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; +use rustc_errors::ErrorReported; use rustc_hir::def_id::{DefId, DefIdMap}; /// A per-trait graph of impls in specialization order. At the moment, this @@ -23,17 +24,20 @@ use rustc_hir::def_id::{DefId, DefIdMap}; /// has at most one parent. #[derive(RustcEncodable, RustcDecodable, HashStable)] pub struct Graph { - // All impls have a parent; the "root" impls have as their parent the `def_id` - // of the trait. + /// All impls have a parent; the "root" impls have as their parent the `def_id` + /// of the trait. pub parent: DefIdMap, - // The "root" impls are found by looking up the trait's def_id. + /// The "root" impls are found by looking up the trait's def_id. pub children: DefIdMap, + + /// Whether an error was emitted while constructing the graph. + pub has_errored: bool, } impl Graph { pub fn new() -> Graph { - Graph { parent: Default::default(), children: Default::default() } + Graph { parent: Default::default(), children: Default::default(), has_errored: false } } /// The parent of a given impl, which is the `DefId` of the trait when the @@ -179,17 +183,22 @@ impl<'tcx> Ancestors<'tcx> { } /// Walk up the specialization ancestors of a given impl, starting with that -/// impl itself. +/// impl itself. Returns `None` if an error was reported while building the +/// specialization graph. pub fn ancestors( tcx: TyCtxt<'tcx>, trait_def_id: DefId, start_from_impl: DefId, -) -> Ancestors<'tcx> { +) -> Result, ErrorReported> { let specialization_graph = tcx.specialization_graph_of(trait_def_id); - Ancestors { - trait_def_id, - specialization_graph, - current_source: Some(Node::Impl(start_from_impl)), + if specialization_graph.has_errored { + Err(ErrorReported) + } else { + Ok(Ancestors { + trait_def_id, + specialization_graph, + current_source: Some(Node::Impl(start_from_impl)), + }) } } diff --git a/src/librustc/ty/trait_def.rs b/src/librustc/ty/trait_def.rs index 2b5d72de358..948f1b8501d 100644 --- a/src/librustc/ty/trait_def.rs +++ b/src/librustc/ty/trait_def.rs @@ -9,6 +9,7 @@ use rustc_hir::def_id::DefId; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; +use rustc_errors::ErrorReported; use rustc_macros::HashStable; /// A trait's definition with type information. @@ -92,7 +93,7 @@ impl<'tcx> TraitDef { &self, tcx: TyCtxt<'tcx>, of_impl: DefId, - ) -> specialization_graph::Ancestors<'tcx> { + ) -> Result, ErrorReported> { specialization_graph::ancestors(tcx, self.def_id, of_impl) } } diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index 1d8b2c0f6f5..3f079b286b4 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -1077,12 +1077,13 @@ impl EncodeContext<'tcx> { let polarity = self.tcx.impl_polarity(def_id); let parent = if let Some(trait_ref) = trait_ref { let trait_def = self.tcx.trait_def(trait_ref.def_id); - trait_def.ancestors(self.tcx, def_id).nth(1).and_then(|node| { - match node { - specialization_graph::Node::Impl(parent) => Some(parent), - _ => None, - } - }) + trait_def.ancestors(self.tcx, def_id).ok() + .and_then(|mut an| an.nth(1).and_then(|node| { + match node { + specialization_graph::Node::Impl(parent) => Some(parent), + _ => None, + } + })) } else { None }; diff --git a/src/librustc_trait_selection/traits/project.rs b/src/librustc_trait_selection/traits/project.rs index dde78aa4357..1ad91574212 100644 --- a/src/librustc_trait_selection/traits/project.rs +++ b/src/librustc_trait_selection/traits/project.rs @@ -21,6 +21,7 @@ use rustc::ty::fold::{TypeFoldable, TypeFolder}; use rustc::ty::subst::{InternalSubsts, Subst}; use rustc::ty::{self, ToPolyTraitRef, ToPredicate, Ty, TyCtxt, WithConstness}; use rustc_ast::ast::Ident; +use rustc_errors::ErrorReported; use rustc_hir::def_id::DefId; use rustc_span::symbol::sym; use rustc_span::DUMMY_SP; @@ -1010,7 +1011,8 @@ fn assemble_candidates_from_impls<'cx, 'tcx>( // NOTE: This should be kept in sync with the similar code in // `rustc::ty::instance::resolve_associated_item()`. let node_item = - assoc_ty_def(selcx, impl_data.impl_def_id, obligation.predicate.item_def_id); + 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` @@ -1405,7 +1407,10 @@ fn confirm_impl_candidate<'cx, 'tcx>( let trait_def_id = tcx.trait_id_of_impl(impl_def_id).unwrap(); let param_env = obligation.param_env; - let assoc_ty = assoc_ty_def(selcx, impl_def_id, assoc_item_id); + let assoc_ty = match assoc_ty_def(selcx, impl_def_id, assoc_item_id) { + Ok(assoc_ty) => assoc_ty, + Err(ErrorReported) => return Progress { ty: tcx.types.err, obligations: nested }, + }; if !assoc_ty.item.defaultness.has_value() { // This means that the impl is missing a definition for the @@ -1444,14 +1449,14 @@ fn assoc_ty_def( selcx: &SelectionContext<'_, '_>, impl_def_id: DefId, assoc_ty_def_id: DefId, -) -> specialization_graph::NodeItem { +) -> Result, ErrorReported> { 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; let trait_def = tcx.trait_def(trait_def_id); // This function may be called while we are still building the - // specialization graph that is queried below (via TraidDef::ancestors()), + // specialization graph that is queried below (via TraitDef::ancestors()), // so, in order to avoid unnecessary infinite recursion, we manually look // for the associated item at the given impl. // If there is no such item in that impl, this function will fail with a @@ -1461,17 +1466,16 @@ 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 specialization_graph::NodeItem { + return Ok(specialization_graph::NodeItem { node: specialization_graph::Node::Impl(impl_def_id), item: *item, - }; + }); } } - if let Some(assoc_item) = - trait_def.ancestors(tcx, impl_def_id).leaf_def(tcx, assoc_ty_name, ty::AssocKind::Type) - { - assoc_item + let ancestors = trait_def.ancestors(tcx, impl_def_id)?; + if let Some(assoc_item) = ancestors.leaf_def(tcx, assoc_ty_name, ty::AssocKind::Type) { + Ok(assoc_item) } else { // This is saying that neither the trait nor // the impl contain a definition for this diff --git a/src/librustc_trait_selection/traits/specialize/mod.rs b/src/librustc_trait_selection/traits/specialize/mod.rs index 770253e635e..b763851b86e 100644 --- a/src/librustc_trait_selection/traits/specialize/mod.rs +++ b/src/librustc_trait_selection/traits/specialize/mod.rs @@ -130,24 +130,27 @@ pub fn find_associated_item<'tcx>( 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 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.node, - ); - infcx.tcx.erase_regions(&substs) - }); - (node_item.item.def_id, substs) + 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.node, + ); + infcx.tcx.erase_regions(&substs) + }); + (node_item.item.def_id, substs) + } + None => bug!("{:?} not found in {:?}", item, impl_data.impl_def_id), } - None => bug!("{:?} not found in {:?}", item, impl_data.impl_def_id), + } else { + (item.def_id, substs) } } @@ -382,6 +385,7 @@ pub(super) fn specialization_graph_provider( match used_to_be_allowed { None => { + sg.has_errored = true; let err = struct_span_err!(tcx.sess, impl_span, E0119, ""); decorate(LintDiagnosticBuilder::new(err)); } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 1975b248999..9e20dcec1af 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1901,8 +1901,11 @@ fn check_specialization_validity<'tcx>( hir::ImplItemKind::TyAlias(_) => ty::AssocKind::Type, }; - let mut ancestor_impls = trait_def - .ancestors(tcx, impl_id) + let ancestors = match trait_def.ancestors(tcx, impl_id) { + Ok(ancestors) => ancestors, + Err(_) => return, + }; + let mut ancestor_impls = ancestors .skip(1) .filter_map(|parent| { if parent.is_from_trait() { @@ -2083,16 +2086,17 @@ fn check_impl_items_against_trait<'tcx>( // Check for missing items from trait let mut missing_items = Vec::new(); - for trait_item in tcx.associated_items(impl_trait_ref.def_id).in_definition_order() { - let is_implemented = trait_def - .ancestors(tcx, impl_id) - .leaf_def(tcx, trait_item.ident, trait_item.kind) - .map(|node_item| !node_item.node.is_from_trait()) - .unwrap_or(false); + if let Ok(ancestors) = trait_def.ancestors(tcx, impl_id) { + 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()) + .unwrap_or(false); - if !is_implemented && !traits::impl_is_default(tcx, impl_id) { - if !trait_item.defaultness.has_value() { - missing_items.push(*trait_item); + if !is_implemented && !traits::impl_is_default(tcx, impl_id) { + if !trait_item.defaultness.has_value() { + missing_items.push(*trait_item); + } } } }