Skip to content

Commit de22388

Browse files
borsbors
committed
Auto merge of #113134 - TaKO8Ki:rollup-4hvqzf6, r=TaKO8Ki
Rollup of 5 pull requests Successful merges: - #112946 (Improve cgu naming and ordering) - #113048 (Fix build on Solaris where fd-lock cannot be used.) - #113100 (Fix display of long items in search results) - #113107 (add check for ConstKind::Value(_) to in_operand()) - #113119 (rustdoc: Reduce internal function visibility.) r? `@ghost` `@rustbot` modify labels: rollup
2 parents 0a32ca9 + 8a5272c commit de22388

File tree

10 files changed

+164
-56
lines changed

10 files changed

+164
-56
lines changed

Diff for: ‎compiler/rustc_codegen_ssa/src/back/write.rs

+42-21
Original file line numberDiff line numberDiff line change
@@ -698,28 +698,49 @@ impl<B: WriteBackendMethods> WorkItem<B> {
698698

699699
/// Generate a short description of this work item suitable for use as a thread name.
700700
fn short_description(&self) -> String {
701-
// `pthread_setname()` on *nix is limited to 15 characters and longer names are ignored.
702-
// Use very short descriptions in this case to maximize the space available for the module name.
703-
// Windows does not have that limitation so use slightly more descriptive names there.
701+
// `pthread_setname()` on *nix ignores anything beyond the first 15
702+
// bytes. Use short descriptions to maximize the space available for
703+
// the module name.
704+
#[cfg(not(windows))]
705+
fn desc(short: &str, _long: &str, name: &str) -> String {
706+
// The short label is three bytes, and is followed by a space. That
707+
// leaves 11 bytes for the CGU name. How we obtain those 11 bytes
708+
// depends on the the CGU name form.
709+
//
710+
// - Non-incremental, e.g. `regex.f10ba03eb5ec7975-cgu.0`: the part
711+
// before the `-cgu.0` is the same for every CGU, so use the
712+
// `cgu.0` part. The number suffix will be different for each
713+
// CGU.
714+
//
715+
// - Incremental (normal), e.g. `2i52vvl2hco29us0`: use the whole
716+
// name because each CGU will have a unique ASCII hash, and the
717+
// first 11 bytes will be enough to identify it.
718+
//
719+
// - Incremental (with `-Zhuman-readable-cgu-names`), e.g.
720+
// `regex.f10ba03eb5ec7975-re_builder.volatile`: use the whole
721+
// name. The first 11 bytes won't be enough to uniquely identify
722+
// it, but no obvious substring will, and this is a rarely used
723+
// option so it doesn't matter much.
724+
//
725+
assert_eq!(short.len(), 3);
726+
let name = if let Some(index) = name.find("-cgu.") {
727+
&name[index + 1..] // +1 skips the leading '-'.
728+
} else {
729+
name
730+
};
731+
format!("{short} {name}")
732+
}
733+
734+
// Windows has no thread name length limit, so use more descriptive names.
735+
#[cfg(windows)]
736+
fn desc(_short: &str, long: &str, name: &str) -> String {
737+
format!("{long} {name}")
738+
}
739+
704740
match self {
705-
WorkItem::Optimize(m) => {
706-
#[cfg(windows)]
707-
return format!("optimize module {}", m.name);
708-
#[cfg(not(windows))]
709-
return format!("opt {}", m.name);
710-
}
711-
WorkItem::CopyPostLtoArtifacts(m) => {
712-
#[cfg(windows)]
713-
return format!("copy LTO artifacts for {}", m.name);
714-
#[cfg(not(windows))]
715-
return format!("copy {}", m.name);
716-
}
717-
WorkItem::LTO(m) => {
718-
#[cfg(windows)]
719-
return format!("LTO module {}", m.name());
720-
#[cfg(not(windows))]
721-
return format!("LTO {}", m.name());
722-
}
741+
WorkItem::Optimize(m) => desc("opt", "optimize module {}", &m.name),
742+
WorkItem::CopyPostLtoArtifacts(m) => desc("cpy", "copy LTO artifacts for {}", &m.name),
743+
WorkItem::LTO(m) => desc("lto", "LTO module {}", m.name()),
723744
}
724745
}
725746
}

