Fix delegation def path hash collision

Author: aerooneqq

compiler/rustc_ast_lowering/src/delegation.rs

@@ -153,8 +153,26 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
         &mut self,
         delegation: &Delegation,
         item_id: NodeId,
+        item_span: Span,
     ) -> DelegationResults<'hir> {
-        let span = self.lower_span(delegation.path.segments.last().unwrap().ident.span);
+        let last_seg_span = self.lower_span(delegation.path.segments.last().unwrap().ident.span);
+        let item_span = self.lower_span(item_span);
+
+        // Check if the span of the last segment is contained in item span. The span of last segment
+        // may refer to completely different entity, for example when we do a glob reuse we
+        // generate segments (in `rustc_expand::expand::build_single_delegations`),
+        // and during `rustc_resolve::macros::glob_delegation_suffixes` we generate
+        // suffixes, where identifiers that are used in generated segment
+        // use spans of the original functions from trait,
+        // thus those spans can point either to local or external
+        // trait functions. This `span` is also used for diagnostics purposes, so we want it to point
+        // to delegation, not to the random trait function, so we perform this check here.
+        // We could have added similar check in `rustc_resolve::macros::glob_delegation_suffixes`
+        // (i.e. create ident with span related to ast path of delegation, not the original trait item),
+        // or in `rustc_expand::expand::build_single_delegations`,
+        // however it will break `impl-reuse-pass` test and checking span here seems to be good solution,
+        // as bad spans may come from different places in future.
+        let span = if item_span.contains(last_seg_span) { last_seg_span } else { item_span };
 
         // Delegation can be unresolved in illegal places such as function bodies in extern blocks (see #151356)
         let ids = if let Some(delegation_info) =
@@ -651,7 +669,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
 
             // FIXME(fn_delegation): proper support for parent generics propagation
             // in method call scenario.
-            let segment = self.process_segment(item_id, span, &segment, &mut generics.child, false);
+            let segment = self.process_segment(item_id, span, &segment, &mut generics.child, true);
             let segment = self.arena.alloc(segment);
 
             self.arena.alloc(hir::Expr {
@@ -677,14 +695,14 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
 
                     new_path.segments = self.arena.alloc_from_iter(
                         new_path.segments.iter().enumerate().map(|(idx, segment)| {
-                            let mut process_segment = |result, add_lifetimes| {
-                                self.process_segment(item_id, span, segment, result, add_lifetimes)
+                            let mut process_segment = |result, is_child| {
+                                self.process_segment(item_id, span, segment, result, is_child)
                             };
 
                             if idx + 2 == len {
-                                process_segment(&mut generics.parent, true)
+                                process_segment(&mut generics.parent, false)
                             } else if idx + 1 == len {
-                                process_segment(&mut generics.child, false)
+                                process_segment(&mut generics.child, true)
                             } else {
                                 segment.clone()
                             }
@@ -695,7 +713,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
                 }
                 hir::QPath::TypeRelative(ty, segment) => {
                     let segment =
-                        self.process_segment(item_id, span, segment, &mut generics.child, false);
+                        self.process_segment(item_id, span, segment, &mut generics.child, true);
 
                     hir::QPath::TypeRelative(ty, self.arena.alloc(segment))
                 }
@@ -723,17 +741,17 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
         span: Span,
         segment: &hir::PathSegment<'hir>,
         result: &mut GenericsGenerationResult<'hir>,
-        add_lifetimes: bool,
+        is_child_segment: bool,
     ) -> hir::PathSegment<'hir> {
         let details = result.generics.args_propagation_details();
 
         // The first condition is needed when there is SelfAndUserSpecified case,
         // we don't want to propagate generics params in this situation.
-        let segment = if details.should_propagate
+        let mut segment = if details.should_propagate
             && let Some(args) = result
                 .generics
                 .into_hir_generics(self, item_id, span)
-                .into_generic_args(self, add_lifetimes, span)
+                .into_generic_args(self, !is_child_segment, span)
         {
             hir::PathSegment { args: Some(args), ..segment.clone() }
         } else {
@@ -744,6 +762,12 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
             result.args_segment_id = Some(segment.hir_id);
         }
 
+        // Update segment ident span if it is not contained in delegation span, see comment
+        // in `lower_delegation` function in this file.
+        if is_child_segment && !span.contains(segment.ident.span) {
+            segment.ident.span = span;
+        }
+
         segment
     }
 

compiler/rustc_ast_lowering/src/delegation/generics.rs

@@ -3,6 +3,7 @@ use hir::def::{DefKind, Res};
 use rustc_ast::*;
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
+use rustc_hir::definitions::{DefPathData, DelegationDefPathKind};
 use rustc_middle::ty::GenericParamDefKind;
 use rustc_middle::{bug, ty};
 use rustc_span::sym::{self};
@@ -332,20 +333,27 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
                 GenericParamKind::Const { default, .. } => *default = None,
             }
 
+            let name = p.ident.name;
+            let kind = match p.kind {
+                GenericParamKind::Lifetime => DelegationDefPathKind::Lifetime,
+                GenericParamKind::Type { .. } => DelegationDefPathKind::TyParam,
+                GenericParamKind::Const { .. } => DelegationDefPathKind::ConstParam,
+            };
+
             // Note that we use self.disambiguator here, if we will create new every time
             // we will get ICE if params have the same name.
             self.resolver.insert_new_def_id(
                 p.id,
                 self.tcx
                     .create_def(
                         self.local_def_id(item_id),
-                        Some(p.ident.name),
+                        Some(name),
                         match p.kind {
                             GenericParamKind::Lifetime => DefKind::LifetimeParam,
                             GenericParamKind::Type { .. } => DefKind::TyParam,
                             GenericParamKind::Const { .. } => DefKind::ConstParam,
                         },
-                        None,
+                        Some(DefPathData::Delegation { name, kind }),
                         &mut self.disambiguator,
                     )
                     .def_id(),
@@ -439,7 +447,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
                                 res,
                             }]),
                             res,
