Skip to content

Commit 5bfb50b

Browse files
committed
Improve path segment joining.
There are many places that join path segments with `::` to produce a string. A lot of these use `join("::")`. Many in rustdoc use `join_with_double_colon`, and a few use `.joined("..")`. One in Clippy uses `itertools::join`. A couple of them look for `kw::PathRoot` in the first segment, which can be important. This commit introduces `rustc_ast::join_path_{syms,ident}` to do the joining for everyone. `rustc_ast` is as good a location for these as any, being the earliest-running of the several crates with a `Path` type. Two functions are needed because `Ident` printing is more complex than simple `Symbol` printing. The commit also removes `join_with_double_colon`, and `estimate_item_path_byte_length` with it. There are still a handful of places that join strings with "::" that are unchanged. They are not that important: some of them are in tests, and some of them first split a path around "::" and then rejoin with "::". This fixes one test case where `{{root}}` shows up in an error message.
1 parent 3014e79 commit 5bfb50b

File tree

20 files changed

+127
-107
lines changed

20 files changed

+127
-107
lines changed

compiler/rustc_ast/src/ast.rs

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
//! - [`Attribute`]: Metadata associated with item.
1919
//! - [`UnOp`], [`BinOp`], and [`BinOpKind`]: Unary and binary operators.
2020
21-
use std::borrow::Cow;
21+
use std::borrow::{Borrow, Cow};
2222
use std::{cmp, fmt};
2323

2424
pub use GenericArgs::*;
@@ -155,6 +155,57 @@ impl Path {
155155
}
156156
}
157157

158+
/// Joins multiple symbols with "::" into a path, e.g. "a::b::c". If the first
159+
/// segment is `kw::PathRoot` it will be printed as empty, e.g. "::b::c".
160+
///
161+
/// The generics on the `path` argument mean it can accept many forms, such as:
162+
/// - `&[Symbol]`
163+
/// - `Vec<Symbol>`
164+
/// - `Vec<&Symbol>`
165+
/// - `impl Iterator<Item = Symbol>`
166+
/// - `impl Iterator<Item = &Symbol>`
167+
///
168+
/// Panics if `path` is empty or a segment after the first is `kw::PathRoot`.
169+
pub fn join_path_syms(path: impl IntoIterator<Item = impl Borrow<Symbol>>) -> String {
170+
// This is a guess at the needed capacity that works well in practice. It is slightly faster
171+
// than (a) starting with an empty string, or (b) computing the exact capacity required.
172+
let mut iter = path.into_iter();
173+
let len_hint = iter.size_hint().1.unwrap_or(1);
174+
let mut s = String::with_capacity(len_hint * 8);
175+
176+
let first_sym = *iter.next().unwrap().borrow();
177+
if first_sym != kw::PathRoot {
178+
s.push_str(first_sym.as_str());
179+
}
180+
for sym in iter {
181+
let sym = *sym.borrow();
182+
debug_assert_ne!(sym, kw::PathRoot);
183+
s.push_str("::");
184+
s.push_str(sym.as_str());
185+
}
186+
s
187+
}
188+
189+
/// Like `join_path_syms`, but for `Ident`s. This function is necessary because
190+
/// `Ident::to_string` does more than just print the symbol in the `name` field.
191+
pub fn join_path_idents(path: impl IntoIterator<Item = impl Borrow<Ident>>) -> String {
192+
let mut iter = path.into_iter();
193+
let len_hint = iter.size_hint().1.unwrap_or(1);
194+
let mut s = String::with_capacity(len_hint * 8);
195+
196+
let first_ident = *iter.next().unwrap().borrow();
197+
if first_ident.name != kw::PathRoot {
198+
s.push_str(&first_ident.to_string());
199+
}
200+
for ident in iter {
201+
let ident = *ident.borrow();
202+
debug_assert_ne!(ident.name, kw::PathRoot);
203+
s.push_str("::");
204+
s.push_str(&ident.to_string());
205+
}
206+
s
207+
}
208+
158209
/// A segment of a path: an identifier, an optional lifetime, and a set of types.
159210
///
160211
/// E.g., `std`, `String` or `Box<T>`.

compiler/rustc_builtin_macros/src/source_util.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ use std::sync::Arc;
44

