Skip to content

Commit

Permalink
Avoid ICEs when we emit errors constructing the specialization graph
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewjasper committed Mar 15, 2020
1 parent c24b4bf commit 32d330d
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 56 deletions.
29 changes: 19 additions & 10 deletions src/librustc/traits/specialization_graph.rs
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)),
})
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc/ty/trait_def.rs
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
}
Expand Down
13 changes: 7 additions & 6 deletions src/librustc_metadata/rmeta/encoder.rs
Expand Up @@ -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
};
Expand Down
24 changes: 14 additions & 10 deletions src/librustc_trait_selection/traits/project.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
38 changes: 21 additions & 17 deletions src/librustc_trait_selection/traits/specialize/mod.rs
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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));
}
Expand Down
28 changes: 16 additions & 12 deletions src/librustc_typeck/check/mod.rs
Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
}
}
}
}
Expand Down

0 comments on commit 32d330d

Please sign in to comment.