Auto merge of #139023 - jhpratt:rollup-4ou6ei4, r=jhpratt

Rollup of 7 pull requests

Successful merges:

 - #138844 (expand: Leave traces when expanding `cfg` attributes)
 - #138926 (Remove `kw::Empty` uses from `rustc_middle`.)
 - #138989 (Clean up a few things in rustc_hir_analysis::check::region)
 - #138999 (Report compiletest pass mode if forced)
 - #139014 (Improve suggest construct with literal syntax instead of calling)
 - #139015 (Remove unneeded LLVM CI test assertions)
 - #139016 (Add job duration changes to post-merge analysis report)

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

15 files changed, 148 insertions, 58 deletions

diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs
index 068e237c2cd..6d63709f474 100644
--- a/src/bootstrap/src/core/config/tests.rs
+++ b/src/bootstrap/src/core/config/tests.rs
@@ -24,31 +24,11 @@ pub(crate) fn parse(config: &str) -> Config {
 
 #[test]
 fn download_ci_llvm() {
-    let config = parse("");
-    let is_available = llvm::is_ci_llvm_available_for_target(&config, config.llvm_assertions);
-    if is_available {
-        assert!(config.llvm_from_ci);
-    }
-
-    let config = Config::parse_inner(
-        Flags::parse(&[
-            "check".to_string(),
-            "--config=/does/not/exist".to_string(),
-            "--ci".to_string(),
-            "false".to_string(),
-        ]),
-        |&_| toml::from_str("llvm.download-ci-llvm = true"),
-    );
-    let is_available = llvm::is_ci_llvm_available_for_target(&config, config.llvm_assertions);
-    if is_available {
-        assert!(config.llvm_from_ci);
-    }
-
     let config = parse("llvm.download-ci-llvm = false");
     assert!(!config.llvm_from_ci);
 
     let if_unchanged_config = parse("llvm.download-ci-llvm = \"if-unchanged\"");
-    if if_unchanged_config.llvm_from_ci {
+    if if_unchanged_config.llvm_from_ci && if_unchanged_config.is_running_on_ci {
         let has_changes = if_unchanged_config
             .last_modified_commit(LLVM_INVALIDATION_PATHS, "download-ci-llvm", true)
             .is_none();
diff --git a/src/bootstrap/src/utils/metrics.rs b/src/bootstrap/src/utils/metrics.rs
index 885fff9c32c..862c4449624 100644
--- a/src/bootstrap/src/utils/metrics.rs
+++ b/src/bootstrap/src/utils/metrics.rs
@@ -9,9 +9,10 @@ use std::fs::File;
 use std::io::BufWriter;
 use std::time::{Duration, Instant, SystemTime};
 
+use build_helper::ci::CiEnv;
 use build_helper::metrics::{
-    JsonInvocation, JsonInvocationSystemStats, JsonNode, JsonRoot, JsonStepSystemStats, Test,
-    TestOutcome, TestSuite, TestSuiteMetadata,
+    CiMetadata, JsonInvocation, JsonInvocationSystemStats, JsonNode, JsonRoot, JsonStepSystemStats,
+    Test, TestOutcome, TestSuite, TestSuiteMetadata,
 };
 use sysinfo::{CpuRefreshKind, RefreshKind, System};
 
@@ -217,7 +218,12 @@ impl BuildMetrics {
             children: steps.into_iter().map(|step| self.prepare_json_step(step)).collect(),
         });
 
-        let json = JsonRoot { format_version: CURRENT_FORMAT_VERSION, system_stats, invocations };
+        let json = JsonRoot {
+            format_version: CURRENT_FORMAT_VERSION,
+            system_stats,
+            invocations,
+            ci_metadata: get_ci_metadata(CiEnv::current()),
+        };
 
         t!(std::fs::create_dir_all(dest.parent().unwrap()));
         let mut file = BufWriter::new(t!(File::create(&dest)));
@@ -245,6 +251,16 @@ impl BuildMetrics {
     }
 }
 
+fn get_ci_metadata(ci_env: CiEnv) -> Option<CiMetadata> {
+    if ci_env != CiEnv::GitHubActions {
+        return None;
+    }
+    let workflow_run_id =
+        std::env::var("GITHUB_WORKFLOW_RUN_ID").ok().and_then(|id| id.parse::<u64>().ok())?;
+    let repository = std::env::var("GITHUB_REPOSITORY").ok()?;
+    Some(CiMetadata { workflow_run_id, repository })
+}
+
 struct MetricsState {
     finished_steps: Vec<StepMetrics>,
     running_steps: Vec<StepMetrics>,
diff --git a/src/build_helper/src/metrics.rs b/src/build_helper/src/metrics.rs
index fdff9cd18ce..8b82e62a327 100644
--- a/src/build_helper/src/metrics.rs
+++ b/src/build_helper/src/metrics.rs
@@ -9,6 +9,19 @@ pub struct JsonRoot {
     pub format_version: usize,
     pub system_stats: JsonInvocationSystemStats,
     pub invocations: Vec<JsonInvocation>,
+    #[serde(default)]
+    pub ci_metadata: Option<CiMetadata>,
+}
+
+/// Represents metadata about bootstrap's execution in CI.
+#[derive(Serialize, Deserialize)]
+pub struct CiMetadata {
+    /// GitHub run ID of the workflow where bootstrap was executed.
+    /// Note that the run ID will be shared amongst all jobs executed in that workflow.
+    pub workflow_run_id: u64,
+    /// Full name of a GitHub repository where bootstrap was executed in CI.
+    /// e.g. `rust-lang-ci/rust`.
+    pub repository: String,
 }
 
 #[derive(Serialize, Deserialize)]
diff --git a/src/ci/citool/src/analysis.rs b/src/ci/citool/src/analysis.rs
index 2b001f28b0e..7fbfad467c6 100644
--- a/src/ci/citool/src/analysis.rs
+++ b/src/ci/citool/src/analysis.rs
@@ -1,5 +1,6 @@
 use std::collections::{BTreeMap, HashMap, HashSet};
 use std::fmt::Debug;
+use std::time::Duration;
 
 use build_helper::metrics::{
     BuildStep, JsonRoot, TestOutcome, TestSuite, TestSuiteMetadata, escape_step_name,
@@ -184,11 +185,70 @@ fn render_table(suites: BTreeMap<String, TestSuiteRecord>) -> String {
 }
 
 /// Outputs a report of test differences between the `parent` and `current` commits.
-pub fn output_test_diffs(job_metrics: HashMap<JobName, JobMetrics>) {
+pub fn output_test_diffs(job_metrics: &HashMap<JobName, JobMetrics>) {
     let aggregated_test_diffs = aggregate_test_diffs(&job_metrics);
     report_test_diffs(aggregated_test_diffs);
 }
 
+/// Prints the ten largest differences in bootstrap durations.
+pub fn output_largest_duration_changes(job_metrics: &HashMap<JobName, JobMetrics>) {
+    struct Entry<'a> {
+        job: &'a JobName,
+        before: Duration,
+        after: Duration,
+        change: f64,
+    }
+
+    let mut changes: Vec<Entry> = vec![];
+    for (job, metrics) in job_metrics {
+        if let Some(parent) = &metrics.parent {
+            let duration_before = parent
+                .invocations
+                .iter()
+                .map(|i| BuildStep::from_invocation(i).duration)
+                .sum::<Duration>();
+            let duration_after = metrics
+                .current
+                .invocations
+                .iter()
+                .map(|i| BuildStep::from_invocation(i).duration)
+                .sum::<Duration>();
+            let pct_change = duration_after.as_secs_f64() / duration_before.as_secs_f64();
+            let pct_change = pct_change * 100.0;
+            // Normalize around 100, to get + for regression and - for improvements
+            let pct_change = pct_change - 100.0;
+            changes.push(Entry {
+                job,
+                before: duration_before,
+                after: duration_after,
+                change: pct_change,
+            });
+        }
+    }
+    changes.sort_by(|e1, e2| e1.change.partial_cmp(&e2.change).unwrap().reverse());
+
+    println!("# Job duration changes");
+    for (index, entry) in changes.into_iter().take(10).enumerate() {
+        println!(
+            "{}. `{}`: {:.1}s -> {:.1}s ({:.1}%)",
+            index + 1,
+            entry.job,
+            entry.before.as_secs_f64(),
+            entry.after.as_secs_f64(),
+            entry.change
+        );
+    }
+
+    println!();
+    output_details("How to interpret the job duration changes?", || {
+        println!(
+            r#"Job durations can vary a lot, based on the actual runner instance
+that executed the job, system noise, invalidated caches, etc. The table above is provided
+mostly for t-infra members, for simpler debugging of potential CI slow-downs."#
+        );
+    });
+}
+
 #[derive(Default)]
 struct TestSuiteRecord {
     passed: u64,
diff --git a/src/ci/citool/src/main.rs b/src/ci/citool/src/main.rs
index 5f5c50dc43a..6db5eab458c 100644
--- a/src/ci/citool/src/main.rs
+++ b/src/ci/citool/src/main.rs
@@ -15,7 +15,7 @@ use clap::Parser;
 use jobs::JobDatabase;
 use serde_yaml::Value;
 
-use crate::analysis::output_test_diffs;
+use crate::analysis::{output_largest_duration_changes, output_test_diffs};
 use crate::cpu_usage::load_cpu_usage;
 use crate::datadog::upload_datadog_metric;
 use crate::jobs::RunType;
@@ -160,7 +160,7 @@ fn postprocess_metrics(
                     job_name,
                     JobMetrics { parent: Some(parent_metrics), current: metrics },
                 )]);
-                output_test_diffs(job_metrics);
+                output_test_diffs(&job_metrics);
                 return Ok(());
             }
             Err(error) => {
@@ -180,7 +180,8 @@ fn post_merge_report(db: JobDatabase, current: String, parent: String) -> anyhow
     let metrics = download_auto_job_metrics(&db, &parent, &current)?;
 
     println!("\nComparing {parent} (parent) -> {current} (this PR)\n");
-    output_test_diffs(metrics);
+    output_test_diffs(&metrics);
+    output_largest_duration_changes(&metrics);
 
     Ok(())
 }
diff --git a/src/ci/citool/src/metrics.rs b/src/ci/citool/src/metrics.rs
index 086aa5009f3..a816fb3c4f1 100644
--- a/src/ci/citool/src/metrics.rs
+++ b/src/ci/citool/src/metrics.rs
@@ -1,5 +1,5 @@
 use std::collections::HashMap;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 use anyhow::Context;
 use build_helper::metrics::{JsonNode, JsonRoot, TestSuite};
@@ -74,6 +74,17 @@ Maybe it was newly added?"#,
 }
 
 pub fn download_job_metrics(job_name: &str, sha: &str) -> anyhow::Result<JsonRoot> {
+    // Best effort cache to speed-up local re-executions of citool
+    let cache_path = PathBuf::from(".citool-cache").join(sha).join(format!("{job_name}.json"));
+    if cache_path.is_file() {
+        if let Ok(metrics) = std::fs::read_to_string(&cache_path)
+            .map_err(|err| err.into())
+            .and_then(|data| anyhow::Ok::<JsonRoot>(serde_json::from_str::<JsonRoot>(&data)?))
+        {
+            return Ok(metrics);
+        }
+    }
+
     let url = get_metrics_url(job_name, sha);
     let mut response = ureq::get(&url).call()?;
     if !response.status().is_success() {
@@ -87,6 +98,13 @@ pub fn download_job_metrics(job_name: &str, sha: &str) -> anyhow::Result<JsonRoo
         .body_mut()
         .read_json()
         .with_context(|| anyhow::anyhow!("cannot deserialize metrics from {url}"))?;
+
+    if let Ok(_) = std::fs::create_dir_all(cache_path.parent().unwrap()) {
+        if let Ok(data) = serde_json::to_string(&data) {
+            let _ = std::fs::write(cache_path, data);
+        }
+    }
+
     Ok(data)
 }
 
diff --git a/src/ci/citool/src/utils.rs b/src/ci/citool/src/utils.rs
index b9b1bf4d455..a4c6ff85ef7 100644
--- a/src/ci/citool/src/utils.rs
+++ b/src/ci/citool/src/utils.rs
@@ -23,7 +23,6 @@ where
     println!(
         r"<details>
 <summary>{summary}</summary>
-
 "
     );
     func();
diff --git a/src/ci/docker/run.sh b/src/ci/docker/run.sh
index 2805bb1118d..00d791eeb6b 100755
--- a/src/ci/docker/run.sh
+++ b/src/ci/docker/run.sh
@@ -355,6 +355,8 @@ docker \
   --env GITHUB_ACTIONS \
   --env GITHUB_REF \
   --env GITHUB_STEP_SUMMARY="/checkout/obj/${SUMMARY_FILE}" \
+  --env GITHUB_WORKFLOW_RUN_ID \
+  --env GITHUB_REPOSITORY \
   --env RUST_BACKTRACE \
   --env TOOLSTATE_REPO_ACCESS_TOKEN \
   --env TOOLSTATE_REPO \
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index ada370a8d52..c08ae168d69 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -1943,14 +1943,11 @@ fn clean_trait_object_lifetime_bound<'tcx>(
     // latter contrary to `clean_middle_region`.
     match *region {
         ty::ReStatic => Some(Lifetime::statik()),
-        ty::ReEarlyParam(region) if region.name != kw::Empty => Some(Lifetime(region.name)),
-        ty::ReBound(_, ty::BoundRegion { kind: ty::BoundRegionKind::Named(_, name), .. })
-            if name != kw::Empty =>
-        {
+        ty::ReEarlyParam(region) => Some(Lifetime(region.name)),
+        ty::ReBound(_, ty::BoundRegion { kind: ty::BoundRegionKind::Named(_, name), .. }) => {
             Some(Lifetime(name))
         }
-        ty::ReEarlyParam(_)
-        | ty::ReBound(..)
+        ty::ReBound(..)
         | ty::ReLateParam(_)
         | ty::ReVar(_)
         | ty::RePlaceholder(_)
@@ -2773,7 +2770,7 @@ fn add_without_unwanted_attributes<'hir>(
                 if ident == sym::doc {
                     filter_doc_attr(&mut normal.args, is_inline);
                     attrs.push((Cow::Owned(attr), import_parent));
-                } else if is_inline || ident != sym::cfg {
+                } else if is_inline || ident != sym::cfg_trace {
                     // If it's not a `cfg()` attribute, we keep it.
                     attrs.push((Cow::Owned(attr), import_parent));
                 }
diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
index 2cfbbd75de8..143191a6f2a 100644
--- a/src/librustdoc/clean/types.rs
+++ b/src/librustdoc/clean/types.rs
@@ -1068,7 +1068,7 @@ pub(crate) fn extract_cfg_from_attrs<'a, I: Iterator<Item = &'a hir::Attribute>
             // `doc(cfg())` overrides `cfg()`).
             attrs
                 .clone()
-                .filter(|attr| attr.has_name(sym::cfg))
+                .filter(|attr| attr.has_name(sym::cfg_trace))
                 .filter_map(|attr| single(attr.meta_item_list()?))
                 .filter_map(|attr| Cfg::parse_without(attr.meta_item()?, hidden_cfg).ok().flatten())
                 .fold(Cfg::True, |cfg, new_cfg| cfg & new_cfg)
diff --git a/src/tools/clippy/clippy_lints/src/attrs/duplicated_attributes.rs b/src/tools/clippy/clippy_lints/src/attrs/duplicated_attributes.rs
index 5c486eb90cc..4c84e61b1f2 100644
--- a/src/tools/clippy/clippy_lints/src/attrs/duplicated_attributes.rs
+++ b/src/tools/clippy/clippy_lints/src/attrs/duplicated_attributes.rs
@@ -37,7 +37,6 @@ fn check_duplicated_attr(
     let Some(ident) = attr.ident() else { return };
     let name = ident.name;
     if name == sym::doc
-        || name == sym::cfg_attr
         || name == sym::cfg_attr_trace
         || name == sym::rustc_on_unimplemented
         || name == sym::reason {
@@ -47,7 +46,7 @@ fn check_duplicated_attr(
         return;
     }
     if let Some(direct_parent) = parent.last()
-        && ["cfg", "cfg_attr"].contains(&direct_parent.as_str())
+        && direct_parent == sym::cfg_trace.as_str()
         && [sym::all, sym::not, sym::any].contains(&name)
     {
         // FIXME: We don't correctly check `cfg`s for now, so if it's more complex than just a one
diff --git a/src/tools/clippy/clippy_lints/src/cfg_not_test.rs b/src/tools/clippy/clippy_lints/src/cfg_not_test.rs
index 84136a2e6c2..7590fe96fd2 100644
--- a/src/tools/clippy/clippy_lints/src/cfg_not_test.rs
+++ b/src/tools/clippy/clippy_lints/src/cfg_not_test.rs
@@ -32,7 +32,7 @@ declare_lint_pass!(CfgNotTest => [CFG_NOT_TEST]);
 
 impl EarlyLintPass for CfgNotTest {
     fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &rustc_ast::Attribute) {
-        if attr.has_name(rustc_span::sym::cfg) && contains_not_test(attr.meta_item_list().as_deref(), false) {
+        if attr.has_name(rustc_span::sym::cfg_trace) && contains_not_test(attr.meta_item_list().as_deref(), false) {
             span_lint_and_then(
                 cx,
                 CFG_NOT_TEST,
diff --git a/src/tools/clippy/clippy_lints/src/methods/is_empty.rs b/src/tools/clippy/clippy_lints/src/methods/is_empty.rs
index 7c190e123b7..4c81b22861b 100644
--- a/src/tools/clippy/clippy_lints/src/methods/is_empty.rs
+++ b/src/tools/clippy/clippy_lints/src/methods/is_empty.rs
@@ -41,7 +41,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, receiver: &Expr<'_
 fn is_under_cfg(cx: &LateContext<'_>, id: HirId) -> bool {
     cx.tcx
         .hir_parent_id_iter(id)
-        .any(|id| cx.tcx.hir_attrs(id).iter().any(|attr| attr.has_name(sym::cfg)))
+        .any(|id| cx.tcx.hir_attrs(id).iter().any(|attr| attr.has_name(sym::cfg_trace)))
 }
 
 /// Similar to [`clippy_utils::expr_or_init`], but does not go up the chain if the initialization
diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs
index 1307ff79bc5..668b0cb69e2 100644
--- a/src/tools/clippy/clippy_utils/src/lib.rs
+++ b/src/tools/clippy/clippy_utils/src/lib.rs
@@ -2629,7 +2629,7 @@ pub fn peel_ref_operators<'hir>(cx: &LateContext<'_>, mut expr: &'hir Expr<'hir>
 pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool {
     if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind {
         if let Res::Def(_, def_id) = path.res {
-            return cx.tcx.has_attr(def_id, sym::cfg) || cx.tcx.has_attr(def_id, sym::cfg_attr);
+            return cx.tcx.has_attr(def_id, sym::cfg_trace) || cx.tcx.has_attr(def_id, sym::cfg_attr);
         }
     }
     false
@@ -2699,7 +2699,7 @@ pub fn is_in_test_function(tcx: TyCtxt<'_>, id: HirId) -> bool {
 /// use [`is_in_cfg_test`]
 pub fn is_cfg_test(tcx: TyCtxt<'_>, id: HirId) -> bool {
     tcx.hir_attrs(id).iter().any(|attr| {
-        if attr.has_name(sym::cfg)
+        if attr.has_name(sym::cfg_trace)
             && let Some(items) = attr.meta_item_list()
             && let [item] = &*items
             && item.has_name(sym::test)
@@ -2723,11 +2723,11 @@ pub fn is_in_test(tcx: TyCtxt<'_>, hir_id: HirId) -> bool {
 
 /// Checks if the item of any of its parents has `#[cfg(...)]` attribute applied.
 pub fn inherits_cfg(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
-    tcx.has_attr(def_id, sym::cfg)
+    tcx.has_attr(def_id, sym::cfg_trace)
         || tcx
             .hir_parent_iter(tcx.local_def_id_to_hir_id(def_id))
             .flat_map(|(parent_id, _)| tcx.hir_attrs(parent_id))
-            .any(|attr| attr.has_name(sym::cfg))
+            .any(|attr| attr.has_name(sym::cfg_trace))
 }
 
 /// Walks up the HIR tree from the given expression in an attempt to find where the value is
diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs
index 3ec984edacb..950566b2582 100644
--- a/src/tools/compiletest/src/lib.rs
+++ b/src/tools/compiletest/src/lib.rs
@@ -22,6 +22,7 @@ pub mod util;
 use core::panic;
 use std::collections::HashSet;
 use std::ffi::OsString;
+use std::fmt::Write;
 use std::io::{self, ErrorKind};
 use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
@@ -570,18 +571,22 @@ pub fn run_tests(config: Arc<Config>) {
             // easy to miss which tests failed, and as such fail to reproduce
             // the failure locally.
 
-            println!(
-                "Some tests failed in compiletest suite={}{} mode={} host={} target={}",
-                config.suite,
-                config
-                    .compare_mode
-                    .as_ref()
-                    .map(|c| format!(" compare_mode={:?}", c))
-                    .unwrap_or_default(),
-                config.mode,
-                config.host,
-                config.target
-            );
+            let mut msg = String::from("Some tests failed in compiletest");
+            write!(msg, " suite={}", config.suite).unwrap();
+
+            if let Some(compare_mode) = config.compare_mode.as_ref() {
+                write!(msg, " compare_mode={}", compare_mode).unwrap();
+            }
+
+            if let Some(pass_mode) = config.force_pass_mode.as_ref() {
+                write!(msg, " pass_mode={}", pass_mode).unwrap();
+            }
+
+            write!(msg, " mode={}", config.mode).unwrap();
+            write!(msg, " host={}", config.host).unwrap();
+            write!(msg, " target={}", config.target).unwrap();
+
+            println!("{msg}");
 
             std::process::exit(1);
         }