55
use rustc_ast as ast;
66
use rustc_ast::ptr::P;
7-
use rustc_ast::token;
87
use rustc_ast::tokenstream::TokenStream;
8+
use rustc_ast::{join_path_idents, token};
99
use rustc_ast_pretty::pprust;
1010
use rustc_expand::base::{
1111
DummyResult, ExpandResult, ExtCtxt, MacEager, MacResult, MacroExpanderResult, resolve_path,
@@ -100,7 +100,7 @@ pub(crate) fn expand_mod(
100100
let sp = cx.with_def_site_ctxt(sp);
101101
check_zero_tts(cx, sp, tts, "module_path!");
102102
let mod_path = &cx.current_expansion.module.mod_path;
103-
let string = mod_path.iter().map(|x| x.to_string()).collect::<Vec<String>>().join("::");
103+
let string = join_path_idents(mod_path);
104104

105105
ExpandResult::Ready(MacEager::expr(cx.expr_str(sp, Symbol::intern(&string))))
106106
}

compiler/rustc_builtin_macros/src/test.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::assert_matches::assert_matches;
55
use std::iter;
66

77
use rustc_ast::ptr::P;
8-
use rustc_ast::{self as ast, GenericParamKind, attr};
8+
use rustc_ast::{self as ast, GenericParamKind, attr, join_path_idents};
99
use rustc_ast_pretty::pprust;
1010
use rustc_errors::{Applicability, Diag, Level};
1111
use rustc_expand::base::*;
@@ -446,12 +446,7 @@ fn get_location_info(cx: &ExtCtxt<'_>, fn_: &ast::Fn) -> (Symbol, usize, usize,
446446
}
447447

448448
fn item_path(mod_path: &[Ident], item_ident: &Ident) -> String {
449-
mod_path
450-
.iter()
451-
.chain(iter::once(item_ident))
452-
.map(|x| x.to_string())
453-
.collect::<Vec<String>>()
454-
.join("::")
449+
join_path_idents(mod_path.iter().chain(iter::once(item_ident)))
455450
}
456451

457452
enum ShouldPanic {

compiler/rustc_hir/src/hir.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use rustc_ast::token::CommentKind;
77
use rustc_ast::util::parser::ExprPrecedence;
88
use rustc_ast::{
99
self as ast, FloatTy, InlineAsmOptions, InlineAsmTemplatePiece, IntTy, Label, LitIntType,
10-
LitKind, TraitObjectSyntax, UintTy, UnsafeBinderCastKind,
10+
LitKind, TraitObjectSyntax, UintTy, UnsafeBinderCastKind, join_path_idents,
1111
};
1212
pub use rustc_ast::{
1313
AssignOp, AssignOpKind, AttrId, AttrStyle, BinOp, BinOpKind, BindingMode, BorrowKind,
@@ -1168,7 +1168,7 @@ impl AttrPath {
11681168

11691169
impl fmt::Display for AttrPath {
11701170
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
1171-
write!(f, "{}", self.segments.iter().map(|i| i.to_string()).collect::<Vec<_>>().join("::"))
1171+
write!(f, "{}", join_path_idents(&self.segments))
11721172
}
11731173
}
11741174

compiler/rustc_hir_typeck/src/method/prelude_edition_lints.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::fmt::Write;
22

33
use hir::def_id::DefId;
44
use hir::{HirId, ItemKind};
5+
use rustc_ast::join_path_idents;
56
use rustc_errors::Applicability;
67
use rustc_hir as hir;
78
use rustc_lint::{ARRAY_INTO_ITER, BOXED_SLICE_INTO_ITER};
@@ -383,13 +384,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
383384
// All that is left is `_`! We need to use the full path. It doesn't matter which one we
384385
// pick, so just take the first one.
385386
match import_items[0].kind {
386-
ItemKind::Use(path, _) => Some(
387-
path.segments
388-
.iter()
389-
.map(|segment| segment.ident.to_string())
390-
.collect::<Vec<_>>()
391-
.join("::"),
392-
),
387+
ItemKind::Use(path, _) => {
388+
Some(join_path_idents(path.segments.iter().map(|seg| seg.ident)))
389+
}
393390
_ => {
394391
span_bug!(span, "unexpected item kind, expected a use: {:?}", import_items[0].kind);
395392
}

compiler/rustc_passes/src/check_attr.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use std::collections::hash_map::Entry;
1010
use std::slice;
1111

1212
use rustc_abi::{Align, ExternAbi, Size};
13-
use rustc_ast::{AttrStyle, LitKind, MetaItemInner, MetaItemKind, ast};
13+
use rustc_ast::{AttrStyle, LitKind, MetaItemInner, MetaItemKind, ast, join_path_syms};
1414
use rustc_attr_data_structures::{AttributeKind, InlineAttr, ReprAttr, find_attr};
1515
use rustc_attr_parsing::{AttributeParser, Late};
1616
use rustc_data_structures::fx::FxHashMap;
@@ -678,9 +678,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
678678
allowed_target: Target,
679679
) {
680680
if target != allowed_target {
681-
let path = attr.path();
682-
let path: Vec<_> = path.iter().map(|s| s.as_str()).collect();
683-
let attr_name = path.join("::");
681+
let attr_name = join_path_syms(attr.path());
684682
self.tcx.emit_node_span_lint(
685683
UNUSED_ATTRIBUTES,
686684
hir_id,

compiler/rustc_resolve/src/diagnostics.rs

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use rustc_ast::ptr::P;
22
use rustc_ast::visit::{self, Visitor};
3-
use rustc_ast::{self as ast, CRATE_NODE_ID, Crate, ItemKind, ModKind, NodeId, Path};
3+
use rustc_ast::{
4+
self as ast, CRATE_NODE_ID, Crate, ItemKind, ModKind, NodeId, Path, join_path_idents,
5+
};
46
use rustc_ast_pretty::pprust;
57
use rustc_attr_data_structures::{
68
self as attr, AttributeKind, CfgEntry, Stability, StrippedCfgItem, find_attr,
@@ -2018,7 +2020,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
20182020
}
20192021
}
20202022

2021-
let mut sugg_paths = vec![];
2023+
let mut sugg_paths: Vec<(Vec<Ident>, bool)> = vec![];
20222024
if let Some(mut def_id) = res.opt_def_id() {
20232025
// We can't use `def_path_str` in resolve.
20242026
let mut path = vec![def_id];
@@ -2031,16 +2033,16 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
20312033
}
20322034
}
20332035
// We will only suggest importing directly if it is accessible through that path.
2034-
let path_names: Option<Vec<String>> = path
2036+
let path_names: Option<Vec<Ident>> = path
20352037
.iter()
20362038
.rev()
20372039
.map(|def_id| {
2038-
self.tcx.opt_item_name(*def_id).map(|n| {
2039-
if def_id.is_top_level_module() {
2040-
"crate".to_string()
2040+
self.tcx.opt_item_name(*def_id).map(|name| {
2041+
Ident::with_dummy_span(if def_id.is_top_level_module() {
2042+
kw::Crate
20412043
} else {
2042-
n.to_string()
2043-
}
2044+
name
2045+
})
20442046
})
20452047
})
20462048
.collect();
@@ -2084,13 +2086,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
20842086
match binding.kind {
20852087
NameBindingKind::Import { import, .. } => {
20862088
for segment in import.module_path.iter().skip(1) {
2087-
path.push(segment.ident.to_string());
2089+
path.push(segment.ident);
20882090
}
20892091
sugg_paths.push((
2090-
path.iter()
2091-
.cloned()
2092-
.chain(vec![ident.to_string()].into_iter())
2093-
.collect::<Vec<_>>(),
2092+
path.iter().cloned().chain(std::iter::once(ident)).collect::<Vec<_>>(),
20942093
true, // re-export
20952094
));
20962095
}
@@ -2126,7 +2125,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
21262125
err.subdiagnostic(note);
21272126
}
21282127
// We prioritize shorter paths, non-core imports and direct imports over the alternatives.
2129-
sugg_paths.sort_by_key(|(p, reexport)| (p.len(), p[0] == "core", *reexport));
2128+
sugg_paths.sort_by_key(|(p, reexport)| (p.len(), p[0].name == sym::core, *reexport));
21302129
for (sugg, reexport) in sugg_paths {
21312130
if not_publicly_reexported {
21322131
break;
@@ -2136,7 +2135,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
21362135
// `tests/ui/imports/issue-55884-2.rs`
21372136
continue;
21382137
}
2139-
let path = sugg.join("::");
2138+
let path = join_path_idents(sugg);
21402139
let sugg = if reexport {
21412140
errors::ImportIdent::ThroughReExport { span: dedup_span, ident, path }
21422141
} else {

compiler/rustc_resolve/src/rustdoc.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use pulldown_cmark::{
77
};
88
use rustc_ast as ast;
99
use rustc_ast::attr::AttributeExt;
10+
use rustc_ast::join_path_syms;
1011
use rustc_ast::util::comments::beautify_doc_string;
1112
use rustc_data_structures::fx::FxIndexMap;
1213
use rustc_data_structures::unord::UnordSet;
@@ -259,7 +260,7 @@ pub fn main_body_opts() -> Options {
259260
| Options::ENABLE_SMART_PUNCTUATION
260261
}
261262

262-
fn strip_generics_from_path_segment(segment: Vec<char>) -> Result<String, MalformedGenerics> {
263+
fn strip_generics_from_path_segment(segment: Vec<char>) -> Result<Symbol, MalformedGenerics> {
263264
let mut stripped_segment = String::new();
264265
let mut param_depth = 0;
265266

@@ -284,7 +285,7 @@ fn strip_generics_from_path_segment(segment: Vec<char>) -> Result<String, Malfor
284285
}
285286

286287
if param_depth == 0 {
287-
Ok(stripped_segment)
288+
Ok(Symbol::intern(&stripped_segment))
288289
} else {
289290
// The segment has unbalanced angle brackets, e.g. `Vec<T` or `Vec<T>>`
290291
Err(MalformedGenerics::UnbalancedAngleBrackets)
@@ -346,9 +347,8 @@ pub fn strip_generics_from_path(path_str: &str) -> Result<Box<str>, MalformedGen
346347

347348
debug!("path_str: {path_str:?}\nstripped segments: {stripped_segments:?}");
348349

349-
let stripped_path = stripped_segments.join("::");
350-
351-
if !stripped_path.is_empty() {
350+
if !stripped_segments.is_empty() {
351+
let stripped_path = join_path_syms(stripped_segments);
352352
Ok(stripped_path.into())
353353
} else {
354354
Err(MalformedGenerics::MissingType)

src/librustdoc/clean/utils.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::fmt::{self, Display, Write as _};
33
use std::sync::LazyLock as Lazy;
44
use std::{ascii, mem};
55

6+
use rustc_ast::join_path_idents;
67
use rustc_ast::tokenstream::TokenTree;
78
use rustc_hir::def::{DefKind, Res};
89
use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId};
@@ -24,7 +25,7 @@ use crate::clean::{
2425
clean_middle_ty, inline,
2526
};
2627
use crate::core::DocContext;
27-
use crate::display::{Joined as _, MaybeDisplay as _};
28+
use crate::display::Joined as _;
2829

2930
#[cfg(test)]
3031
mod tests;
@@ -251,13 +252,7 @@ pub(crate) fn qpath_to_string(p: &hir::QPath<'_>) -> String {
251252
hir::QPath::LangItem(lang_item, ..) => return lang_item.name().to_string(),
252253
};
253254

254-
fmt::from_fn(|f| {
255-
segments
256-
.iter()
257-
.map(|seg| (seg.ident.name != kw::PathRoot).then_some(seg.ident).maybe_display())
258-
.joined("::", f)
259-
})
260-
.to_string()
255+
join_path_idents(segments.iter().map(|seg| seg.ident))
261256
}
262257

263258
pub(crate) fn build_deref_target_impls(

src/librustdoc/formats/cache.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::mem;
22

3+
use rustc_ast::join_path_syms;
34
use rustc_attr_data_structures::StabilityLevel;
45
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
56
use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, DefIdSet};
@@ -13,7 +14,6 @@ use crate::core::DocContext;
1314
use crate::fold::DocFolder;
1415
use crate::formats::Impl;
1516
use crate::formats::item_type::ItemType;
16-
use crate::html::format::join_with_double_colon;
1717
use crate::html::markdown::short_markdown_summary;
1818
use crate::html::render::IndexItem;
1919
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
558558
clean::ItemKind::ImportItem(import) => import.source.did.unwrap_or(item_def_id),
559559
_ => item_def_id,
560560
};
561-
let path = join_with_double_colon(parent_path);
561+
let path = join_path_syms(parent_path);
562562
let impl_id = if let Some(ParentStackItem::Impl { item_id, .. }) = cache.parent_stack.last() {
563563
item_id.as_def_id()
564564
} else {

0 commit comments

Comments
 (0)