Rollup merge of #143922 - nnethercote:join_path, r=petrochenkov

Improve path segment joining

Currently paths are joined with `::` in many places, in a variety of ways. This PR unifies things.

r? ``@petrochenkov``

11 files changed, 44 insertions, 66 deletions

diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs
index bf3f7607274..fd1b17b6476 100644
--- a/src/librustdoc/clean/utils.rs
+++ b/src/librustdoc/clean/utils.rs
@@ -3,6 +3,7 @@ use std::fmt::{self, Display, Write as _};
 use std::sync::LazyLock as Lazy;
 use std::{ascii, mem};
 
+use rustc_ast::join_path_idents;
 use rustc_ast::tokenstream::TokenTree;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId};
@@ -24,7 +25,7 @@ use crate::clean::{
     clean_middle_ty, inline,
 };
 use crate::core::DocContext;
-use crate::display::{Joined as _, MaybeDisplay as _};
+use crate::display::Joined as _;
 
 #[cfg(test)]
 mod tests;
@@ -251,13 +252,7 @@ pub(crate) fn qpath_to_string(p: &hir::QPath<'_>) -> String {
         hir::QPath::LangItem(lang_item, ..) => return lang_item.name().to_string(),
     };
 
-    fmt::from_fn(|f| {
-        segments
-            .iter()
-            .map(|seg| (seg.ident.name != kw::PathRoot).then_some(seg.ident).maybe_display())
-            .joined("::", f)
-    })
-    .to_string()
+    join_path_idents(segments.iter().map(|seg| seg.ident))
 }
 
 pub(crate) fn build_deref_target_impls(
diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs
index 4989bd718c9..5191120ebdb 100644
--- a/src/librustdoc/formats/cache.rs
+++ b/src/librustdoc/formats/cache.rs
@@ -1,5 +1,6 @@
 use std::mem;
 
+use rustc_ast::join_path_syms;
 use rustc_attr_data_structures::StabilityLevel;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
 use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, DefIdSet};
@@ -13,7 +14,6 @@ use crate::core::DocContext;
 use crate::fold::DocFolder;
 use crate::formats::Impl;
 use crate::formats::item_type::ItemType;
-use crate::html::format::join_with_double_colon;
 use crate::html::markdown::short_markdown_summary;
 use crate::html::render::IndexItem;
 use crate::html::render::search_index::get_function_type_for_search;
@@ -558,7 +558,7 @@ fn add_item_to_search_index(tcx: TyCtxt<'_>, cache: &mut Cache, item: &clean::It
         clean::ItemKind::ImportItem(import) => import.source.did.unwrap_or(item_def_id),
         _ => item_def_id,
     };
-    let path = join_with_double_colon(parent_path);
+    let path = join_path_syms(parent_path);
     let impl_id = if let Some(ParentStackItem::Impl { item_id, .. }) = cache.parent_stack.last() {
         item_id.as_def_id()
     } else {
diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs
index bcb3e57c844..b16485107a0 100644
--- a/src/librustdoc/html/format.rs
+++ b/src/librustdoc/html/format.rs
@@ -14,6 +14,7 @@ use std::slice;
 
 use itertools::{Either, Itertools};
 use rustc_abi::ExternAbi;
+use rustc_ast::join_path_syms;
 use rustc_attr_data_structures::{ConstStability, StabilityLevel, StableSince};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
@@ -25,7 +26,7 @@ use rustc_span::symbol::kw;
 use rustc_span::{Symbol, sym};
 use tracing::{debug, trace};
 
-use super::url_parts_builder::{UrlPartsBuilder, estimate_item_path_byte_length};
+use super::url_parts_builder::UrlPartsBuilder;
 use crate::clean::types::ExternalLocation;
 use crate::clean::utils::find_nearest_parent_module;
 use crate::clean::{self, ExternalCrate, PrimitiveType};
@@ -369,18 +370,6 @@ pub(crate) enum HrefError {
     NotInExternalCache,
 }
 
-// Panics if `syms` is empty.
-pub(crate) fn join_with_double_colon(syms: &[Symbol]) -> String {
-    let mut s = String::with_capacity(estimate_item_path_byte_length(syms.len()));
-    // NOTE: using `Joined::joined` here causes a noticeable perf regression
-    s.push_str(syms[0].as_str());
-    for sym in &syms[1..] {
-        s.push_str("::");
-        s.push_str(sym.as_str());
-    }
-    s
-}
-
 /// This function is to get the external macro path because they are not in the cache used in
 /// `href_with_root_path`.
 fn generate_macro_def_id_path(
@@ -672,7 +661,7 @@ pub(crate) fn link_tooltip(
             write!(f, "{}", cx.tcx().item_name(id))?;
         } else if !fqp.is_empty() {
             write!(f, "{shortty} ")?;
-            fqp.iter().joined("::", f)?;
+            write!(f, "{}", join_path_syms(fqp))?;
         }
         Ok(())
     })