Diff for: ‎compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -344,15 +344,18 @@ where
344344
};
345345

346346
// Check the qualifs of the value of `const` items.
347-
// FIXME(valtrees): check whether const qualifs should behave the same
348-
// way for type and mir constants.
349347
let uneval = match constant.literal {
350348
ConstantKind::Ty(ct)
351-
if matches!(ct.kind(), ty::ConstKind::Param(_) | ty::ConstKind::Error(_)) =>
349+
if matches!(
350+
ct.kind(),
351+
ty::ConstKind::Param(_) | ty::ConstKind::Error(_) | ty::ConstKind::Value(_)
352+
) =>
352353
{
353354
None
354355
}
355-
ConstantKind::Ty(c) => bug!("expected ConstKind::Param here, found {:?}", c),
356+
ConstantKind::Ty(c) => {
357+
bug!("expected ConstKind::Param or ConstKind::Value here, found {:?}", c)
358+
}
356359
ConstantKind::Unevaluated(uv, _) => Some(uv),
357360
ConstantKind::Val(..) => None,
358361
};

Diff for: ‎compiler/rustc_monomorphize/src/partitioning.rs

+30-6
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,7 @@ fn merge_codegen_units<'tcx>(
368368

369369
let cgu_name_builder = &mut CodegenUnitNameBuilder::new(cx.tcx);
370370

371+
// Rename the newly merged CGUs.
371372
if cx.tcx.sess.opts.incremental.is_some() {
372373
// If we are doing incremental compilation, we want CGU names to
373374
// reflect the path of the source level module they correspond to.
@@ -404,18 +405,41 @@ fn merge_codegen_units<'tcx>(
404405
}
405406
}
406407
}
408+
409+
// A sorted order here ensures what follows can be deterministic.
410+
codegen_units.sort_by(|a, b| a.name().as_str().cmp(b.name().as_str()));
407411
} else {
408-
// If we are compiling non-incrementally we just generate simple CGU
409-
// names containing an index.
412+
// When compiling non-incrementally, we rename the CGUS so they have
413+
// identical names except for the numeric suffix, something like
414+
// `regex.f10ba03eb5ec7975-cgu.N`, where `N` varies.
415+
//
416+
// It is useful for debugging and profiling purposes if the resulting
417+
// CGUs are sorted by name *and* reverse sorted by size. (CGU 0 is the
418+
// biggest, CGU 1 is the second biggest, etc.)
419+
//
420+
// So first we reverse sort by size. Then we generate the names with
421+
// zero-padded suffixes, which means they are automatically sorted by
422+
// names. The numeric suffix width depends on the number of CGUs, which
423+
// is always greater than zero:
424+
// - [1,9] CGUS: `0`, `1`, `2`, ...
425+
// - [10,99] CGUS: `00`, `01`, `02`, ...
426+
// - [100,999] CGUS: `000`, `001`, `002`, ...
427+
// - etc.
428+
//
429+
// If we didn't zero-pad the sorted-by-name order would be `XYZ-cgu.0`,
430+
// `XYZ-cgu.1`, `XYZ-cgu.10`, `XYZ-cgu.11`, ..., `XYZ-cgu.2`, etc.
431+
codegen_units.sort_by_key(|cgu| cmp::Reverse(cgu.size_estimate()));
432+
let num_digits = codegen_units.len().ilog10() as usize + 1;
410433
for (index, cgu) in codegen_units.iter_mut().enumerate() {
434+
// Note: `WorkItem::short_description` depends on this name ending
435+
// with `-cgu.` followed by a numeric suffix. Please keep it in
436+
// sync with this code.
437+
let suffix = format!("{index:0num_digits$}");
411438
let numbered_codegen_unit_name =
412-
cgu_name_builder.build_cgu_name_no_mangle(LOCAL_CRATE, &["cgu"], Some(index));
439+
cgu_name_builder.build_cgu_name_no_mangle(LOCAL_CRATE, &["cgu"], Some(suffix));
413440
cgu.set_name(numbered_codegen_unit_name);
414441
}
415442
}
416-
417-
// A sorted order here ensures what follows can be deterministic.
418-
codegen_units.sort_by(|a, b| a.name().as_str().cmp(b.name().as_str()));
419443
}
420444