-                            span: p.span,
+                            span,
                         }),
                     )
                 };
@@ -460,15 +468,15 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
                     hir::GenericParamKind::Type { .. } => {
                         Some(hir::GenericArg::Type(self.arena.alloc(hir::Ty {
                             hir_id: self.next_id(),
-                            span: p.span,
+                            span,
                             kind: hir::TyKind::Path(create_path(self)),
                         })))
                     }
                     hir::GenericParamKind::Const { .. } => {
                         Some(hir::GenericArg::Const(self.arena.alloc(hir::ConstArg {
                             hir_id: self.next_id(),
                             kind: hir::ConstArgKind::Path(create_path(self)),
-                            span: p.span,
+                            span,
                         })))
                     }
                 }
@@ -519,7 +527,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
                     bounds: Default::default(),
                     colon_span: None,
                     id: self.next_node_id(),
-                    ident: Ident::with_dummy_span(p.name),
+                    ident: Ident::new(p.name, span),
                     is_placeholder: false,
                     kind: match p.kind {
                         GenericParamDefKind::Lifetime => GenericParamKind::Lifetime,
@@ -563,18 +571,18 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
                     None,
                     Path {
                         segments: thin_vec![PathSegment {
-                            ident: Ident::with_dummy_span(type_symbol),
+                            ident: Ident::new(type_symbol, span),
                             id: self.next_node_id(),
                             args: None
                         }],
-                        span: DUMMY_SP,
+                        span,
                         tokens: None,
                     },
                 ),
-                span: DUMMY_SP,
+                span,
                 tokens: None,
             }),
-            span: DUMMY_SP,
+            span,
             default: None,
         }
     }
