Auto merge of rust-lang#113134 - TaKO8Ki:rollup-4hvqzf6, r=TaKO8Ki

Rollup of 5 pull requests

Successful merges:

 - rust-lang#112946 (Improve cgu naming and ordering)
 - rust-lang#113048 (Fix build on Solaris where fd-lock cannot be used.)
 - rust-lang#113100 (Fix display of long items in search results)
 - rust-lang#113107 (add check for ConstKind::Value(_) to in_operand())
 - rust-lang#113119 (rustdoc: Reduce internal function visibility.)

r? `@ghost`
`@rustbot` modify labels: rollup

Author: bors

compiler/rustc_codegen_ssa/src/back/write.rs

@@ -698,28 +698,49 @@ impl<B: WriteBackendMethods> WorkItem<B> {
 
     /// Generate a short description of this work item suitable for use as a thread name.
     fn short_description(&self) -> String {
-        // `pthread_setname()` on *nix is limited to 15 characters and longer names are ignored.
-        // Use very short descriptions in this case to maximize the space available for the module name.
-        // Windows does not have that limitation so use slightly more descriptive names there.
+        // `pthread_setname()` on *nix ignores anything beyond the first 15
+        // bytes. Use short descriptions to maximize the space available for
+        // the module name.
+        #[cfg(not(windows))]
+        fn desc(short: &str, _long: &str, name: &str) -> String {
+            // The short label is three bytes, and is followed by a space. That
+            // leaves 11 bytes for the CGU name. How we obtain those 11 bytes
+            // depends on the the CGU name form.
+            //
+            // - Non-incremental, e.g. `regex.f10ba03eb5ec7975-cgu.0`: the part
+            //   before the `-cgu.0` is the same for every CGU, so use the
+            //   `cgu.0` part. The number suffix will be different for each
+            //   CGU.
+            //
+            // - Incremental (normal), e.g. `2i52vvl2hco29us0`: use the whole
+            //   name because each CGU will have a unique ASCII hash, and the
+            //   first 11 bytes will be enough to identify it.
+            //
+            // - Incremental (with `-Zhuman-readable-cgu-names`), e.g.
+            //   `regex.f10ba03eb5ec7975-re_builder.volatile`: use the whole
+            //   name. The first 11 bytes won't be enough to uniquely identify
+            //   it, but no obvious substring will, and this is a rarely used
+            //   option so it doesn't matter much.
+            //
+            assert_eq!(short.len(), 3);
+            let name = if let Some(index) = name.find("-cgu.") {
+                &name[index + 1..] // +1 skips the leading '-'.
+            } else {
+                name
+            };
+            format!("{short} {name}")
+        }
+
+        // Windows has no thread name length limit, so use more descriptive names.
+        #[cfg(windows)]
+        fn desc(_short: &str, long: &str, name: &str) -> String {
+            format!("{long} {name}")
+        }
+
         match self {
-            WorkItem::Optimize(m) => {
-                #[cfg(windows)]
-                return format!("optimize module {}", m.name);
-                #[cfg(not(windows))]
-                return format!("opt {}", m.name);
-            }
-            WorkItem::CopyPostLtoArtifacts(m) => {
-                #[cfg(windows)]
-                return format!("copy LTO artifacts for {}", m.name);
-                #[cfg(not(windows))]
-                return format!("copy {}", m.name);
-            }
-            WorkItem::LTO(m) => {
-                #[cfg(windows)]
-                return format!("LTO module {}", m.name());
-                #[cfg(not(windows))]
-                return format!("LTO {}", m.name());
-            }
+            WorkItem::Optimize(m) => desc("opt", "optimize module {}", &m.name),
+            WorkItem::CopyPostLtoArtifacts(m) => desc("cpy", "copy LTO artifacts for {}", &m.name),
+            WorkItem::LTO(m) => desc("lto", "LTO module {}", m.name()),
         }
     }
 }

compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs

@@ -344,15 +344,18 @@ where
     };
 
     // Check the qualifs of the value of `const` items.
