Auto merge of #76176 - marmeladema:fix-closure-path-printing, r=eddyb

Move from {{closure}}#0 syntax to {closure#0} for (def) path components

Part of #70334

I followed the approach described by `@eddyb` and introduced a `DefPathDataName` enum.
To preserve compatibility, in various places, I had to rely on formatting manually by calling `format!("{{{{{}}}}}", namespace)`.

My questions are:
* Do we want to convert for places to use the new naming scheme? Or shall I re-add `DefPathData::as_symbol` but renamed as `DefPathData::as_legacy_symbol` to avoid manually allocating the legacy symbols?
* Do we want to `impl Display for DisambiguatedDefPathData` to avoid manually calling `write!(s, "{{{}#{}}}", namespace, component.disambiguator)`?
* We might also want to improve naming for `DefPathDataName` and `DefPathData::get_name`

r? `@eddyb`

Author: bors

compiler/rustc_codegen_llvm/src/debuginfo/namespace.rs

@@ -27,11 +27,18 @@ pub fn item_namespace(cx: &CodegenCx<'ll, '_>, def_id: DefId) -> &'ll DIScope {
         .parent
         .map(|parent| item_namespace(cx, DefId { krate: def_id.krate, index: parent }));
 
+    let crate_name_as_str;
+    let name_to_string;
     let namespace_name = match def_key.disambiguated_data.data {
-        DefPathData::CrateRoot => cx.tcx.crate_name(def_id.krate),
-        data => data.as_symbol(),
+        DefPathData::CrateRoot => {
+            crate_name_as_str = cx.tcx.crate_name(def_id.krate).as_str();
+            &*crate_name_as_str
+        }
+        data => {
+            name_to_string = data.to_string();
+            &*name_to_string
+        }
     };