@@ -608,7 +616,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
                 0,
                 GenericParam {
                     id: self.next_node_id(),
-                    ident: Ident::new(kw::SelfUpper, DUMMY_SP),
+                    ident: Ident::new(kw::SelfUpper, span),
                     attrs: Default::default(),
                     bounds: vec![],
                     is_placeholder: false,

compiler/rustc_ast_lowering/src/item.rs

@@ -579,7 +579,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
                 hir::ItemKind::Macro(ident, macro_def, macro_kinds)
             }
             ItemKind::Delegation(box delegation) => {
-                let delegation_results = self.lower_delegation(delegation, id);
+                let delegation_results = self.lower_delegation(delegation, id, span);
                 hir::ItemKind::Fn {
                     sig: delegation_results.sig,
                     ident: delegation_results.ident,
@@ -1079,7 +1079,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
                 (*ident, generics, kind, ty.is_some())
             }
             AssocItemKind::Delegation(box delegation) => {
-                let delegation_results = self.lower_delegation(delegation, i.id);
+                let delegation_results = self.lower_delegation(delegation, i.id, i.span);
                 let item_kind = hir::TraitItemKind::Fn(
                     delegation_results.sig,
                     hir::TraitFn::Provided(delegation_results.body_id),
@@ -1272,7 +1272,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
                 )
             }
             AssocItemKind::Delegation(box delegation) => {
-                let delegation_results = self.lower_delegation(delegation, i.id);
+                let delegation_results = self.lower_delegation(delegation, i.id, i.span);
                 (
                     delegation.ident,
                     (

compiler/rustc_hir/src/definitions.rs

@@ -269,6 +269,13 @@ impl DefPath {
     }
 }
 
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Encodable, BlobDecodable)]
+pub enum DelegationDefPathKind {
+    Lifetime,
+    ConstParam,
+    TyParam,
+}
+
 /// New variants should only be added in synchronization with `enum DefKind`.
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Encodable, BlobDecodable)]
 pub enum DefPathData {
@@ -318,6 +325,10 @@ pub enum DefPathData {
     SyntheticCoroutineBody,
     /// Additional static data referred to by a static.
     NestedStatic,
+    Delegation {
+        name: Symbol,
+        kind: DelegationDefPathKind,
+    },
 }
 
 impl Definitions {
@@ -455,8 +466,12 @@ impl DefPathData {
     pub fn get_opt_name(&self) -> Option<Symbol> {
         use self::DefPathData::*;
         match *self {
-            TypeNs(name) | ValueNs(name) | MacroNs(name) | LifetimeNs(name)
-            | OpaqueLifetime(name) => Some(name),
+            TypeNs(name)
+            | ValueNs(name)
+            | MacroNs(name)
+            | LifetimeNs(name)
+            | OpaqueLifetime(name)
+            | Delegation { name, .. } => Some(name),
 
             DesugaredAnonymousLifetime => Some(kw::UnderscoreLifetime),
 
@@ -479,8 +494,13 @@ impl DefPathData {
     fn hashed_symbol(&self) -> Option<Symbol> {
         use self::DefPathData::*;
         match *self {
-            TypeNs(name) | ValueNs(name) | MacroNs(name) | LifetimeNs(name) | AnonAssocTy(name)
-            | OpaqueLifetime(name) => Some(name),
+            TypeNs(name)
+            | ValueNs(name)
+            | MacroNs(name)
+            | LifetimeNs(name)
+            | AnonAssocTy(name)
+            | OpaqueLifetime(name)
+            | Delegation { name, .. } => Some(name),
 
             DesugaredAnonymousLifetime => Some(kw::UnderscoreLifetime),
 
@@ -502,8 +522,12 @@ impl DefPathData {
     pub fn name(&self) -> DefPathDataName {
         use self::DefPathData::*;
         match *self {
-            TypeNs(name) | ValueNs(name) | MacroNs(name) | LifetimeNs(name)
-            | OpaqueLifetime(name) => DefPathDataName::Named(name),
+            TypeNs(name)
+            | ValueNs(name)
+            | MacroNs(name)
+            | LifetimeNs(name)
+            | OpaqueLifetime(name)
+            | Delegation { name, .. } => DefPathDataName::Named(name),
             // Note that this does not show up in user print-outs.
             CrateRoot => DefPathDataName::Anon { namespace: kw::Crate },
             Impl => DefPathDataName::Anon { namespace: kw::Impl },

compiler/rustc_sanitizers/src/cfi/typeid/itanium_cxx_abi/encode.rs

@@ -10,7 +10,7 @@ use std::fmt::Write as _;
 use rustc_abi::{ExternAbi, Integer};
 use rustc_data_structures::base_n::{ALPHANUMERIC_ONLY, CASE_INSENSITIVE, ToBaseN};
 use rustc_data_structures::fx::FxHashMap;
-use rustc_hir as hir;
+use rustc_hir::definitions::{DefPathData, DelegationDefPathKind};
 use rustc_hir::find_attr;
 use rustc_middle::bug;
 use rustc_middle::ty::layout::IntegerExt;
@@ -672,26 +672,33 @@ fn encode_ty_name(tcx: TyCtxt<'_>, def_id: DefId) -> String {
     def_path.data.reverse();
     for disambiguated_data in &def_path.data {
         s.push('N');
+
+        const TYPE_NS: &str = "t";
+        const VALUE_NS: &str = "v";
         s.push_str(match disambiguated_data.data {
-            hir::definitions::DefPathData::Impl => "I", // Not specified in v0's <namespace>
-            hir::definitions::DefPathData::ForeignMod => "F", // Not specified in v0's <namespace>
-            hir::definitions::DefPathData::TypeNs(..) => "t",
-            hir::definitions::DefPathData::ValueNs(..) => "v",
-            hir::definitions::DefPathData::Closure => "C",
-            hir::definitions::DefPathData::Ctor => "c",
-            hir::definitions::DefPathData::AnonConst => "K",
-            hir::definitions::DefPathData::LateAnonConst => "k",
-            hir::definitions::DefPathData::OpaqueTy => "i",
-            hir::definitions::DefPathData::SyntheticCoroutineBody => "s",
-            hir::definitions::DefPathData::NestedStatic => "n",
-            hir::definitions::DefPathData::CrateRoot
-            | hir::definitions::DefPathData::Use
-            | hir::definitions::DefPathData::GlobalAsm
-            | hir::definitions::DefPathData::MacroNs(..)
-            | hir::definitions::DefPathData::OpaqueLifetime(..)
-            | hir::definitions::DefPathData::LifetimeNs(..)
-            | hir::definitions::DefPathData::DesugaredAnonymousLifetime
-            | hir::definitions::DefPathData::AnonAssocTy(..) => {
+            DefPathData::Impl => "I",       // Not specified in v0's <namespace>
+            DefPathData::ForeignMod => "F", // Not specified in v0's <namespace>
+            DefPathData::TypeNs(..) => TYPE_NS,
+            DefPathData::ValueNs(..) => VALUE_NS,
+            DefPathData::Closure => "C",
+            DefPathData::Ctor => "c",
+            DefPathData::AnonConst => "K",
+            DefPathData::LateAnonConst => "k",
+            DefPathData::OpaqueTy => "i",
+            DefPathData::SyntheticCoroutineBody => "s",
+            DefPathData::NestedStatic => "n",
+            DefPathData::Delegation { kind: DelegationDefPathKind::ConstParam, .. } => VALUE_NS,
+            DefPathData::Delegation { kind: DelegationDefPathKind::TyParam, .. } => TYPE_NS,
+
+            DefPathData::CrateRoot
+            | DefPathData::Use
+            | DefPathData::GlobalAsm
+            | DefPathData::MacroNs(..)
+            | DefPathData::OpaqueLifetime(..)
+            | DefPathData::LifetimeNs(..)
+            | DefPathData::DesugaredAnonymousLifetime
+            | DefPathData::AnonAssocTy(..)
+            | DefPathData::Delegation { kind: DelegationDefPathKind::Lifetime, .. } => {
                 bug!("encode_ty_name: unexpected `{:?}`", disambiguated_data.data);
             }
         });