-    // FIXME(valtrees): check whether const qualifs should behave the same
-    // way for type and mir constants.
     let uneval = match constant.literal {
         ConstantKind::Ty(ct)
-            if matches!(ct.kind(), ty::ConstKind::Param(_) | ty::ConstKind::Error(_)) =>
+            if matches!(
+                ct.kind(),
+                ty::ConstKind::Param(_) | ty::ConstKind::Error(_) | ty::ConstKind::Value(_)
+            ) =>
         {
             None
         }
-        ConstantKind::Ty(c) => bug!("expected ConstKind::Param here, found {:?}", c),
+        ConstantKind::Ty(c) => {
+            bug!("expected ConstKind::Param or ConstKind::Value here, found {:?}", c)
+        }
         ConstantKind::Unevaluated(uv, _) => Some(uv),
         ConstantKind::Val(..) => None,
     };

compiler/rustc_monomorphize/src/partitioning.rs

@@ -368,6 +368,7 @@ fn merge_codegen_units<'tcx>(
 
     let cgu_name_builder = &mut CodegenUnitNameBuilder::new(cx.tcx);
 
+    // Rename the newly merged CGUs.
     if cx.tcx.sess.opts.incremental.is_some() {
         // If we are doing incremental compilation, we want CGU names to
         // reflect the path of the source level module they correspond to.
@@ -404,18 +405,41 @@ fn merge_codegen_units<'tcx>(
                 }
             }
         }
+
+        // A sorted order here ensures what follows can be deterministic.
+        codegen_units.sort_by(|a, b| a.name().as_str().cmp(b.name().as_str()));
     } else {
-        // If we are compiling non-incrementally we just generate simple CGU
-        // names containing an index.
+        // When compiling non-incrementally, we rename the CGUS so they have
+        // identical names except for the numeric suffix, something like
+        // `regex.f10ba03eb5ec7975-cgu.N`, where `N` varies.
+        //
+        // It is useful for debugging and profiling purposes if the resulting
+        // CGUs are sorted by name *and* reverse sorted by size. (CGU 0 is the
+        // biggest, CGU 1 is the second biggest, etc.)
+        //
+        // So first we reverse sort by size. Then we generate the names with
+        // zero-padded suffixes, which means they are automatically sorted by
+        // names. The numeric suffix width depends on the number of CGUs, which
+        // is always greater than zero:
+        // - [1,9]     CGUS: `0`, `1`, `2`, ...
+        // - [10,99]   CGUS: `00`, `01`, `02`, ...
+        // - [100,999] CGUS: `000`, `001`, `002`, ...
+        // - etc.
+        //
+        // If we didn't zero-pad the sorted-by-name order would be `XYZ-cgu.0`,
+        // `XYZ-cgu.1`, `XYZ-cgu.10`, `XYZ-cgu.11`, ..., `XYZ-cgu.2`, etc.
+        codegen_units.sort_by_key(|cgu| cmp::Reverse(cgu.size_estimate()));
+        let num_digits = codegen_units.len().ilog10() as usize + 1;
         for (index, cgu) in codegen_units.iter_mut().enumerate() {
+            // Note: `WorkItem::short_description` depends on this name ending
+            // with `-cgu.` followed by a numeric suffix. Please keep it in
+            // sync with this code.
+            let suffix = format!("{index:0num_digits$}");
             let numbered_codegen_unit_name =
-                cgu_name_builder.build_cgu_name_no_mangle(LOCAL_CRATE, &["cgu"], Some(index));
+                cgu_name_builder.build_cgu_name_no_mangle(LOCAL_CRATE, &["cgu"], Some(suffix));
             cgu.set_name(numbered_codegen_unit_name);
         }
     }
-
-    // A sorted order here ensures what follows can be deterministic.
-    codegen_units.sort_by(|a, b| a.name().as_str().cmp(b.name().as_str()));
 }
 
 fn internalize_symbols<'tcx>(

src/bootstrap/bin/main.rs

@@ -5,9 +5,11 @@
 //! parent directory, and otherwise documentation can be found throughout the `build`
 //! directory in each respective module.
 
-use std::fs::OpenOptions;
+#[cfg(all(any(unix, windows), not(target_os = "solaris")))]
 use std::io::Write;
-use std::{env, fs, process};
+#[cfg(all(any(unix, windows), not(target_os = "solaris")))]
+use std::process;
+use std::{env, fs};
 
 #[cfg(all(any(unix, windows), not(target_os = "solaris")))]
 use bootstrap::t;
@@ -32,7 +34,7 @@ fn main() {
         };
 
         build_lock =