@@ -703,7 +692,7 @@ fn resolved_path(
                     write!(
                         f,
                         "{path}::{anchor}",
-                        path = join_with_double_colon(&fqp[..fqp.len() - 1]),
+                        path = join_path_syms(&fqp[..fqp.len() - 1]),
                         anchor = print_anchor(did, *fqp.last().unwrap(), cx)
                     )
                 } else {
@@ -835,7 +824,7 @@ pub(crate) fn print_anchor(did: DefId, text: Symbol, cx: &Context<'_>) -> impl D
             write!(
                 f,
                 r#"<a class="{short_ty}" href="{url}" title="{short_ty} {path}">{text}</a>"#,
-                path = join_with_double_colon(&fqp),
+                path = join_path_syms(fqp),
                 text = EscapeBodyText(text.as_str()),
             )
         } else {
@@ -1095,7 +1084,7 @@ impl clean::QPathData {
                                     title=\"type {path}::{name}\">{name}</a>",
                         shortty = ItemType::AssocType,
                         name = assoc.name,
-                        path = join_with_double_colon(&path),
+                        path = join_path_syms(path),
                     )
                 } else {
                     write!(f, "{}", assoc.name)
diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs
index 3b4dae841ee..7b814701a73 100644
--- a/src/librustdoc/html/render/context.rs
+++ b/src/librustdoc/html/render/context.rs
@@ -6,6 +6,7 @@ use std::path::{Path, PathBuf};
 use std::sync::mpsc::{Receiver, channel};
 
 use askama::Template;
+use rustc_ast::join_path_syms;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
 use rustc_hir::def_id::{DefIdMap, LOCAL_CRATE};
 use rustc_middle::ty::TyCtxt;
@@ -27,7 +28,6 @@ use crate::formats::FormatRenderer;
 use crate::formats::cache::Cache;
 use crate::formats::item_type::ItemType;
 use crate::html::escape::Escape;
-use crate::html::format::join_with_double_colon;
 use crate::html::markdown::{self, ErrorCodes, IdMap, plain_text_summary};
 use crate::html::render::write_shared::write_shared;
 use crate::html::url_parts_builder::UrlPartsBuilder;
@@ -211,7 +211,7 @@ impl<'tcx> Context<'tcx> {
                 title.push_str(" in ");
             }
             // No need to include the namespace for primitive types and keywords
-            title.push_str(&join_with_double_colon(&self.current));
+            title.push_str(&join_path_syms(&self.current));
         };
         title.push_str(" - Rust");
         let tyname = it.type_();
diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs
index 06de4944d97..d8c37137bbc 100644
--- a/src/librustdoc/html/render/mod.rs
+++ b/src/librustdoc/html/render/mod.rs
@@ -49,6 +49,7 @@ use std::{fs, str};
 
 use askama::Template;
 use itertools::Either;
+use rustc_ast::join_path_syms;
 use rustc_attr_data_structures::{
     ConstStability, DeprecatedSince, Deprecation, RustcVersion, StabilityLevel, StableSince,
 };
@@ -74,9 +75,9 @@ use crate::formats::cache::Cache;
 use crate::formats::item_type::ItemType;
 use crate::html::escape::Escape;
 use crate::html::format::{
-    Ending, HrefError, PrintWithSpace, href, join_with_double_colon, print_abi_with_space,
-    print_constness_with_space, print_default_space, print_generic_bounds, print_where_clause,
-    visibility_print_with_space, write_str,
+    Ending, HrefError, PrintWithSpace, href, print_abi_with_space, print_constness_with_space,
+    print_default_space, print_generic_bounds, print_where_clause, visibility_print_with_space,
+    write_str,
 };
 use crate::html::markdown::{
     HeadingOffset, IdMap, Markdown, MarkdownItemInfo, MarkdownSummaryLine,
@@ -2555,7 +2556,7 @@ fn collect_paths_for_type(first_ty: &clean::Type, cache: &Cache) -> Vec<String>
         let fqp = cache.exact_paths.get(&did).or_else(get_extern);
 
         if let Some(path) = fqp {
-            out.push(join_with_double_colon(path));
+            out.push(join_path_syms(path));
         }
     };
 
diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs
index 667d39e9bc2..5fbda4797cc 100644
--- a/src/librustdoc/html/render/print_item.rs
+++ b/src/librustdoc/html/render/print_item.rs
@@ -4,6 +4,7 @@ use std::iter;
 
 use askama::Template;
 use rustc_abi::VariantIdx;
+use rustc_ast::join_path_syms;
 use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
 use rustc_hir as hir;
 use rustc_hir::def::CtorKind;
@@ -30,8 +31,8 @@ use crate::formats::Impl;
 use crate::formats::item_type::ItemType;
 use crate::html::escape::{Escape, EscapeBodyTextWithWbr};
 use crate::html::format::{
-    Ending, PrintWithSpace, join_with_double_colon, print_abi_with_space,
-    print_constness_with_space, print_where_clause, visibility_print_with_space,
+    Ending, PrintWithSpace, print_abi_with_space, print_constness_with_space, print_where_clause,
+    visibility_print_with_space,
 };
 use crate::html::markdown::{HeadingOffset, MarkdownSummaryLine};
 use crate::html::render::{document_full, document_item_info};
@@ -1424,7 +1425,7 @@ fn item_type_alias(cx: &Context<'_>, it: &clean::Item, t: &clean::TypeAlias) ->
                 iter::repeat_n("..", cx.current.len()).chain(iter::once("type.impl")).collect();
             js_src_path.extend(target_fqp[..target_fqp.len() - 1].iter().copied());
             js_src_path.push_fmt(format_args!("{target_type}.{}.js", target_fqp.last().unwrap()));
-            let self_path = fmt::from_fn(|f| self_fqp.iter().joined("::", f));
+            let self_path = join_path_syms(self_fqp);
             write!(
                 w,
                 "<script src=\"{src}\" data-self-path=\"{self_path}\" async></script>",
@@ -2256,7 +2257,7 @@ pub(crate) fn compare_names(left: &str, right: &str) -> Ordering {
 }
 
 pub(super) fn full_path(cx: &Context<'_>, item: &clean::Item) -> String {
-    let mut s = join_with_double_colon(&cx.current);
+    let mut s = join_path_syms(&cx.current);
     s.push_str("::");
     s.push_str(item.name.unwrap().as_str());
     s
diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs
index aff8684ee3a..80a59fa218c 100644
--- a/src/librustdoc/html/render/search_index.rs
+++ b/src/librustdoc/html/render/search_index.rs
@@ -4,6 +4,7 @@ use std::collections::hash_map::Entry;
 use std::collections::{BTreeMap, VecDeque};
 
 use encode::{bitmap_to_string, write_vlqhex_to_string};
+use rustc_ast::join_path_syms;
 use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
 use rustc_middle::ty::TyCtxt;
 use rustc_span::def_id::DefId;
@@ -17,7 +18,6 @@ use crate::clean::types::{Function, Generics, ItemId, Type, WherePredicate};
 use crate::clean::{self, utils};
 use crate::formats::cache::{Cache, OrphanImplItem};
 use crate::formats::item_type::ItemType;
-use crate::html::format::join_with_double_colon;
 use crate::html::markdown::short_markdown_summary;
 use crate::html::render::ordered_json::OrderedJson;
 use crate::html::render::{self, IndexItem, IndexItemFunctionType, RenderType, RenderTypeId};
@@ -78,7 +78,7 @@ pub(crate) fn build_index(
                 ty: item.type_(),
                 defid: item.item_id.as_def_id(),
                 name: item.name.unwrap(),
-                path: join_with_double_colon(&fqp[..fqp.len() - 1]),
+                path: join_path_syms(&fqp[..fqp.len() - 1]),
                 desc,
                 parent: Some(parent),
                 parent_idx: None,
@@ -416,7 +416,7 @@ pub(crate) fn build_index(
                             if fqp.len() < 2 {
                                 return None;
                             }
-                            join_with_double_colon(&fqp[..fqp.len() - 1])
+                            join_path_syms(&fqp[..fqp.len() - 1])
                         };
                     if path == item.path {
                         return None;
@@ -427,10 +427,10 @@ pub(crate) fn build_index(
                 let i = <isize as TryInto<usize>>::try_into(parent_idx).unwrap();
                 item.path = {
                     let p = &crate_paths[i].1;
-                    join_with_double_colon(&p[..p.len() - 1])
+                    join_path_syms(&p[..p.len() - 1])
                 };
                 item.exact_path =
-                    crate_paths[i].2.as_ref().map(|xp| join_with_double_colon(&xp[..xp.len() - 1]));
+                    crate_paths[i].2.as_ref().map(|xp| join_path_syms(&xp[..xp.len() - 1]));
             }
 
             // Omit the parent path if it is same to that of the prior item.
@@ -549,11 +549,11 @@ pub(crate) fn build_index(
                     });
                     continue;
                 }
-                let full_path = join_with_double_colon(&path[..path.len() - 1]);
+                let full_path = join_path_syms(&path[..path.len() - 1]);
                 let full_exact_path = exact
                     .as_ref()
                     .filter(|exact| exact.last() == path.last() && exact.len() >= 2)
-                    .map(|exact| join_with_double_colon(&exact[..exact.len() - 1]));
+                    .map(|exact| join_path_syms(&exact[..exact.len() - 1]));
                 let exact_path = extra_paths.len() + self.items.len();
                 let exact_path = full_exact_path.as_ref().map(|full_exact_path| match extra_paths
                     .entry(full_exact_path.clone())
diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs
index 0078671fcc5..1f691392b17 100644
--- a/src/librustdoc/html/render/write_shared.rs
+++ b/src/librustdoc/html/render/write_shared.rs
@@ -26,6 +26,7 @@ use std::{fmt, fs};
 
 use indexmap::IndexMap;
 use regex::Regex;
+use rustc_ast::join_path_syms;
 use rustc_data_structures::flock;
 use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
 use rustc_middle::ty::TyCtxt;
@@ -43,7 +44,6 @@ use crate::docfs::PathError;
 use crate::error::Error;
 use crate::formats::Impl;
 use crate::formats::item_type::ItemType;
-use crate::html::format::join_with_double_colon;
 use crate::html::layout;
 use crate::html::render::ordered_json::{EscapedJson, OrderedJson};
 use crate::html::render::search_index::{SerializedSearchIndex, build_index};
@@ -608,7 +608,7 @@ impl TypeAliasPart {
                     for &(type_alias_fqp, type_alias_item) in type_aliases {
                         cx.id_map.borrow_mut().clear();
                         cx.deref_id_map.borrow_mut().clear();
-                        let type_alias_fqp = join_with_double_colon(&type_alias_fqp);
+                        let type_alias_fqp = join_path_syms(type_alias_fqp);
                         if let Some(ret) = &mut ret {
                             ret.aliases.push(type_alias_fqp);
                         } else {
diff --git a/src/librustdoc/html/url_parts_builder.rs b/src/librustdoc/html/url_parts_builder.rs
index 9a533827441..705fa498e8d 100644
--- a/src/librustdoc/html/url_parts_builder.rs
+++ b/src/librustdoc/html/url_parts_builder.rs
@@ -117,7 +117,7 @@ impl UrlPartsBuilder {
 
 /// This is just a guess at the average length of a URL part,
 /// used for [`String::with_capacity`] calls in the [`FromIterator`]
-/// and [`Extend`] impls, and for [estimating item path lengths].
+/// and [`Extend`] impls.
 ///
 /// The value `8` was chosen for two main reasons:
 ///
@@ -125,18 +125,8 @@ impl UrlPartsBuilder {
 /// * jemalloc's size classes are all multiples of eight,
 ///   which means that the amount of memory it allocates will often match
 ///   the amount requested, avoiding wasted bytes.
-///
-/// [estimating item path lengths]: estimate_item_path_byte_length
 const AVG_PART_LENGTH: usize = 8;
 
-/// Estimate the number of bytes in an item's path, based on how many segments it has.
-///
-/// **Note:** This is only to be used with, e.g., [`String::with_capacity()`];
-/// the return value is just a rough estimate.
-pub(crate) const fn estimate_item_path_byte_length(segment_count: usize) -> usize {
-    AVG_PART_LENGTH * segment_count
-}
-
 impl<'a> FromIterator<&'a str> for UrlPartsBuilder {
     fn from_iter<T: IntoIterator<Item = &'a str>>(iter: T) -> Self {
         let iter = iter.into_iter();
diff --git a/src/tools/clippy/clippy_lints/src/methods/from_iter_instead_of_collect.rs b/src/tools/clippy/clippy_lints/src/methods/from_iter_instead_of_collect.rs
index 045363058d1..d664eaaac70 100644
--- a/src/tools/clippy/clippy_lints/src/methods/from_iter_instead_of_collect.rs
+++ b/src/tools/clippy/clippy_lints/src/methods/from_iter_instead_of_collect.rs
@@ -4,6 +4,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::ty::implements_trait;
 use clippy_utils::{is_path_diagnostic_item, sugg};
+use rustc_ast::join_path_idents;
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
 use rustc_hir::{self as hir, Expr, ExprKind, GenericArg, QPath, TyKind};
@@ -47,7 +48,7 @@ fn build_full_type(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, app: &mut Applica
         && let QPath::Resolved(None, ty_path) = &ty_qpath
         && let Res::Def(_, ty_did) = ty_path.res
     {
-        let mut ty_str = itertools::join(ty_path.segments.iter().map(|s| s.ident), "::");
+        let mut ty_str = join_path_idents(ty_path.segments.iter().map(|seg| seg.ident));
         let mut first = true;
         let mut append = |arg: &str| {
             write!(&mut ty_str, "{}{arg}", [", ", "<"][usize::from(first)]).unwrap();
diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs
index ad8c816e204..ff1ee663f9b 100644
--- a/src/tools/clippy/clippy_utils/src/lib.rs
+++ b/src/tools/clippy/clippy_utils/src/lib.rs
@@ -89,6 +89,7 @@ use std::sync::{Mutex, MutexGuard, OnceLock};
 
 use itertools::Itertools;
 use rustc_abi::Integer;
+use rustc_ast::join_path_syms;
 use rustc_ast::ast::{self, LitKind, RangeLimits};
 use rustc_attr_data_structures::{AttributeKind, find_attr};
 use rustc_data_structures::fx::FxHashMap;
@@ -3245,8 +3246,8 @@ fn maybe_get_relative_path(from: &DefPath, to: &DefPath, max_super: usize) -> St
                 // a::b::c  ::d::sym refers to
                 // e::f::sym:: ::
                 // result should be super::super::super::super::e::f
-                if let DefPathData::TypeNs(s) = l {
-                    path.push(s.to_string());
+                if let DefPathData::TypeNs(sym) = l {
+                    path.push(sym);
                 }
                 if let DefPathData::TypeNs(_) = r {
                     go_up_by += 1;
@@ -3256,7 +3257,7 @@ fn maybe_get_relative_path(from: &DefPath, to: &DefPath, max_super: usize) -> St
             // a::b::sym:: ::    refers to
             // c::d::e  ::f::sym
             // when looking at `f`
-            Left(DefPathData::TypeNs(sym)) => path.push(sym.to_string()),
+            Left(DefPathData::TypeNs(sym)) => path.push(sym),
             // consider:
             // a::b::c  ::d::sym refers to
             // e::f::sym:: ::
@@ -3268,17 +3269,17 @@ fn maybe_get_relative_path(from: &DefPath, to: &DefPath, max_super: usize) -> St
 
     if go_up_by > max_super {
         // `super` chain would be too long, just use the absolute path instead
-        once(String::from("crate"))
-            .chain(to.data.iter().filter_map(|el| {
+        join_path_syms(
+            once(kw::Crate).chain(to.data.iter().filter_map(|el| {
                 if let DefPathData::TypeNs(sym) = el.data {
-                    Some(sym.to_string())
+                    Some(sym)
                 } else {
                     None
                 }
             }))
-            .join("::")
+        )
     } else {
-        repeat_n(String::from("super"), go_up_by).chain(path).join("::")
+        join_path_syms(repeat_n(kw::Super, go_up_by).chain(path))
     }
 }