421445
fn internalize_symbols<'tcx>(

Diff for: ‎src/bootstrap/bin/main.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55
//! parent directory, and otherwise documentation can be found throughout the `build`
66
//! directory in each respective module.
77
8-
use std::fs::OpenOptions;
8+
#[cfg(all(any(unix, windows), not(target_os = "solaris")))]
99
use std::io::Write;
10-
use std::{env, fs, process};
10+
#[cfg(all(any(unix, windows), not(target_os = "solaris")))]
11+
use std::process;
12+
use std::{env, fs};
1113

1214
#[cfg(all(any(unix, windows), not(target_os = "solaris")))]
1315
use bootstrap::t;
@@ -32,7 +34,7 @@ fn main() {
3234
};
3335

3436
build_lock =
35-
fd_lock::RwLock::new(t!(OpenOptions::new().write(true).create(true).open(&path)));
37+
fd_lock::RwLock::new(t!(fs::OpenOptions::new().write(true).create(true).open(&path)));
3638
_build_lock_guard = match build_lock.try_write() {
3739
Ok(mut lock) => {
3840
t!(lock.write(&process::id().to_string().as_ref()));
@@ -85,7 +87,7 @@ fn main() {
8587
// HACK: Since the commit script uses hard links, we can't actually tell if it was installed by x.py setup or not.
8688
// We could see if it's identical to src/etc/pre-push.sh, but pre-push may have been modified in the meantime.
8789
// Instead, look for this comment, which is almost certainly not in any custom hook.
88-
if std::fs::read_to_string(pre_commit).map_or(false, |contents| {
90+
if fs::read_to_string(pre_commit).map_or(false, |contents| {
8991
contents.contains("https://github.com/rust-lang/rust/issues/77620#issuecomment-705144570")
9092
}) {
9193
println!(

Diff for: ‎src/librustdoc/html/format.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1459,7 +1459,7 @@ impl clean::FnDecl {
14591459
Ok(())
14601460
}
14611461

1462-
pub(crate) fn print_output<'a, 'tcx: 'a>(
1462+
fn print_output<'a, 'tcx: 'a>(
14631463
&'a self,
14641464
cx: &'a Context<'tcx>,
14651465
) -> impl fmt::Display + 'a + Captures<'tcx> {

Diff for: ‎src/librustdoc/html/static/css/rustdoc.css

+24-6
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
:root {
1010
--nav-sub-mobile-padding: 8px;
11+
--search-typename-width: 6.75rem;
1112
}
1213

1314
/* See FiraSans-LICENSE.txt for the Fira Sans license. */
@@ -869,32 +870,39 @@ so that we can apply CSS-filters to change the arrow color in themes */
869870
gap: 1em;
870871
}
871872

872-
.search-results > a > div {
873-
flex: 1;
874-
}
875-
876873
.search-results > a > div.desc {
877874
white-space: nowrap;
878875
text-overflow: ellipsis;
879876
overflow: hidden;
877+
flex: 2;
880878
}
881879

882880
.search-results a:hover,
883881
.search-results a:focus {
884882
background-color: var(--search-result-link-focus-background-color);
885883
}
886884

885+
.search-results .result-name {
886+
display: flex;
887+
align-items: center;
888+
justify-content: start;
889+
flex: 3;
890+
}
887891
.search-results .result-name span.alias {
888892
color: var(--search-results-alias-color);
889893
}
890894
.search-results .result-name .grey {
891895
color: var(--search-results-grey-color);
892896
}
893897
.search-results .result-name .typename {
894-
display: inline-block;
895898
color: var(--search-results-grey-color);
896899
font-size: 0.875rem;
897-
width: 6.25rem;
900+
width: var(--search-typename-width);
901+
}
902+
.search-results .result-name .path {
903+
word-break: break-all;
904+
max-width: calc(100% - var(--search-typename-width));
905+
display: inline-block;
898906
}
899907

900908
.popover {
@@ -1730,6 +1738,16 @@ in source-script.js
17301738
.search-results > a > div.desc, .item-table > li > div.desc {
17311739
padding-left: 2em;
17321740
}
1741+
.search-results .result-name {
1742+
display: block;
1743+
}
1744+
.search-results .result-name .typename {
1745+
width: initial;
1746+
margin-right: 0;
1747+
}
1748+
.search-results .result-name .typename, .search-results .result-name .path {
1749+
display: inline;
1750+
}
17331751

17341752
.source-sidebar-expanded .source .sidebar {
17351753
max-width: 100vw;

Diff for: ‎src/librustdoc/html/static/js/search.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -2024,9 +2024,11 @@ function initSearch(rawSearchIndex) {
20242024

20252025
resultName.insertAdjacentHTML(
20262026
"beforeend",
2027-
`<span class="typename">${typeName}</span>`
2028-
+ ` ${item.displayPath}<span class="${type}">${name}</span>`
2029-
);
2027+
`\
2028+
<span class="typename">${typeName}</span>\
2029+
<div class="path">\
2030+
${item.displayPath}<span class="${type}">${name}</span>\
2031+
</div>`);
20302032
link.appendChild(resultName);
20312033

20322034
const description = document.createElement("div");

Diff for: ‎tests/rustdoc-gui/search-result-color.goml

+8-8
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ assert-css: (
6161
{"color": "#c5c5c5"},
6262
)
6363
assert-css: (
64-
"//*[@class='result-name']/*[text()='test_docs::']",
64+
"//*[@class='result-name']//*[text()='test_docs::']",
6565
{"color": "#0096cf"},
6666
)
6767

@@ -138,19 +138,19 @@ call-function: (
138138
move-cursor-to: ".search-input"
139139
focus: ".search-input" // To ensure the `<a>` container isnt focus or hover.
140140
assert-css: (
141-
"//*[@class='result-name']/*[text()='test_docs::']/ancestor::a",
141+
"//*[@class='result-name']//*[text()='test_docs::']/ancestor::a",
142142
{"color": "#0096cf", "background-color": "transparent"},
143143
ALL,
144144
)
145145

146146
// Checking color and background on hover.
147147
move-cursor-to: "//*[@class='desc'][text()='Just a normal struct.']"
148148
assert-css: (
149-
"//*[@class='result-name']/*[text()='test_docs::']",
149+
"//*[@class='result-name']//*[text()='test_docs::']",
150150
{"color": "#fff"},
151151
)
152152
assert-css: (
153-
"//*[@class='result-name']/*[text()='test_docs::']/ancestor::a",
153+
"//*[@class='result-name']//*[text()='test_docs::']/ancestor::a",
154154
{"color": "#fff", "background-color": "rgb(60, 60, 60)"},
155155
)
156156

@@ -173,7 +173,7 @@ assert-css: (
173173
{"color": "#ddd"},
174174
)
175175
assert-css: (
176-
"//*[@class='result-name']/*[text()='test_docs::']",
176+
"//*[@class='result-name']//*[text()='test_docs::']",
177177
{"color": "#ddd"},
178178
)
179179

@@ -250,7 +250,7 @@ call-function: (
250250
move-cursor-to: ".search-input"
251251
focus: ".search-input" // To ensure the `<a>` container isnt focus or hover.
252252
assert-css: (
253-
"//*[@class='result-name']/*[text()='test_docs::']/ancestor::a",
253+
"//*[@class='result-name']//*[text()='test_docs::']/ancestor::a",
254254
{"color": "#ddd", "background-color": "transparent"},
255255
)
256256

@@ -270,7 +270,7 @@ assert-css: (
270270
{"color": "#000"},
271271
)
272272
assert-css: (
273-
"//*[@class='result-name']/*[text()='test_docs::']",
273+
"//*[@class='result-name']//*[text()='test_docs::']",
274274
{"color": "#000"},
275275
)
276276

@@ -347,7 +347,7 @@ call-function: (
347347
move-cursor-to: ".search-input"
348348
focus: ".search-input" // To ensure the `<a>` container isnt focus or hover.
349349
assert-css: (
350-
"//*[@class='result-name']/*[text()='test_docs::']/ancestor::a",
350+
"//*[@class='result-name']//*[text()='test_docs::']/ancestor::a",
351351
{"color": "#000", "background-color": "transparent"},
352352
)
353353

Diff for: ‎tests/rustdoc-gui/search-result-display.goml

+29-3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// ignore-tidy-linelength
12
// Checks that the search results have the expected width.
23
go-to: "file://" + |DOC_PATH| + "/test_docs/index.html"
34
set-window-size: (900, 1000)
@@ -7,15 +8,40 @@ press-key: 'Enter'
78
wait-for: "#crate-search"
89
// The width is returned by "getComputedStyle" which returns the exact number instead of the
910
// CSS rule which is "50%"...
10-
assert-size: (".search-results div.desc", {"width": 310})
11+
assert-size: (".search-results div.desc", {"width": 248})
12+
store-size: (".search-results .result-name .typename", {"width": width})
1113
set-window-size: (600, 100)
1214
// As counter-intuitive as it may seem, in this width, the width is "100%", which is why
1315
// when computed it's larger.
1416
assert-size: (".search-results div.desc", {"width": 566})
1517

1618
// The result set is all on one line.
17-
assert-css: (".search-results .result-name > span:not(.typename)", {"display": "inline"})
18-
assert-css: (".search-results .result-name > span.typename", {"display": "inline-block"})
19+
compare-elements-position-near: (
20+
".search-results .result-name .typename",
21+
".search-results .result-name .path",
22+
{"y": 2},
23+
)
24+
compare-elements-position-near-false: (
25+
".search-results .result-name .typename",
26+
".search-results .result-name .path",
27+
{"x": 5},
28+
)
29+
// The width of the "typename" isn't fixed anymore in this display mode.
30+
store-size: (".search-results .result-name .typename", {"width": new_width})
31+
assert: |new_width| < |width| - 10
32+
33+
// Check that if the search is too long on mobile, it'll go under the "typename".
34+
go-to: "file://" + |DOC_PATH| + "/test_docs/index.html?search=SuperIncrediblyLongLongLongLongLongLongLongGigaGigaGigaMegaLongLongLongStructName"
35+
wait-for: "#crate-search"
36+
compare-elements-position-near: (
37+
".search-results .result-name .typename",
38+
".search-results .result-name .path",
39+
{"y": 2, "x": 0},
40+
)
41+
store-size: (".search-results .result-name", {"width": width, "height": height})
42+
store-size: (".search-results .result-name .path", {"width": sub_width, "height": sub_height})
43+
assert: |width| < |sub_width| + 8 && |width| > |sub_width| - 8
44+
assert: |height| < |sub_height| + 8 && |height| > |sub_height| - 8
1945

2046
// Check that the crate filter `<select>` is correctly handled when it goes to next line.
2147
// To do so we need to update the length of one of its `<option>`.

0 commit comments

Comments
 (0)