Make def_key and HIR parenting consistent.

Author: cjgillot

compiler/rustc_ast_lowering/src/lib.rs

@@ -223,7 +223,7 @@ enum ImplTraitContext<'b, 'a> {
     /// equivalent to a fresh universal parameter like `fn foo<T: Debug>(x: T)`.
     ///
     /// Newly generated parameters should be inserted into the given `Vec`.
-    Universal(&'b mut Vec<hir::GenericParam<'a>>),
+    Universal(&'b mut Vec<hir::GenericParam<'a>>, LocalDefId),
 
     /// Treat `impl Trait` as shorthand for a new opaque type.
     /// Example: `fn foo() -> impl Debug`, where `impl Debug` is conceptually
@@ -278,7 +278,7 @@ impl<'a> ImplTraitContext<'_, 'a> {
     fn reborrow<'this>(&'this mut self) -> ImplTraitContext<'this, 'a> {
         use self::ImplTraitContext::*;
         match self {
-            Universal(params) => Universal(params),
+            Universal(params, parent) => Universal(params, *parent),
             ReturnPositionOpaqueTy { fn_def_id, origin } => {
                 ReturnPositionOpaqueTy { fn_def_id: *fn_def_id, origin: *origin }
             }
@@ -475,25 +475,18 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         }
 
         impl MiscCollector<'_, '_, '_> {
-            fn allocate_use_tree_hir_id_counters(&mut self, tree: &UseTree, owner: LocalDefId) {
+            fn allocate_use_tree_hir_id_counters(&mut self, tree: &UseTree) {
                 match tree.kind {
                     UseTreeKind::Simple(_, id1, id2) => {
                         for &id in &[id1, id2] {
-                            self.lctx.resolver.create_def(
-                                owner,
-                                id,
-                                DefPathData::Misc,
-                                ExpnId::root(),
-                                tree.prefix.span,
-                            );
                             self.lctx.allocate_hir_id_counter(id);
                         }
                     }
                     UseTreeKind::Glob => (),
                     UseTreeKind::Nested(ref trees) => {
                         for &(ref use_tree, id) in trees {
-                            let hir_id = self.lctx.allocate_hir_id_counter(id);
-                            self.allocate_use_tree_hir_id_counters(use_tree, hir_id.owner);
+                            self.lctx.allocate_hir_id_counter(id);
+                            self.allocate_use_tree_hir_id_counters(use_tree);
                         }
                     }
                 }
@@ -502,7 +495,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
 
         impl<'tcx> Visitor<'tcx> for MiscCollector<'tcx, '_, '_> {
             fn visit_item(&mut self, item: &'tcx Item) {
-                let hir_id = self.lctx.allocate_hir_id_counter(item.id);
+                self.lctx.allocate_hir_id_counter(item.id);
 
                 match item.kind {
                     ItemKind::Struct(_, ref generics)
@@ -521,7 +514,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                         self.lctx.type_def_lifetime_params.insert(def_id.to_def_id(), count);
                     }
                     ItemKind::Use(ref use_tree) => {
-                        self.allocate_use_tree_hir_id_counters(use_tree, hir_id.owner);
+                        self.allocate_use_tree_hir_id_counters(use_tree);
                     }
                     _ => {}
                 }
@@ -939,8 +932,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                     // `lifetimes_to_define`. If we swapped the order of these two,
                     // in-band-lifetimes introduced by generics or where-clauses
                     // wouldn't have been added yet.
-                    let generics =
-                        this.lower_generics_mut(generics, ImplTraitContext::Universal(&mut params));
+                    let generics = this.lower_generics_mut(
+                        generics,
+                        ImplTraitContext::Universal(
+                            &mut params,
+                            this.current_hir_id_owner.last().unwrap().0,
+                        ),
+                    );
                     let res = f(this, &mut params);
                     (params, (generics, res))
                 })
@@ -1145,6 +1143,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             }
             AssocTyConstraintKind::Bound { ref bounds } => {
                 let mut capturable_lifetimes;
+                let mut parent_def_id = self.current_hir_id_owner.last().unwrap().0;
                 // Piggy-back on the `impl Trait` context to figure out the correct behavior.
                 let (desugar_to_impl_trait, itctx) = match itctx {
                     // We are in the return position:
@@ -1164,7 +1163,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                     // so desugar to
                     //
                     //     fn foo(x: dyn Iterator<Item = impl Debug>)
-                    ImplTraitContext::Universal(..) if self.is_in_dyn_type => (true, itctx),
+                    ImplTraitContext::Universal(_, parent) if self.is_in_dyn_type => {
+                        parent_def_id = parent;
+                        (true, itctx)
+                    }
 
                     // In `type Foo = dyn Iterator<Item: Debug>` we desugar to
                     // `type Foo = dyn Iterator<Item = impl Debug>` but we have to override the
@@ -1198,7 +1200,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                     // constructing the HIR for `impl bounds...` and then lowering that.
 
                     let impl_trait_node_id = self.resolver.next_node_id();
-                    let parent_def_id = self.current_hir_id_owner.last().unwrap().0;
                     self.resolver.create_def(
                         parent_def_id,
                         impl_trait_node_id,
@@ -1451,7 +1452,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                             |this| this.lower_param_bounds(bounds, nested_itctx),
                         )
                     }
-                    ImplTraitContext::Universal(in_band_ty_params) => {
+                    ImplTraitContext::Universal(in_band_ty_params, parent_def_id) => {
                         // Add a definition for the in-band `Param`.
                         let def_id = self.resolver.local_def_id(def_node_id);
 
@@ -1460,7 +1461,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                         let hir_bounds = self.with_hir_id_owner(def_node_id, |this| {
                             this.lower_param_bounds(
                                 bounds,
-                                ImplTraitContext::Universal(in_band_ty_params),
+                                ImplTraitContext::Universal(in_band_ty_params, parent_def_id),
                             )
                         });
                         // Set the name to `impl Bound1 + Bound2`.
@@ -1891,7 +1892,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             }
             this.arena.alloc_from_iter(inputs.iter().map(|param| {
                 if let Some((_, ibty)) = &mut in_band_ty_params {
-                    this.lower_ty_direct(&param.ty, ImplTraitContext::Universal(ibty))
+                    this.lower_ty_direct(
+                        &param.ty,
+                        ImplTraitContext::Universal(
+                            ibty,
+                            this.current_hir_id_owner.last().unwrap().0,
+                        ),
+                    )
                 } else {
                     this.lower_ty_direct(&param.ty, ImplTraitContext::disallowed())
                 }

compiler/rustc_middle/src/hir/map/collector.rs

@@ -221,6 +221,18 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
 
             data.signature =
                 Some(self.arena.alloc(Owner { parent: entry.parent, node: entry.node }));
+
+            let dk_parent = self.definitions.def_key(id.owner).parent;
+            if let Some(dk_parent) = dk_parent {
+                let dk_parent = LocalDefId { local_def_index: dk_parent };
+                let dk_parent = self.definitions.local_def_id_to_hir_id(dk_parent);
+                if dk_parent.owner != entry.parent.owner {
+                    panic!(
+                        "Different parents for {:?} => dk_parent={:?} actual={:?}",
+                        id.owner, dk_parent, entry.parent,
+                    )
+                }
+            }
         } else {
             assert_eq!(entry.parent.owner, id.owner);
             insert_vec_map(

compiler/rustc_resolve/src/def_collector.rs

@@ -1,4 +1,4 @@
-use crate::Resolver;
+use crate::{ImplTraitContext, Resolver};
 use rustc_ast::visit::{self, FnKind};
 use rustc_ast::walk_list;
 use rustc_ast::*;
@@ -16,14 +16,15 @@ crate fn collect_definitions(
     fragment: &AstFragment,
     expansion: ExpnId,
 ) {
-    let parent_def = resolver.invocation_parents[&expansion];
-    fragment.visit_with(&mut DefCollector { resolver, parent_def, expansion });
+    let (parent_def, impl_trait_context) = resolver.invocation_parents[&expansion];
+    fragment.visit_with(&mut DefCollector { resolver, parent_def, expansion, impl_trait_context });
 }
 
 /// Creates `DefId`s for nodes in the AST.
 struct DefCollector<'a, 'b> {
     resolver: &'a mut Resolver<'b>,
     parent_def: LocalDefId,
+    impl_trait_context: ImplTraitContext,
     expansion: ExpnId,
 }
 
@@ -40,6 +41,16 @@ impl<'a, 'b> DefCollector<'a, 'b> {
         self.parent_def = orig_parent_def;
     }
 
+    fn with_impl_trait<F: FnOnce(&mut Self)>(
+        &mut self,
+        impl_trait_context: ImplTraitContext,
+        f: F,
+    ) {
+        let orig_itc = std::mem::replace(&mut self.impl_trait_context, impl_trait_context);
+        f(self);
+        self.impl_trait_context = orig_itc;
+    }
+
     fn collect_field(&mut self, field: &'a StructField, index: Option<usize>) {
         let index = |this: &Self| {
             index.unwrap_or_else(|| {
@@ -60,8 +71,9 @@ impl<'a, 'b> DefCollector<'a, 'b> {
     }
 
     fn visit_macro_invoc(&mut self, id: NodeId) {
+        let id = id.placeholder_to_expn_id();
         let old_parent =
-            self.resolver.invocation_parents.insert(id.placeholder_to_expn_id(), self.parent_def);
+            self.resolver.invocation_parents.insert(id, (self.parent_def, self.impl_trait_context));
         assert!(old_parent.is_none(), "parent `LocalDefId` is reset for an invocation");
     }
 }
@@ -103,29 +115,37 @@ impl<'a, 'b> visit::Visitor<'a> for DefCollector<'a, 'b> {
         let def = self.create_def(i.id, def_data, i.span);
 
         self.with_parent(def, |this| {
-            match i.kind {
-                ItemKind::Struct(ref struct_def, _) | ItemKind::Union(ref struct_def, _) => {
-                    // If this is a unit or tuple-like struct, register the constructor.
-                    if let Some(ctor_hir_id) = struct_def.ctor_id() {
-                        this.create_def(ctor_hir_id, DefPathData::Ctor, i.span);
+            this.with_impl_trait(ImplTraitContext::Existential, |this| {
+                match i.kind {
+                    ItemKind::Struct(ref struct_def, _) | ItemKind::Union(ref struct_def, _) => {
+                        // If this is a unit or tuple-like struct, register the constructor.
+                        if let Some(ctor_hir_id) = struct_def.ctor_id() {
+                            this.create_def(ctor_hir_id, DefPathData::Ctor, i.span);
+                        }
                     }
+                    _ => {}
                 }
-                _ => {}
-            }
-            visit::walk_item(this, i);
+                visit::walk_item(this, i);
+            })
         });
     }
 
     fn visit_fn(&mut self, fn_kind: FnKind<'a>, span: Span, _: NodeId) {
         if let FnKind::Fn(_, _, sig, _, body) = fn_kind {
             if let Async::Yes { closure_id, return_impl_trait_id, .. } = sig.header.asyncness {
-                self.create_def(return_impl_trait_id, DefPathData::ImplTrait, span);
+                let return_impl_trait_id =
+                    self.create_def(return_impl_trait_id, DefPathData::ImplTrait, span);
 
                 // For async functions, we need to create their inner defs inside of a
                 // closure to match their desugared representation. Besides that,
                 // we must mirror everything that `visit::walk_fn` below does.
                 self.visit_fn_header(&sig.header);
-                visit::walk_fn_decl(self, &sig.decl);
+                for param in &sig.decl.inputs {
+                    self.visit_param(param);
+                }
+                self.with_parent(return_impl_trait_id, |this| {
+                    this.visit_fn_ret_ty(&sig.decl.output)
+                });
                 let closure_def = self.create_def(closure_id, DefPathData::ClosureExpr, span);
                 self.with_parent(closure_def, |this| walk_list!(this, visit_block, body));
                 return;
@@ -137,6 +157,14 @@ impl<'a, 'b> visit::Visitor<'a> for DefCollector<'a, 'b> {
 
     fn visit_use_tree(&mut self, use_tree: &'a UseTree, id: NodeId, _nested: bool) {
         self.create_def(id, DefPathData::Misc, use_tree.span);
+        match use_tree.kind {
+            UseTreeKind::Simple(_, id1, id2) => {
+                self.create_def(id1, DefPathData::Misc, use_tree.prefix.span);
+                self.create_def(id2, DefPathData::Misc, use_tree.prefix.span);
+            }
+            UseTreeKind::Glob => (),
+            UseTreeKind::Nested(..) => {}
+        }
         visit::walk_use_tree(self, use_tree, id);
     }
 
@@ -191,7 +219,15 @@ impl<'a, 'b> visit::Visitor<'a> for DefCollector<'a, 'b> {
         };
         self.create_def(param.id, def_path_data, param.ident.span);
 
-        visit::walk_generic_param(self, param);
+        // impl-Trait can happen inside generic parameters, like
+        // ```
+        // fn foo<U: Iterator<Item = impl Clone>>() {}
+        // ```
+        //
+        // In that case, the impl-trait is lowered as an additional generic parameter.
+        self.with_impl_trait(ImplTraitContext::Universal(self.parent_def), |this| {
+            visit::walk_generic_param(this, param)
+        });
     }
 
     fn visit_assoc_item(&mut self, i: &'a AssocItem, ctxt: visit::AssocCtxt) {
@@ -244,8 +280,19 @@ impl<'a, 'b> visit::Visitor<'a> for DefCollector<'a, 'b> {
         match ty.kind {
             TyKind::MacCall(..) => self.visit_macro_invoc(ty.id),
             TyKind::ImplTrait(node_id, _) => {
-                let parent_def = self.create_def(node_id, DefPathData::ImplTrait, ty.span);
-                self.with_parent(parent_def, |this| visit::walk_ty(this, ty));
+                let parent_def = match self.impl_trait_context {
+                    ImplTraitContext::Universal(item_def) => self.resolver.create_def(
+                        item_def,
+                        node_id,
+                        DefPathData::ImplTrait,
+                        self.expansion,
+                        ty.span,
+                    ),
+                    ImplTraitContext::Existential => {
+                        self.create_def(node_id, DefPathData::ImplTrait, ty.span)
+                    }
+                };
+                self.with_parent(parent_def, |this| visit::walk_ty(this, ty))
             }
             _ => visit::walk_ty(self, ty),
         }
@@ -275,7 +322,13 @@ impl<'a, 'b> visit::Visitor<'a> for DefCollector<'a, 'b> {
     }
 
     fn visit_param(&mut self, p: &'a Param) {
-        if p.is_placeholder { self.visit_macro_invoc(p.id) } else { visit::walk_param(self, p) }
+        if p.is_placeholder {
+            self.visit_macro_invoc(p.id)
+        } else {
+            self.with_impl_trait(ImplTraitContext::Universal(self.parent_def), |this| {
+                visit::walk_param(this, p)
+            })
+        }
     }
 
     // This method is called only when we are visiting an individual field

compiler/rustc_resolve/src/lib.rs

@@ -156,6 +156,12 @@ impl<'a> ParentScope<'a> {
     }
 }
 
+#[derive(Copy, Debug, Clone)]
+enum ImplTraitContext {
+    Existential,
+    Universal(LocalDefId),
+}
+
 #[derive(Eq)]
 struct BindingError {
     name: Symbol,
@@ -989,8 +995,9 @@ pub struct Resolver<'a> {
     /// Indices of unnamed struct or variant fields with unresolved attributes.
     placeholder_field_indices: FxHashMap<NodeId, usize>,
     /// When collecting definitions from an AST fragment produced by a macro invocation `ExpnId`
-    /// we know what parent node that fragment should be attached to thanks to this table.
-    invocation_parents: FxHashMap<ExpnId, LocalDefId>,
+    /// we know what parent node that fragment should be attached to thanks to this table,
+    /// and how the `impl Trait` fragments were introduced.
+    invocation_parents: FxHashMap<ExpnId, (LocalDefId, ImplTraitContext)>,
 
     next_disambiguator: FxHashMap<(LocalDefId, DefPathData), u32>,
     /// Some way to know that we are in a *trait* impl in `visit_assoc_item`.
@@ -1205,7 +1212,7 @@ impl<'a> Resolver<'a> {
         node_id_to_def_id.insert(CRATE_NODE_ID, root);
 
         let mut invocation_parents = FxHashMap::default();
-        invocation_parents.insert(ExpnId::root(), root);
+        invocation_parents.insert(ExpnId::root(), (root, ImplTraitContext::Existential));
 
         let mut extern_prelude: FxHashMap<Ident, ExternPreludeEntry<'_>> = session
             .opts

compiler/rustc_resolve/src/macros.rs

@@ -326,7 +326,7 @@ impl<'a> ResolverExpand for Resolver<'a> {
         // nearest closing item - we should try to return the closest parent of the ExpnId
         self.invocation_parents
             .get(&expn_id)
-            .map_or(ast::CRATE_NODE_ID, |id| self.def_id_to_node_id[*id])
+            .map_or(ast::CRATE_NODE_ID, |id| self.def_id_to_node_id[id.0])
     }
 
     fn has_derive_copy(&self, expn_id: ExpnId) -> bool {

src/test/ui/consts/miri_unleashed/tls.stderr

@@ -2,13 +2,13 @@ error[E0080]: could not evaluate static initializer
   --> $DIR/tls.rs:12:25
    |
 LL |     unsafe { let _val = A; }
-   |                         ^ cannot access thread local static (DefId(0:4 ~ tls[317d]::A))
+   |                         ^ cannot access thread local static (DefId(0:6 ~ tls[317d]::A))
 
 error[E0080]: could not evaluate static initializer
   --> $DIR/tls.rs:19:26
    |
 LL |     unsafe { let _val = &A; }
-   |                          ^ cannot access thread local static (DefId(0:4 ~ tls[317d]::A))
+   |                          ^ cannot access thread local static (DefId(0:6 ~ tls[317d]::A))
 
 warning: skipping const checks
    |