-            fd_lock::RwLock::new(t!(OpenOptions::new().write(true).create(true).open(&path)));
+            fd_lock::RwLock::new(t!(fs::OpenOptions::new().write(true).create(true).open(&path)));
         _build_lock_guard = match build_lock.try_write() {
             Ok(mut lock) => {
                 t!(lock.write(&process::id().to_string().as_ref()));
@@ -85,7 +87,7 @@ fn main() {
     // HACK: Since the commit script uses hard links, we can't actually tell if it was installed by x.py setup or not.
     // We could see if it's identical to src/etc/pre-push.sh, but pre-push may have been modified in the meantime.
     // Instead, look for this comment, which is almost certainly not in any custom hook.
-    if std::fs::read_to_string(pre_commit).map_or(false, |contents| {
+    if fs::read_to_string(pre_commit).map_or(false, |contents| {
         contents.contains("https://github.com/rust-lang/rust/issues/77620#issuecomment-705144570")
     }) {
         println!(

src/librustdoc/html/format.rs

@@ -1459,7 +1459,7 @@ impl clean::FnDecl {
         Ok(())
     }
 
-    pub(crate) fn print_output<'a, 'tcx: 'a>(
+    fn print_output<'a, 'tcx: 'a>(
         &'a self,
         cx: &'a Context<'tcx>,
     ) -> impl fmt::Display + 'a + Captures<'tcx> {

src/librustdoc/html/static/css/rustdoc.css

@@ -8,6 +8,7 @@
 
 :root {
 	--nav-sub-mobile-padding: 8px;
+	--search-typename-width: 6.75rem;
 }
 
 /* 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 */
 	gap: 1em;
 }
 
-.search-results > a > div {
-	flex: 1;
-}
-
 .search-results > a > div.desc {
 	white-space: nowrap;
 	text-overflow: ellipsis;
 	overflow: hidden;
+	flex: 2;
 }
 
 .search-results a:hover,
 .search-results a:focus {
 	background-color: var(--search-result-link-focus-background-color);
 }
 
+.search-results .result-name {
+	display: flex;
+	align-items: center;
+	justify-content: start;
+	flex: 3;
+}
 .search-results .result-name span.alias {
 	color: var(--search-results-alias-color);
 }
 .search-results .result-name .grey {
 	color: var(--search-results-grey-color);
 }
 .search-results .result-name .typename {
-	display: inline-block;
 	color: var(--search-results-grey-color);
 	font-size: 0.875rem;
-	width: 6.25rem;
+	width: var(--search-typename-width);
+}
+.search-results .result-name .path {
+	word-break: break-all;
+	max-width: calc(100% - var(--search-typename-width));
+	display: inline-block;
 }
 
 .popover {
@@ -1730,6 +1738,16 @@ in source-script.js
 	.search-results > a > div.desc, .item-table > li > div.desc {
 		padding-left: 2em;
 	}
+	.search-results .result-name {
+		display: block;
+	}
+	.search-results .result-name .typename {
+		width: initial;
+		margin-right: 0;
+	}
+	.search-results .result-name .typename, .search-results .result-name .path {
+		display: inline;
+	}
 
 	.source-sidebar-expanded .source .sidebar {
 		max-width: 100vw;

src/librustdoc/html/static/js/search.js

@@ -2024,9 +2024,11 @@ function initSearch(rawSearchIndex) {
 
                 resultName.insertAdjacentHTML(
                     "beforeend",
-                    `<span class="typename">${typeName}</span>`
-                    + ` ${item.displayPath}<span class="${type}">${name}</span>`
-                );
+                    `\
+<span class="typename">${typeName}</span>\
+<div class="path">\
+ ${item.displayPath}<span class="${type}">${name}</span>\
+</div>`);
                 link.appendChild(resultName);
 
                 const description = document.createElement("div");

tests/rustdoc-gui/search-result-color.goml

@@ -61,7 +61,7 @@ assert-css: (
     {"color": "#c5c5c5"},
 )
 assert-css: (
-    "//*[@class='result-name']/*[text()='test_docs::']",
+    "//*[@class='result-name']//*[text()='test_docs::']",
     {"color": "#0096cf"},
 )
 
@@ -138,19 +138,19 @@ call-function: (
 move-cursor-to: ".search-input"
 focus: ".search-input" // To ensure the `<a>` container isnt focus or hover.
 assert-css: (
-    "//*[@class='result-name']/*[text()='test_docs::']/ancestor::a",
+    "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a",
     {"color": "#0096cf", "background-color": "transparent"},
     ALL,
 )
 
 // Checking color and background on hover.
 move-cursor-to: "//*[@class='desc'][text()='Just a normal struct.']"
 assert-css: (
-    "//*[@class='result-name']/*[text()='test_docs::']",
+    "//*[@class='result-name']//*[text()='test_docs::']",
     {"color": "#fff"},
 )
 assert-css: (
-    "//*[@class='result-name']/*[text()='test_docs::']/ancestor::a",
+    "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a",
     {"color": "#fff", "background-color": "rgb(60, 60, 60)"},
 )
 
@@ -173,7 +173,7 @@ assert-css: (
     {"color": "#ddd"},
 )
 assert-css: (
-    "//*[@class='result-name']/*[text()='test_docs::']",
+    "//*[@class='result-name']//*[text()='test_docs::']",
     {"color": "#ddd"},
 )
 
@@ -250,7 +250,7 @@ call-function: (
 move-cursor-to: ".search-input"
 focus: ".search-input" // To ensure the `<a>` container isnt focus or hover.
 assert-css: (
-    "//*[@class='result-name']/*[text()='test_docs::']/ancestor::a",
+    "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a",
     {"color": "#ddd", "background-color": "transparent"},
 )
 
@@ -270,7 +270,7 @@ assert-css: (
     {"color": "#000"},
 )
 assert-css: (
-    "//*[@class='result-name']/*[text()='test_docs::']",
+    "//*[@class='result-name']//*[text()='test_docs::']",
     {"color": "#000"},
 )
 
@@ -347,7 +347,7 @@ call-function: (
 move-cursor-to: ".search-input"
 focus: ".search-input" // To ensure the `<a>` container isnt focus or hover.
 assert-css: (
-    "//*[@class='result-name']/*[text()='test_docs::']/ancestor::a",
+    "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a",
     {"color": "#000", "background-color": "transparent"},
 )
 

tests/rustdoc-gui/search-result-display.goml

@@ -1,3 +1,4 @@
+// ignore-tidy-linelength
 // Checks that the search results have the expected width.
 go-to: "file://" + |DOC_PATH| + "/test_docs/index.html"
 set-window-size: (900, 1000)
@@ -7,15 +8,40 @@ press-key: 'Enter'
 wait-for: "#crate-search"
 // The width is returned by "getComputedStyle" which returns the exact number instead of the
 // CSS rule which is "50%"...
-assert-size: (".search-results div.desc", {"width": 310})
+assert-size: (".search-results div.desc", {"width": 248})
+store-size: (".search-results .result-name .typename", {"width": width})
 set-window-size: (600, 100)
 // As counter-intuitive as it may seem, in this width, the width is "100%", which is why
 // when computed it's larger.
 assert-size: (".search-results div.desc", {"width": 566})
 
 // The result set is all on one line.
-assert-css: (".search-results .result-name > span:not(.typename)", {"display": "inline"})
-assert-css: (".search-results .result-name > span.typename", {"display": "inline-block"})
+compare-elements-position-near: (
+    ".search-results .result-name .typename",
+    ".search-results .result-name .path",
+    {"y": 2},
+)
+compare-elements-position-near-false: (
+    ".search-results .result-name .typename",
+    ".search-results .result-name .path",
+    {"x": 5},
+)
+// The width of the "typename" isn't fixed anymore in this display mode.
+store-size: (".search-results .result-name .typename", {"width": new_width})
+assert: |new_width| < |width| - 10
+
+// Check that if the search is too long on mobile, it'll go under the "typename".
+go-to: "file://" + |DOC_PATH| + "/test_docs/index.html?search=SuperIncrediblyLongLongLongLongLongLongLongGigaGigaGigaMegaLongLongLongStructName"
+wait-for: "#crate-search"
+compare-elements-position-near: (
+    ".search-results .result-name .typename",
+    ".search-results .result-name .path",
+    {"y": 2, "x": 0},
+)
+store-size: (".search-results .result-name", {"width": width, "height": height})
+store-size: (".search-results .result-name .path", {"width": sub_width, "height": sub_height})
+assert: |width| < |sub_width| + 8 && |width| > |sub_width| - 8
+assert: |height| < |sub_height| + 8 && |height| > |sub_height| - 8
 
 // Check that the crate filter `<select>` is correctly handled when it goes to next line.
 // To do so we need to update the length of one of its `<option>`.