-    let namespace_name = namespace_name.as_str();
 
     let scope = unsafe {
         llvm::LLVMRustDIBuilderCreateNameSpace(

compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs

@@ -5,6 +5,8 @@ use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_middle::ty::{self, subst::SubstsRef, Ty, TyCtxt};
 
+use std::fmt::Write;
+
 // Compute the name of the type as it should be stored in debuginfo. Does not do
 // any caching, i.e., calling the function twice with the same type will also do
 // the work twice. The `qualified` parameter only affects the first level of the
@@ -228,8 +230,7 @@ pub fn push_debuginfo_type_name<'tcx>(
         if qualified {
             output.push_str(&tcx.crate_name(def_id.krate).as_str());
             for path_element in tcx.def_path(def_id).data {
-                output.push_str("::");
-                output.push_str(&path_element.data.as_symbol().as_str());
+                write!(output, "::{}", path_element.data).unwrap();
             }
         } else {
             output.push_str(&tcx.item_name(def_id).as_str());

compiler/rustc_hir/src/definitions.rs

@@ -13,9 +13,9 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stable_hasher::StableHasher;
 use rustc_index::vec::IndexVec;
 use rustc_span::hygiene::ExpnId;
-use rustc_span::symbol::{sym, Symbol};
+use rustc_span::symbol::{kw, sym, Symbol};
 
-use std::fmt::Write;
+use std::fmt::{self, Write};
 use std::hash::Hash;
 use tracing::debug;
 
@@ -155,6 +155,29 @@ pub struct DisambiguatedDefPathData {
     pub disambiguator: u32,
 }
 
+impl DisambiguatedDefPathData {
+    pub fn fmt_maybe_verbose(&self, writer: &mut impl Write, verbose: bool) -> fmt::Result {
+        match self.data.name() {
+            DefPathDataName::Named(name) => {
+                if verbose && self.disambiguator != 0 {
+                    write!(writer, "{}#{}", name, self.disambiguator)
+                } else {
+                    writer.write_str(&name.as_str())
+                }
+            }
+            DefPathDataName::Anon { namespace } => {
+                write!(writer, "{{{}#{}}}", namespace, self.disambiguator)
+            }
+        }
+    }
+}
+
+impl fmt::Display for DisambiguatedDefPathData {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.fmt_maybe_verbose(f, true)
+    }
+}
+
 #[derive(Clone, Debug, Encodable, Decodable)]
 pub struct DefPath {
     /// The path leading from the crate root to the item.
@@ -198,33 +221,11 @@ impl DefPath {
     /// Returns a string representation of the `DefPath` without
     /// the crate-prefix. This method is useful if you don't have
     /// a `TyCtxt` available.
-    pub fn to_string_no_crate(&self) -> String {
+    pub fn to_string_no_crate_verbose(&self) -> String {
         let mut s = String::with_capacity(self.data.len() * 16);
 
         for component in &self.data {
-            write!(s, "::{}[{}]", component.data.as_symbol(), component.disambiguator).unwrap();
-        }
-
-        s
-    }
-
-    /// Returns a filename-friendly string for the `DefPath`, with the
-    /// crate-prefix.
-    pub fn to_string_friendly<F>(&self, crate_imported_name: F) -> String
-    where
-        F: FnOnce(CrateNum) -> Symbol,
-    {
-        let crate_name_str = crate_imported_name(self.krate).as_str();
-        let mut s = String::with_capacity(crate_name_str.len() + self.data.len() * 16);
-
-        write!(s, "::{}", crate_name_str).unwrap();
-
-        for component in &self.data {
-            if component.disambiguator == 0 {
-                write!(s, "::{}", component.data.as_symbol()).unwrap();
-            } else {
-                write!(s, "{}[{}]", component.data.as_symbol(), component.disambiguator).unwrap();
-            }
+            write!(s, "::{}", component).unwrap();
         }
 
         s
@@ -240,12 +241,9 @@ impl DefPath {
         for component in &self.data {
             s.extend(opt_delimiter);
             opt_delimiter = Some('-');
-            if component.disambiguator == 0 {
-                write!(s, "{}", component.data.as_symbol()).unwrap();
-            } else {
-                write!(s, "{}[{}]", component.data.as_symbol(), component.disambiguator).unwrap();
-            }
+            write!(s, "{}", component).unwrap();
         }
+
         s
     }
 }
@@ -427,6 +425,12 @@ impl Definitions {
     }
 }
 
+#[derive(Copy, Clone, PartialEq, Debug)]
+pub enum DefPathDataName {
+    Named(Symbol),
+    Anon { namespace: Symbol },
+}
+
 impl DefPathData {
     pub fn get_opt_name(&self) -> Option<Symbol> {
         use self::DefPathData::*;
@@ -437,22 +441,30 @@ impl DefPathData {
         }
     }
 
-    pub fn as_symbol(&self) -> Symbol {
+    pub fn name(&self) -> DefPathDataName {
         use self::DefPathData::*;
         match *self {
-            TypeNs(name) | ValueNs(name) | MacroNs(name) | LifetimeNs(name) => name,
+            TypeNs(name) | ValueNs(name) | MacroNs(name) | LifetimeNs(name) => {
+                DefPathDataName::Named(name)
+            }
             // Note that this does not show up in user print-outs.
-            CrateRoot => sym::double_braced_crate,
-            Impl => sym::double_braced_impl,
-            Misc => sym::double_braced_misc,
-            ClosureExpr => sym::double_braced_closure,
-            Ctor => sym::double_braced_constructor,
-            AnonConst => sym::double_braced_constant,
-            ImplTrait => sym::double_braced_opaque,
+            CrateRoot => DefPathDataName::Anon { namespace: kw::Crate },
+            Impl => DefPathDataName::Anon { namespace: kw::Impl },
+            Misc => DefPathDataName::Anon { namespace: sym::misc },
+            ClosureExpr => DefPathDataName::Anon { namespace: sym::closure },
+            Ctor => DefPathDataName::Anon { namespace: sym::constructor },
+            AnonConst => DefPathDataName::Anon { namespace: sym::constant },
+            ImplTrait => DefPathDataName::Anon { namespace: sym::opaque },
         }
     }
+}
 
-    pub fn to_string(&self) -> String {
-        self.as_symbol().to_string()
+impl fmt::Display for DefPathData {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self.name() {
+            DefPathDataName::Named(name) => f.write_str(&name.as_str()),
+            // FIXME(#70334): this will generate legacy {{closure}}, {{impl}}, etc
+            DefPathDataName::Anon { namespace } => write!(f, "{{{{{}}}}}", namespace),
+        }
     }
 }

compiler/rustc_infer/src/infer/error_reporting/mod.rs

@@ -531,7 +531,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                 disambiguated_data: &DisambiguatedDefPathData,
             ) -> Result<Self::Path, Self::Error> {
                 let mut path = print_prefix(self)?;
-                path.push(disambiguated_data.data.as_symbol().to_string());
+                path.push(disambiguated_data.to_string());
                 Ok(path)
             }
             fn path_generic_args(

compiler/rustc_lint/src/context.rs

@@ -846,7 +846,7 @@ impl<'tcx> LateContext<'tcx> {
                     return Ok(path);
                 }
 
-                path.push(disambiguated_data.data.as_symbol());
+                path.push(Symbol::intern(&disambiguated_data.data.to_string()));
                 Ok(path)
             }
 

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

@@ -244,7 +244,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
         if cfg!(debug_assertions) {
             if hir_id.owner != self.current_dep_node_owner {
                 let node_str = match self.definitions.opt_hir_id_to_local_def_id(hir_id) {
-                    Some(def_id) => self.definitions.def_path(def_id).to_string_no_crate(),
+                    Some(def_id) => self.definitions.def_path(def_id).to_string_no_crate_verbose(),
                     None => format!("{:?}", node),
                 };
 
@@ -254,9 +254,11 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
                      current_dep_node_owner={} ({:?}), hir_id.owner={} ({:?})",
                     self.source_map.span_to_string(span),
                     node_str,
-                    self.definitions.def_path(self.current_dep_node_owner).to_string_no_crate(),
+                    self.definitions
+                        .def_path(self.current_dep_node_owner)
+                        .to_string_no_crate_verbose(),
                     self.current_dep_node_owner,
-                    self.definitions.def_path(hir_id.owner).to_string_no_crate(),
+                    self.definitions.def_path(hir_id.owner).to_string_no_crate_verbose(),
                     hir_id.owner,
                 )
             }

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

@@ -1002,11 +1002,7 @@ fn hir_id_to_string(map: &Map<'_>, id: HirId) -> String {
                 let def_id = map.local_def_id(id);
                 tcx.def_path_str(def_id.to_def_id())
             } else if let Some(path) = map.def_path_from_hir_id(id) {
-                path.data
-                    .into_iter()
-                    .map(|elem| elem.data.to_string())
-                    .collect::<Vec<_>>()
-                    .join("::")
+                path.data.into_iter().map(|elem| elem.to_string()).collect::<Vec<_>>().join("::")
             } else {
                 String::from("<missing path>")
             }

compiler/rustc_middle/src/ty/context.rs

@@ -1272,7 +1272,7 @@ impl<'tcx> TyCtxt<'tcx> {
             // Don't print the whole crate disambiguator. That's just
             // annoying in debug output.
             &(crate_disambiguator.to_fingerprint().to_hex())[..4],
-            self.def_path(def_id).to_string_no_crate()
+            self.def_path(def_id).to_string_no_crate_verbose()
         )
     }
 

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

@@ -11,7 +11,7 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_hir as hir;
 use rustc_hir::def::{self, CtorKind, DefKind, Namespace};
 use rustc_hir::def_id::{CrateNum, DefId, DefIdSet, CRATE_DEF_INDEX, LOCAL_CRATE};
-use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData};
+use rustc_hir::definitions::{DefPathData, DefPathDataName, DisambiguatedDefPathData};
 use rustc_hir::ItemKind;
 use rustc_session::config::TrimmedDefPaths;
 use rustc_span::symbol::{kw, Ident, Symbol};
@@ -1498,25 +1498,21 @@ impl<F: fmt::Write> Printer<'tcx> for FmtPrinter<'_, 'tcx, F> {
 
         // FIXME(eddyb) `name` should never be empty, but it
         // currently is for `extern { ... }` "foreign modules".
-        let name = disambiguated_data.data.as_symbol();
-        if name != kw::Invalid {
+        let name = disambiguated_data.data.name();
+        if name != DefPathDataName::Named(kw::Invalid) {
             if !self.empty_path {
                 write!(self, "::")?;
             }
-            if Ident::with_dummy_span(name).is_raw_guess() {
-                write!(self, "r#")?;
-            }
-            write!(self, "{}", name)?;
 
-            // FIXME(eddyb) this will print e.g. `{{closure}}#3`, but it
-            // might be nicer to use something else, e.g. `{closure#3}`.
-            let dis = disambiguated_data.disambiguator;
-            let print_dis = disambiguated_data.data.get_opt_name().is_none()
-                || dis != 0 && self.tcx.sess.verbose();
-            if print_dis {
-                write!(self, "#{}", dis)?;
+            if let DefPathDataName::Named(name) = name {
+                if Ident::with_dummy_span(name).is_raw_guess() {
+                    write!(self, "r#")?;
+                }
             }
 
+            let verbose = self.tcx.sess.verbose();
+            disambiguated_data.fmt_maybe_verbose(&mut self, verbose)?;
+
             self.empty_path = false;
         }
 

compiler/rustc_middle/src/ty/query/profiling_support.rs

@@ -55,18 +55,22 @@ impl<'p, 'c, 'tcx> QueryKeyStringBuilder<'p, 'c, 'tcx> {
         };
 
         let dis_buffer = &mut [0u8; 16];
+        let crate_name;
+        let other_name;
         let name;
         let dis;
         let end_index;
 
         match def_key.disambiguated_data.data {
             DefPathData::CrateRoot => {
-                name = self.tcx.original_crate_name(def_id.krate);
+                crate_name = self.tcx.original_crate_name(def_id.krate).as_str();
+                name = &*crate_name;
                 dis = "";
                 end_index = 3;
             }
             other => {
-                name = other.as_symbol();
+                other_name = other.to_string();
+                name = other_name.as_str();
                 if def_key.disambiguated_data.disambiguator == 0 {
                     dis = "";
                     end_index = 3;
@@ -80,7 +84,6 @@ impl<'p, 'c, 'tcx> QueryKeyStringBuilder<'p, 'c, 'tcx> {
             }
         }
 
-        let name = &*name.as_str();
         let components = [
             StringComponent::Ref(parent_string_id),
             StringComponent::Value("::"),

compiler/rustc_mir/src/interpret/intrinsics/type_name.rs

@@ -132,9 +132,8 @@ impl<'tcx> Printer<'tcx> for AbsolutePathPrinter<'tcx> {
             return Ok(self);
         }
 
-        self.path.push_str("::");
+        write!(self.path, "::{}", disambiguated_data.data).unwrap();
 
-        self.path.push_str(&disambiguated_data.data.as_symbol().as_str());
         Ok(self)
     }
 

compiler/rustc_mir/src/monomorphize/partitioning/default.rs

@@ -3,6 +3,7 @@ use std::collections::hash_map::Entry;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX, LOCAL_CRATE};
+use rustc_hir::definitions::DefPathDataName;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
 use rustc_middle::middle::exported_symbols::SymbolExportLevel;
 use rustc_middle::mir::mono::{CodegenUnit, CodegenUnitNameBuilder, Linkage, Visibility};
@@ -354,7 +355,10 @@ fn compute_codegen_unit_name(
     *cache.entry((cgu_def_id, volatile)).or_insert_with(|| {
         let def_path = tcx.def_path(cgu_def_id);
 
-        let components = def_path.data.iter().map(|part| part.data.as_symbol());
+        let components = def_path.data.iter().map(|part| match part.data.name() {
+            DefPathDataName::Named(name) => name,
+            DefPathDataName::Anon { .. } => unreachable!(),
+        });
 
         let volatile_suffix = volatile.then_some("volatile");
 

compiler/rustc_passes/src/hir_id_validator.rs

@@ -112,14 +112,14 @@ impl<'a, 'hir> HirIdValidator<'a, 'hir> {
                 missing_items.push(format!(
                     "[local_id: {}, owner: {}]",
                     local_id,
-                    self.hir_map.def_path(owner).to_string_no_crate()
+                    self.hir_map.def_path(owner).to_string_no_crate_verbose()
                 ));
             }
             self.error(|| {
                 format!(
                     "ItemLocalIds not assigned densely in {}. \
                 Max ItemLocalId = {}, missing IDs = {:?}; seens IDs = {:?}",
-                    self.hir_map.def_path(owner).to_string_no_crate(),
+                    self.hir_map.def_path(owner).to_string_no_crate_verbose(),
                     max,
                     missing_items,
                     self.hir_ids_seen
@@ -148,8 +148,8 @@ impl<'a, 'hir> intravisit::Visitor<'hir> for HirIdValidator<'a, 'hir> {
                 format!(
                     "HirIdValidator: The recorded owner of {} is {} instead of {}",
                     self.hir_map.node_to_string(hir_id),
-                    self.hir_map.def_path(hir_id.owner).to_string_no_crate(),
-                    self.hir_map.def_path(owner).to_string_no_crate()
+                    self.hir_map.def_path(hir_id.owner).to_string_no_crate_verbose(),
+                    self.hir_map.def_path(owner).to_string_no_crate_verbose()
                 )
             });
         }