Skip to content

Commit

Permalink
Propagate lifetime resolution errors into tcx.type_of
Browse files Browse the repository at this point in the history
Fixes rust-lang#69136

Previously, lifetime resolution errors would cause an error to be
emitted, but would not mark the parent type as 'tainted' in any way.
We usually abort compilation before this becomes an issue - however,
opaque types can cause us to type-check function bodies before such an
abort occurs. Ths can result in trying to instantiate opaque types that
have invalid computed generics. Currently, this only causes issues for
nested opaque types, but there's no reason to expect the computed
generics to be sane when we had unresolved lifetimes (which can result
in extra lifetime parameters getting added to the generics).

This commit tracks 'unresolved lifetime' errors that occur during
lifetime resolution. When we type-check an item, we bail out and return
`tcx.types.err` if a lifetime error was reported for that type. This
causes us to skip type-checking of types affected by the lifetime error,
while still checking unrelated types.

Additionally, we now check for errors in 'parent' opaque types (if such
a 'parent' exists) when collecting constraints for opaque types. This
reflects the fact that opaque types inherit generics from 'parent'
opaque types - if an error ocurred while type-checking the parent,
we don't attempt to type-check the child.
  • Loading branch information
Aaron1011 committed Feb 17, 2020
1 parent 75b98fb commit 50a3c38
Show file tree
Hide file tree
Showing 9 changed files with 122 additions and 126 deletions.
4 changes: 4 additions & 0 deletions src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,8 @@ pub struct ResolveLifetimes {
/// to the trait object lifetime defaults computed from them.
pub object_lifetime_defaults:
FxHashMap<LocalDefId, FxHashMap<ItemLocalId, Vec<ObjectLifetimeDefault>>>,

/// Contains the ids all HIR items for which we encountered an error
/// when resolving lifetimes.
pub has_lifetime_error: FxHashSet<LocalDefId>,
}
8 changes: 8 additions & 0 deletions src/librustc/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,14 @@ rustc_queries! {
-> Option<&'tcx FxHashMap<ItemLocalId, Vec<ObjectLifetimeDefault>>> {
desc { "looking up lifetime defaults for a region" }
}
/// Determiens if any eccors occured when resolving lifetimes
/// for the specified item. A return value of 'true' means
/// that compilation will eventually fail, and it's safe
/// to create and propagate a TyKind::Error or skip
/// running checks on the affected item.
query has_lifetime_error(_: DefIndex) -> bool {
desc { "determining if a lifetime error was emitted" }
}
}

TypeChecking {
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_resolve/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1507,7 +1507,8 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
)
}

