Avoid ICEs when we emit errors constructing the specialization graph

Author: matthewjasper

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<DefId>,
 
-    // 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<Children>,
+
+    /// 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<Ancestors<'tcx>, 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)),
+        })
     }
 }
 

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<specialization_graph::Ancestors<'tcx>, ErrorReported> {
         specialization_graph::ancestors(tcx, self.def_id, of_impl)
     }
 }

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
                 };

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<ty::AssocItem> {
+) -> Result<specialization_graph::NodeItem<ty::AssocItem>, 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

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));
                     }

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 !is_implemented && !traits::impl_is_default(tcx, impl_id) {
-            if !trait_item.defaultness.has_value() {
-                missing_items.push(*trait_item);
+    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);
+                }
             }
         }
     }