Auto merge of #153955 - aerooneqq:def-path-hash-collision, r=petrochenkov

delegation: fix def path hash collision, add per parent disambiguators



This PR addresses the following delegation issues:
- It fixes #153410 when generating new `DefId`s for generic parameters by ~saving `DisambiguatorState`s from resolve stage and using them at AST -> HIR lowering~ introducing per owner disambiguators and transferring them to AST -> HIR lowering stage
- ~Next it fixes the ICE which is connected to using `DUMMY_SP` in delegation code, which was found during previous fix~
- ~Finally, after those fixes the #143498 is also fixed, only bugs with propagating synthetic generic params are left.~

Fixes #153410. Part of #118212.

r? @petrochenkov

Author: bors

compiler/rustc_ast_lowering/src/delegation/generics.rs

@@ -295,10 +295,9 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
 
             let param_ident = Ident::new(p.name, span);
             let def_name = Some(param_ident.name);
-            let path_data = def_kind.def_path_data(def_name);
             let node_id = self.next_node_id();
 
-            let def_id = self.create_def(node_id, def_name, def_kind, path_data, span);
+            let def_id = self.create_def(node_id, def_name, def_kind, span);
 
             let kind = match p.kind {
                 GenericParamDefKind::Lifetime => {

compiler/rustc_ast_lowering/src/expr.rs

@@ -8,7 +8,6 @@ use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_errors::msg;
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
-use rustc_hir::definitions::DefPathData;
 use rustc_hir::{HirId, Target, find_attr};
 use rustc_middle::span_bug;
 use rustc_middle::ty::TyCtxt;
@@ -472,13 +471,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
         for (idx, arg) in args.iter().cloned().enumerate() {
             if legacy_args_idx.contains(&idx) {
                 let node_id = self.next_node_id();
-                self.create_def(
-                    node_id,
-                    None,
-                    DefKind::AnonConst,
-                    DefPathData::LateAnonConst,
-                    f.span,
-                );
+                self.create_def(node_id, None, DefKind::AnonConst, f.span);
                 let const_value =
                     if let ControlFlow::Break(span) = WillCreateDefIdsVisitor.visit_expr(&arg) {
                         Box::new(Expr {

compiler/rustc_ast_lowering/src/lib.rs

@@ -51,7 +51,7 @@ use rustc_data_structures::tagged_ptr::TaggedRef;
 use rustc_errors::{DiagArgFromDisplay, DiagCtxtHandle};
 use rustc_hir::def::{DefKind, LifetimeRes, Namespace, PartialRes, PerNS, Res};
 use rustc_hir::def_id::{CRATE_DEF_ID, LOCAL_CRATE, LocalDefId};
-use rustc_hir::definitions::{DefPathData, DisambiguatorState};
+use rustc_hir::definitions::PerParentDisambiguatorState;
 use rustc_hir::lints::{AttributeLint, DelayedLint};
 use rustc_hir::{
     self as hir, AngleBrackets, ConstArg, GenericArg, HirId, ItemLocalMap, LifetimeSource,
@@ -94,7 +94,7 @@ pub mod stability;
 struct LoweringContext<'a, 'hir, R> {
     tcx: TyCtxt<'hir>,
     resolver: &'a mut R,
-    disambiguator: DisambiguatorState,
+    disambiguator: PerParentDisambiguatorState,
 
     /// Used to allocate HIR nodes.
     arena: &'hir hir::Arena<'hir>,
@@ -159,7 +159,7 @@ impl<'a, 'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'a, 'hir, R> {
         Self {
             tcx,
             resolver,
-            disambiguator: DisambiguatorState::new(),
+            disambiguator: Default::default(),
             arena: tcx.hir_arena,
 
             // HirId handling.
@@ -302,6 +302,10 @@ impl<'a, 'tcx> ResolverAstLoweringExt<'tcx> for ResolverDelayedAstLowering<'a, '
     fn next_node_id(&mut self) -> NodeId {
         next_node_id(&mut self.next_node_id)
     }
+
+    fn steal_or_create_disambiguator(&self, parent: LocalDefId) -> PerParentDisambiguatorState {
+        self.base.steal_or_create_disambiguator(parent)
+    }
 }
 
 fn next_node_id(current_id: &mut NodeId) -> NodeId {
@@ -404,6 +408,10 @@ impl<'tcx> ResolverAstLowering<'tcx> {
     fn next_node_id(&mut self) -> NodeId {
         next_node_id(&mut self.next_node_id)
     }
+
+    fn steal_or_create_disambiguator(&self, parent: LocalDefId) -> PerParentDisambiguatorState {
+        self.per_parent_disambiguators.get(&parent).map(|s| s.steal()).unwrap_or_default()
+    }
 }
 
 /// How relaxed bounds `?Trait` should be treated.
@@ -717,7 +725,6 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
         node_id: ast::NodeId,
         name: Option<Symbol>,
         def_kind: DefKind,
-        def_path_data: DefPathData,
         span: Span,
     ) -> LocalDefId {
         let parent = self.current_hir_id_owner.def_id;
@@ -733,7 +740,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
         let def_id = self
             .tcx
             .at(span)
-            .create_def(parent, name, def_kind, Some(def_path_data), &mut self.disambiguator)
+            .create_def(parent, name, def_kind, None, &mut self.disambiguator)
             .def_id();
 
         debug!("create_def: def_id_to_node_id[{:?}] <-> {:?}", def_id, node_id);
@@ -774,6 +781,8 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
     ) {
         let owner_id = self.owner_id(owner);
 
+        let new_disambig = self.resolver.steal_or_create_disambiguator(owner_id.def_id);
+        let disambiguator = std::mem::replace(&mut self.disambiguator, new_disambig);
         let current_attrs = std::mem::take(&mut self.attrs);
         let current_bodies = std::mem::take(&mut self.bodies);
         let current_define_opaque = std::mem::take(&mut self.define_opaque);
@@ -808,6 +817,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
         assert!(self.impl_trait_bounds.is_empty());
         let info = self.make_owner_info(item);
 
+        self.disambiguator = disambiguator;
         self.attrs = current_attrs;
         self.bodies = current_bodies;
         self.define_opaque = current_define_opaque;
@@ -1015,7 +1025,6 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
                     param,
                     Some(kw::UnderscoreLifetime),
                     DefKind::LifetimeParam,
-                    DefPathData::DesugaredAnonymousLifetime,
                     ident.span,
                 );
                 debug!(?_def_id);
@@ -2504,13 +2513,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
             // We're lowering a const argument that was originally thought to be a type argument,
             // so the def collector didn't create the def ahead of time. That's why we have to do
             // it here.
-            let def_id = self.create_def(
-                node_id,
-                None,
-                DefKind::AnonConst,
-                DefPathData::LateAnonConst,
-                span,
-            );
+            let def_id = self.create_def(node_id, None, DefKind::AnonConst, span);
             let hir_id = self.lower_node_id(node_id);
 
             let path_expr = Expr {

compiler/rustc_ast_lowering/src/pat.rs

@@ -3,7 +3,6 @@ use std::sync::Arc;
 use rustc_ast::*;
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_hir::def::{DefKind, Res};
-use rustc_hir::definitions::DefPathData;
 use rustc_hir::{self as hir, LangItem, Target};
 use rustc_middle::span_bug;
 use rustc_span::{DesugaringKind, Ident, Span, Spanned, respan};
@@ -538,8 +537,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
         // We're generating a range end that didn't exist in the AST,
         // so the def collector didn't create the def ahead of time. That's why we have to do
         // it here.
-        let def_id =
-            self.create_def(node_id, None, DefKind::AnonConst, DefPathData::LateAnonConst, span);
+        let def_id = self.create_def(node_id, None, DefKind::AnonConst, span);
         let hir_id = self.lower_node_id(node_id);
 
         let unstable_span = self.mark_span_with_reason(

compiler/rustc_hir/src/definitions.rs

@@ -97,11 +97,34 @@ impl DefPathTable {
     }
 }
 
-#[derive(Debug)]
+pub trait Disambiguator {
+    fn entry(&mut self, parent: LocalDefId, data: DefPathData) -> &mut u32;
+}
+
+#[derive(Debug, Default, Clone)]
+pub struct PerParentDisambiguatorState {
+    next: UnordMap<DefPathData, u32>,
+}
+
+impl Disambiguator for PerParentDisambiguatorState {
+    #[inline]
+    fn entry(&mut self, _: LocalDefId, data: DefPathData) -> &mut u32 {
+        self.next.entry(data).or_insert(0)
+    }
+}
+
+#[derive(Debug, Default, Clone)]
 pub struct DisambiguatorState {
     next: UnordMap<(LocalDefId, DefPathData), u32>,
 }
 
+impl Disambiguator for DisambiguatorState {
+    #[inline]
+    fn entry(&mut self, parent: LocalDefId, data: DefPathData) -> &mut u32 {
+        self.next.entry((parent, data)).or_insert(0)
+    }
+}
+
 impl DisambiguatorState {
     pub const fn new() -> Self {
         Self { next: Default::default() }
@@ -302,10 +325,6 @@ pub enum DefPathData {
     Ctor,
     /// A constant expression (see `{ast,hir}::AnonConst`).
     AnonConst,
-    /// A constant expression created during AST->HIR lowering..
-    LateAnonConst,
-    /// A fresh anonymous lifetime created by desugaring elided lifetimes.
-    DesugaredAnonymousLifetime,
     /// An existential `impl Trait` type node.
     /// Argument position `impl Trait` have a `TypeNs` with their pretty-printed name.
     OpaqueTy,
@@ -389,7 +408,7 @@ impl Definitions {
         &mut self,
         parent: LocalDefId,
         data: DefPathData,
-        disambiguator: &mut DisambiguatorState,
+        disambiguator: &mut impl Disambiguator,
     ) -> LocalDefId {
         // We can't use `Debug` implementation for `LocalDefId` here, since it tries to acquire a
         // reference to `Definitions` and we're already holding a mutable reference.
@@ -403,7 +422,7 @@ impl Definitions {
 
         // Find the next free disambiguator for this key.
         let disambiguator = {
-            let next_disamb = disambiguator.next.entry((parent, data)).or_insert(0);
+            let next_disamb = disambiguator.entry(parent, data);
             let disambiguator = *next_disamb;
             *next_disamb = next_disamb.checked_add(1).expect("disambiguator overflow");
             disambiguator
@@ -458,8 +477,6 @@ impl DefPathData {
             TypeNs(name) | ValueNs(name) | MacroNs(name) | LifetimeNs(name)
             | OpaqueLifetime(name) => Some(name),
 
-            DesugaredAnonymousLifetime => Some(kw::UnderscoreLifetime),
-
             Impl
             | ForeignMod
             | CrateRoot
@@ -468,7 +485,6 @@ impl DefPathData {
             | Closure
             | Ctor
             | AnonConst
-            | LateAnonConst
             | OpaqueTy
             | AnonAssocTy(..)
             | SyntheticCoroutineBody
@@ -482,8 +498,6 @@ impl DefPathData {
             TypeNs(name) | ValueNs(name) | MacroNs(name) | LifetimeNs(name) | AnonAssocTy(name)
             | OpaqueLifetime(name) => Some(name),
 
-            DesugaredAnonymousLifetime => Some(kw::UnderscoreLifetime),
-
             Impl
             | ForeignMod
             | CrateRoot
@@ -492,7 +506,6 @@ impl DefPathData {
             | Closure
             | Ctor
             | AnonConst
-            | LateAnonConst
             | OpaqueTy
             | SyntheticCoroutineBody
             | NestedStatic => None,
@@ -512,8 +525,7 @@ impl DefPathData {
             GlobalAsm => DefPathDataName::Anon { namespace: sym::global_asm },
             Closure => DefPathDataName::Anon { namespace: sym::closure },
             Ctor => DefPathDataName::Anon { namespace: sym::constructor },
-            AnonConst | LateAnonConst => DefPathDataName::Anon { namespace: sym::constant },
-            DesugaredAnonymousLifetime => DefPathDataName::Named(kw::UnderscoreLifetime),
+            AnonConst => DefPathDataName::Anon { namespace: sym::constant },
             OpaqueTy => DefPathDataName::Anon { namespace: sym::opaque },
             AnonAssocTy(..) => DefPathDataName::Anon { namespace: sym::anon_assoc },
             SyntheticCoroutineBody => DefPathDataName::Anon { namespace: sym::synthetic },

compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs

@@ -1523,7 +1523,7 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
                     None,
                     DefKind::LifetimeParam,
                     Some(DefPathData::OpaqueLifetime(ident.name)),
-                    &mut self.disambiguator,
+                    self.disambiguator,
                 );
                 feed.def_span(ident.span);
                 feed.def_ident_span(Some(ident.span));

compiler/rustc_middle/src/ty/context.rs

@@ -32,7 +32,7 @@ use rustc_data_structures::sync::{
 use rustc_errors::{Applicability, Diag, DiagCtxtHandle, Diagnostic, MultiSpan};
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE, LocalDefId};
-use rustc_hir::definitions::{DefPathData, Definitions, DisambiguatorState};
+use rustc_hir::definitions::{DefPathData, Definitions, Disambiguator};
 use rustc_hir::intravisit::VisitorExt;
 use rustc_hir::lang_items::LangItem;
 use rustc_hir::limit::Limit;
@@ -1355,7 +1355,7 @@ impl<'tcx> TyCtxtAt<'tcx> {
         name: Option<Symbol>,
         def_kind: DefKind,
         override_def_path_data: Option<DefPathData>,
-        disambiguator: &mut DisambiguatorState,
+        disambiguator: &mut impl Disambiguator,
     ) -> TyCtxtFeed<'tcx, LocalDefId> {
         let feed =
             self.tcx.create_def(parent, name, def_kind, override_def_path_data, disambiguator);
@@ -1373,7 +1373,7 @@ impl<'tcx> TyCtxt<'tcx> {
         name: Option<Symbol>,
         def_kind: DefKind,
         override_def_path_data: Option<DefPathData>,
-        disambiguator: &mut DisambiguatorState,
+        disambiguator: &mut impl Disambiguator,
     ) -> TyCtxtFeed<'tcx, LocalDefId> {
         let data = override_def_path_data.unwrap_or_else(|| def_kind.def_path_data(name));
         // The following call has the side effect of modifying the tables inside `definitions`.

compiler/rustc_middle/src/ty/mod.rs

@@ -40,6 +40,7 @@ use rustc_hir as hir;
 use rustc_hir::attrs::StrippedCfgItem;
 use rustc_hir::def::{CtorKind, CtorOf, DefKind, DocLinkResMap, LifetimeRes, Res};
 use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, LocalDefId, LocalDefIdMap};
+use rustc_hir::definitions::PerParentDisambiguatorState;
 use rustc_hir::{LangItem, attrs as attr, find_attr};
 use rustc_index::IndexVec;
 use rustc_index::bit_set::BitMatrix;
@@ -224,6 +225,8 @@ pub struct ResolverAstLowering<'tcx> {
 
     // Information about delegations which is used when handling recursive delegations
     pub delegation_infos: LocalDefIdMap<DelegationInfo>,
+
+    pub per_parent_disambiguators: LocalDefIdMap<Steal<PerParentDisambiguatorState>>,
 }
 
 #[derive(Debug)]

compiler/rustc_middle/src/ty/print/pretty.rs

@@ -2172,7 +2172,6 @@ fn guess_def_namespace(tcx: TyCtxt<'_>, def_id: DefId) -> Namespace {
 
         DefPathData::ValueNs(..)
         | DefPathData::AnonConst
-        | DefPathData::LateAnonConst
         | DefPathData::Closure
         | DefPathData::Ctor => Namespace::ValueNS,
 

compiler/rustc_resolve/src/lib.rs

@@ -57,7 +57,7 @@ use rustc_hir::def::{
     PerNS,
 };
 use rustc_hir::def_id::{CRATE_DEF_ID, CrateNum, DefId, LOCAL_CRATE, LocalDefId, LocalDefIdMap};
-use rustc_hir::definitions::DisambiguatorState;
+use rustc_hir::definitions::PerParentDisambiguatorState;
 use rustc_hir::{PrimTy, TraitCandidate, find_attr};
 use rustc_index::bit_set::DenseBitSet;
 use rustc_metadata::creader::CStore;
@@ -1353,7 +1353,7 @@ pub struct Resolver<'ra, 'tcx> {
 
     node_id_to_def_id: NodeMap<Feed<'tcx, LocalDefId>>,
 
-    disambiguator: DisambiguatorState = DisambiguatorState::new(),
+    per_parent_disambiguators: LocalDefIdMap<PerParentDisambiguatorState>,
 
     /// Indices of unnamed struct or variant fields with unresolved attributes.
     placeholder_field_indices: FxHashMap<NodeId, usize> = default::fx_hash_map(),
@@ -1557,7 +1557,13 @@ impl<'tcx> Resolver<'_, 'tcx> {
         );
 
         // FIXME: remove `def_span` body, pass in the right spans here and call `tcx.at().create_def()`
-        let feed = self.tcx.create_def(parent, name, def_kind, None, &mut self.disambiguator);
+        let feed = self.tcx.create_def(
+            parent,
+            name,
+            def_kind,
+            None,
+            self.per_parent_disambiguators.entry(parent).or_default(),
+        );
         let def_id = feed.def_id();
 
         // Create the definition.
@@ -1739,6 +1745,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             doc_link_resolutions: Default::default(),
             doc_link_traits_in_scope: Default::default(),
             current_crate_outer_attr_insert_span,
+            per_parent_disambiguators: Default::default(),
             ..
         };
 
@@ -1870,6 +1877,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             lifetime_elision_allowed: self.lifetime_elision_allowed,
             lint_buffer: Steal::new(self.lint_buffer),
             delegation_infos: self.delegation_infos,
+            per_parent_disambiguators: self
+                .per_parent_disambiguators
+                .into_items()
+                .map(|(k, d)| (k, Steal::new(d)))
+                .collect(),
         };
         ResolverOutputs { global_ctxt, ast_lowering }
     }