crate fn emit_undeclared_lifetime_error(&self, lifetime_ref: &hir::Lifetime) {
crate fn emit_undeclared_lifetime_error(&mut self, lifetime_ref: &hir::Lifetime) {
self.map.has_lifetime_error.insert(lifetime_ref.hir_id.owner_local_def_id());
let mut err = struct_span_err!(
self.tcx.sess,
lifetime_ref.span,
Expand Down
15 changes: 13 additions & 2 deletions src/librustc_resolve/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ impl RegionExt for Region {
/// actual use. It has the same data, but indexed by `DefIndex`. This
/// is silly.
#[derive(Default)]
struct NamedRegionMap {
crate struct NamedRegionMap {
// maps from every use of a named (not anonymous) lifetime to a
// `Region` describing how that region is bound
defs: HirIdMap<Region>,
Expand All @@ -149,11 +149,15 @@ struct NamedRegionMap {
// For each type and trait definition, maps type parameters
// to the trait object lifetime defaults computed from them.
object_lifetime_defaults: HirIdMap<Vec<ObjectLifetimeDefault>>,

/// Contains the ids all HIR items for which we encountered an error
/// when resolving lifetimes.
crate has_lifetime_error: FxHashSet<LocalDefId>,
}

crate struct LifetimeContext<'a, 'tcx> {
crate tcx: TyCtxt<'tcx>,
map: &'a mut NamedRegionMap,
crate map: &'a mut NamedRegionMap,
scope: ScopeRef<'a>,

/// This is slightly complicated. Our representation for poly-trait-refs contains a single
Expand Down Expand Up @@ -295,6 +299,11 @@ pub fn provide(providers: &mut ty::query::Providers<'_>) {
tcx.resolve_lifetimes(LOCAL_CRATE).object_lifetime_defaults.get(&id)
},

has_lifetime_error: |tcx, id| {
let id = LocalDefId::from_def_id(DefId::local(id)); // (*)
tcx.resolve_lifetimes(LOCAL_CRATE).has_lifetime_error.contains(&id)
},

..*providers
};

Expand Down Expand Up @@ -324,6 +333,7 @@ fn resolve_lifetimes(tcx: TyCtxt<'_>, for_krate: CrateNum) -> &ResolveLifetimes
let map = rl.object_lifetime_defaults.entry(hir_id.owner_local_def_id()).or_default();
map.insert(hir_id.local_id, v);
}
rl.has_lifetime_error = named_region_map.has_lifetime_error;

tcx.arena.alloc(rl)
}
Expand All @@ -334,6 +344,7 @@ fn krate(tcx: TyCtxt<'_>) -> NamedRegionMap {
defs: Default::default(),
late_bound: Default::default(),
object_lifetime_defaults: compute_object_lifetime_defaults(tcx),
has_lifetime_error: Default::default(),
};
{
let mut visitor = LifetimeContext {
Expand Down
29 changes: 28 additions & 1 deletion src/librustc_typeck/collect/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit;
use rustc_hir::intravisit::Visitor;
use rustc_hir::Node;
use rustc_infer::traits;
use rustc_hir::{ItemKind, Node, CRATE_HIR_ID};
use rustc_span::symbol::{sym, Ident};
use rustc_span::{Span, DUMMY_SP};

Expand All @@ -23,6 +23,13 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {

let hir_id = tcx.hir().as_local_hir_id(def_id).unwrap();

// We just checked that `DefId` is crate-local
if tcx.has_lifetime_error(def_id.index) {
debug!("type_of: item {:?} has a lifetime-related error, bailing out", def_id);
assert!(tcx.sess.has_errors()); // Sanity check: compilation should be going to fail
return tcx.types.err;
}

let icx = ItemCtxt::new(tcx, def_id);

match tcx.hir().get(hir_id) {
Expand Down Expand Up @@ -358,6 +365,26 @@ fn find_opaque_ty_constraints(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {

debug!("find_opaque_ty_constraints({:?})", def_id);

// It's possible for one opaque type to be nested inside another
// (e.g. `impl Foo<MyType = impl Bar<A>>`)
// We don't explicitly depend on the type of the parent - however,
// we do depend on its generics. We propagate any errors that occur
// during type-checking of the parent, to ensure that we don't create
// an invalid 'child' opaque type.
let hir_id = tcx.hir().as_local_hir_id(def_id).unwrap();
let parent_id = tcx.hir().get_parent_item(hir_id);
// Type-alias-impl-trait types may have no parent
if parent_id != hir_id && parent_id != CRATE_HIR_ID {
// We only care about checking the parent if it's an opaque type
if let Node::Item(hir::Item { kind: ItemKind::OpaqueTy(..), .. }) = tcx.hir().get(parent_id)
{
if tcx.type_of(tcx.hir().local_def_id(parent_id)) == tcx.types.err {
debug!("find_opaque_ty_constraints: parent opaque type has error, bailing out");
return tcx.types.err;
}
}
}

struct ConstraintLocator<'tcx> {
tcx: TyCtxt<'tcx>,
def_id: DefId,
Expand Down
12 changes: 0 additions & 12 deletions src/test/ui/associated-type-bounds/duplicate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,33 +108,21 @@ type TAW3<T> where T: Iterator<Item: 'static, Item: 'static> = T;
type ETAI1<T: Iterator<Item: Copy, Item: Send>> = impl Copy;
//~^ ERROR the value of the associated type `Item` (from trait `std::iter::Iterator`) is already specified [E0719]
//~| ERROR could not find defining uses
//~| ERROR could not find defining uses
//~| ERROR could not find defining uses
type ETAI2<T: Iterator<Item: Copy, Item: Copy>> = impl Copy;
//~^ ERROR the value of the associated type `Item` (from trait `std::iter::Iterator`) is already specified [E0719]
//~| ERROR could not find defining uses
//~| ERROR could not find defining uses
//~| ERROR could not find defining uses
type ETAI3<T: Iterator<Item: 'static, Item: 'static>> = impl Copy;
//~^ ERROR the value of the associated type `Item` (from trait `std::iter::Iterator`) is already specified [E0719]
//~| ERROR could not find defining uses
//~| ERROR could not find defining uses
//~| ERROR could not find defining uses
type ETAI4 = impl Iterator<Item: Copy, Item: Send>;
//~^ ERROR the value of the associated type `Item` (from trait `std::iter::Iterator`) is already specified [E0719]
//~| ERROR could not find defining uses
//~| ERROR could not find defining uses
//~| ERROR could not find defining uses
type ETAI5 = impl Iterator<Item: Copy, Item: Copy>;
//~^ ERROR the value of the associated type `Item` (from trait `std::iter::Iterator`) is already specified [E0719]
//~| ERROR could not find defining uses
//~| ERROR could not find defining uses
//~| ERROR could not find defining uses
type ETAI6 = impl Iterator<Item: 'static, Item: 'static>;
//~^ ERROR the value of the associated type `Item` (from trait `std::iter::Iterator`) is already specified [E0719]
//~| ERROR could not find defining uses
//~| ERROR could not find defining uses
//~| ERROR could not find defining uses

trait TRI1<T: Iterator<Item: Copy, Item: Send>> {}
//~^ ERROR the value of the associated type `Item` (from trait `std::iter::Iterator`) is already specified [E0719]
Expand Down
Loading

0 comments on commit 50a3c38

Please sign in to comment.