Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit de018e3

Browse files
committedMar 14, 2025·
Auto merge of rust-lang#138486 - jhpratt:rollup-765wijj, r=jhpratt
Rollup of 6 pull requests Successful merges: - rust-lang#134720 (Display valid crate types in error message for --crate-type flag) - rust-lang#137424 (uefi: helpers: Add DevicePathNode abstractions) - rust-lang#137736 (Don't attempt to export compiler-builtins symbols from rust dylibs) - rust-lang#138451 (Build GCC on CI with GCC, not Clang) - rust-lang#138454 (Improve post-merge workflow) - rust-lang#138477 (Deny impls for `BikeshedGuaranteedNoDrop`) r? `@ghost` `@rustbot` modify labels: rollup
2 parents f7b4354 + 98c85a3 commit de018e3

File tree

21 files changed

+447
-138
lines changed

21 files changed

+447
-138
lines changed
 

‎.github/workflows/post-merge.yml

+6-1
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,13 @@ jobs:
3535
3636
cd src/ci/citool
3737
38-
echo "Post-merge analysis result" > output.log
38+
printf "*This is an experimental post-merge analysis report. You can ignore it.*\n\n" > output.log
39+
printf "<details>\n<summary>Post-merge report</summary>\n\n" >> output.log
40+
3941
cargo run --release post-merge-report ${PARENT_COMMIT} ${{ github.sha }} >> output.log
42+
43+
printf "</details>\n" >> output.log
44+
4045
cat output.log
4146
4247
gh pr comment ${HEAD_PR} -F output.log

‎compiler/rustc_codegen_ssa/src/back/linker.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -1784,7 +1784,10 @@ fn exported_symbols_for_non_proc_macro(tcx: TyCtxt<'_>, crate_type: CrateType) -
17841784
let mut symbols = Vec::new();
17851785
let export_threshold = symbol_export::crates_export_threshold(&[crate_type]);
17861786
for_each_exported_symbols_include_dep(tcx, crate_type, |symbol, info, cnum| {
1787-
if info.level.is_below_threshold(export_threshold) {
1787+
// Do not export mangled symbols from cdylibs and don't attempt to export compiler-builtins
1788+
// from any cdylib. The latter doesn't work anyway as we use hidden visibility for
1789+
// compiler-builtins. Most linkers silently ignore it, but ld64 gives a warning.
1790+
if info.level.is_below_threshold(export_threshold) && !tcx.is_compiler_builtins(cnum) {
17881791
symbols.push(symbol_export::exporting_symbol_name_for_instance_in_crate(
17891792
tcx, symbol, cnum,
17901793
));

‎compiler/rustc_passes/messages.ftl

+2-2
Original file line numberDiff line numberDiff line change
@@ -811,8 +811,8 @@ passes_unused_duplicate =
811811
passes_unused_empty_lints_note =
812812
attribute `{$name}` with an empty list has no effect
813813
814-
passes_unused_linker_warnings_note =
815-
the `linker_warnings` lint can only be controlled at the root of a crate that needs to be linked
814+
passes_unused_linker_messages_note =
815+
the `linker_messages` lint can only be controlled at the root of a crate that needs to be linked
816816
817817
passes_unused_multiple =
818818
multiple `{$name}` attributes

‎compiler/rustc_passes/src/check_attr.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2387,7 +2387,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
23872387
.iter()
23882388
.all(|kind| matches!(kind, CrateType::Rlib | CrateType::Staticlib));
23892389
if never_needs_link {
2390-
errors::UnusedNote::LinkerWarningsBinaryCrateOnly
2390+
errors::UnusedNote::LinkerMessagesBinaryCrateOnly
23912391
} else {
23922392
return;
23932393
}

‎compiler/rustc_passes/src/errors.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -770,8 +770,8 @@ pub(crate) enum UnusedNote {
770770
NoLints { name: Symbol },
771771
#[note(passes_unused_default_method_body_const_note)]
772772
DefaultMethodBodyConst,
773-
#[note(passes_unused_linker_warnings_note)]
774-
LinkerWarningsBinaryCrateOnly,
773+
#[note(passes_unused_linker_messages_note)]
774+
LinkerMessagesBinaryCrateOnly,
775775
}
776776

777777
#[derive(LintDiagnostic)]

‎compiler/rustc_session/src/config.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -2709,7 +2709,12 @@ pub fn parse_crate_types_from_list(list_list: Vec<String>) -> Result<Vec<CrateTy
27092709
"cdylib" => CrateType::Cdylib,
27102710
"bin" => CrateType::Executable,
27112711
"proc-macro" => CrateType::ProcMacro,
2712-
_ => return Err(format!("unknown crate type: `{part}`")),
2712+
_ => {
2713+
return Err(format!(
2714+
"unknown crate type: `{part}`, expected one of: \
2715+
`lib`, `rlib`, `staticlib`, `dylib`, `cdylib`, `bin`, `proc-macro`",
2716+
));
2717+
}
27132718
};
27142719
if !crate_types.contains(&new_part) {
27152720
crate_types.push(new_part)

‎library/core/src/marker.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -465,9 +465,13 @@ impl<T: ?Sized> Copy for &T {}
465465
/// Notably, this doesn't include all trivially-destructible types for semver
466466
/// reasons.
467467
///
468-
/// Bikeshed name for now.
468+
/// Bikeshed name for now. This trait does not do anything other than reflect the
469+
/// set of types that are allowed within unions for field validity.
469470
#[unstable(feature = "bikeshed_guaranteed_no_drop", issue = "none")]
470471
#[lang = "bikeshed_guaranteed_no_drop"]
472+
#[rustc_deny_explicit_impl]
473+
#[rustc_do_not_implement_via_object]
474+
#[doc(hidden)]
471475
pub trait BikeshedGuaranteedNoDrop {}
472476

473477
/// Types for which it is safe to share references between threads.

‎library/std/src/sys/pal/uefi/helpers.rs

+179
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,60 @@ pub(crate) fn device_path_to_text(path: NonNull<device_path::Protocol>) -> io::R
216216
Err(io::const_error!(io::ErrorKind::NotFound, "no device path to text protocol found"))
217217
}
218218

219+
fn device_node_to_text(path: NonNull<device_path::Protocol>) -> io::Result<OsString> {
220+
fn node_to_text(
221+
protocol: NonNull<device_path_to_text::Protocol>,
222+
path: NonNull<device_path::Protocol>,
223+
) -> io::Result<OsString> {
224+
let path_ptr: *mut r_efi::efi::Char16 = unsafe {
225+
((*protocol.as_ptr()).convert_device_node_to_text)(
226+
path.as_ptr(),
227+
// DisplayOnly
228+
r_efi::efi::Boolean::FALSE,
229+
// AllowShortcuts
230+
r_efi::efi::Boolean::FALSE,
231+
)
232+
};
233+
234+
let path = os_string_from_raw(path_ptr)
235+
.ok_or(io::const_error!(io::ErrorKind::InvalidData, "Invalid path"))?;
236+
237+
if let Some(boot_services) = crate::os::uefi::env::boot_services() {
238+
let boot_services: NonNull<r_efi::efi::BootServices> = boot_services.cast();
239+
unsafe {
240+
((*boot_services.as_ptr()).free_pool)(path_ptr.cast());
241+
}
242+
}
243+
244+
Ok(path)
245+
}
246+
247+
static LAST_VALID_HANDLE: AtomicPtr<crate::ffi::c_void> =
248+
AtomicPtr::new(crate::ptr::null_mut());
249+
250+
if let Some(handle) = NonNull::new(LAST_VALID_HANDLE.load(Ordering::Acquire)) {
251+
if let Ok(protocol) = open_protocol::<device_path_to_text::Protocol>(
252+
handle,
253+
device_path_to_text::PROTOCOL_GUID,
254+
) {
255+
return node_to_text(protocol, path);
256+
}
257+
}
258+
259+
let device_path_to_text_handles = locate_handles(device_path_to_text::PROTOCOL_GUID)?;
260+
for handle in device_path_to_text_handles {
261+
if let Ok(protocol) = open_protocol::<device_path_to_text::Protocol>(
262+
handle,
263+
device_path_to_text::PROTOCOL_GUID,
264+
) {
265+
LAST_VALID_HANDLE.store(handle.as_ptr(), Ordering::Release);
266+
return node_to_text(protocol, path);
267+
}
268+
}
269+
270+
Err(io::const_error!(io::ErrorKind::NotFound, "No device path to text protocol found"))
271+
}
272+
219273
/// Gets RuntimeServices.
220274
pub(crate) fn runtime_services() -> Option<NonNull<r_efi::efi::RuntimeServices>> {
221275
let system_table: NonNull<r_efi::efi::SystemTable> =
@@ -319,6 +373,11 @@ impl<'a> BorrowedDevicePath<'a> {
319373
pub(crate) fn to_text(&self) -> io::Result<OsString> {
320374
device_path_to_text(self.protocol)
321375
}
376+
377+
#[expect(dead_code)]
378+
pub(crate) const fn iter(&'a self) -> DevicePathIterator<'a> {
379+
DevicePathIterator::new(DevicePathNode::new(self.protocol))
380+
}
322381
}
323382

324383
impl<'a> crate::fmt::Debug for BorrowedDevicePath<'a> {
@@ -330,6 +389,126 @@ impl<'a> crate::fmt::Debug for BorrowedDevicePath<'a> {
330389
}
331390
}
332391

392+
pub(crate) struct DevicePathIterator<'a>(Option<DevicePathNode<'a>>);
393+
394+
impl<'a> DevicePathIterator<'a> {
395+
const fn new(node: DevicePathNode<'a>) -> Self {
396+
if node.is_end() { Self(None) } else { Self(Some(node)) }
397+
}
398+
}
399+
400+
impl<'a> Iterator for DevicePathIterator<'a> {
401+
type Item = DevicePathNode<'a>;
402+
403+
fn next(&mut self) -> Option<Self::Item> {
404+
let cur_node = self.0?;
405+
406+
let next_node = unsafe { cur_node.next_node() };
407+
self.0 = if next_node.is_end() { None } else { Some(next_node) };
408+
409+
Some(cur_node)
410+
}
411+
}
412+
413+
#[derive(Copy, Clone)]
414+
pub(crate) struct DevicePathNode<'a> {
415+
protocol: NonNull<r_efi::protocols::device_path::Protocol>,
416+
phantom: PhantomData<&'a r_efi::protocols::device_path::Protocol>,
417+
}
418+
419+
impl<'a> DevicePathNode<'a> {
420+
pub(crate) const fn new(protocol: NonNull<r_efi::protocols::device_path::Protocol>) -> Self {
421+
Self { protocol, phantom: PhantomData }
422+
}
423+
424+
pub(crate) const fn length(&self) -> u16 {
425+
let len = unsafe { (*self.protocol.as_ptr()).length };
426+
u16::from_le_bytes(len)
427+
}
428+
429+
pub(crate) const fn node_type(&self) -> u8 {
430+
unsafe { (*self.protocol.as_ptr()).r#type }
431+
}
432+
433+
pub(crate) const fn sub_type(&self) -> u8 {
434+
unsafe { (*self.protocol.as_ptr()).sub_type }
435+
}
436+
437+
pub(crate) fn data(&self) -> &[u8] {
438+
let length: usize = self.length().into();
439+
440+
// Some nodes do not have any special data
441+
if length > 4 {
442+
let raw_ptr: *const u8 = self.protocol.as_ptr().cast();
443+
let data = unsafe { raw_ptr.add(4) };
444+
unsafe { crate::slice::from_raw_parts(data, length - 4) }
445+
} else {
446+
&[]
447+
}
448+
}
449+
450+
pub(crate) const fn is_end(&self) -> bool {
451+
self.node_type() == r_efi::protocols::device_path::TYPE_END
452+
&& self.sub_type() == r_efi::protocols::device_path::End::SUBTYPE_ENTIRE
453+
}
454+
455+
#[expect(dead_code)]
456+
pub(crate) const fn is_end_instance(&self) -> bool {
457+
self.node_type() == r_efi::protocols::device_path::TYPE_END
458+
&& self.sub_type() == r_efi::protocols::device_path::End::SUBTYPE_INSTANCE
459+
}
460+
461+
pub(crate) unsafe fn next_node(&self) -> Self {
462+
let node = unsafe {
463+
self.protocol
464+
.cast::<u8>()
465+
.add(self.length().into())
466+
.cast::<r_efi::protocols::device_path::Protocol>()
467+
};
468+
Self::new(node)
469+
}
470+
471+
#[expect(dead_code)]
472+
pub(crate) fn to_path(&'a self) -> BorrowedDevicePath<'a> {
473+
BorrowedDevicePath::new(self.protocol)
474+
}
475+
476+
pub(crate) fn to_text(&self) -> io::Result<OsString> {
477+
device_node_to_text(self.protocol)
478+
}
479+
}
480+
481+
impl<'a> PartialEq for DevicePathNode<'a> {
482+
fn eq(&self, other: &Self) -> bool {
483+
let self_len = self.length();
484+
let other_len = other.length();
485+
486+
self_len == other_len
487+
&& unsafe {
488+
compiler_builtins::mem::memcmp(
489+
self.protocol.as_ptr().cast(),
490+
other.protocol.as_ptr().cast(),
491+
usize::from(self_len),
492+
) == 0
493+
}
494+
}
495+
}
496+
497+
impl<'a> crate::fmt::Debug for DevicePathNode<'a> {
498+
fn fmt(&self, f: &mut crate::fmt::Formatter<'_>) -> crate::fmt::Result {
499+
match self.to_text() {
500+
Ok(p) => p.fmt(f),
501+
Err(_) => f
502+
.debug_struct("DevicePathNode")
503+
.field("type", &self.node_type())
504+
.field("sub_type", &self.sub_type())
505+
.field("length", &self.length())
506+
.field("specific_device_path_data", &self.data())
507+
.finish(),
508+
}
509+
}
510+
}
511+
333512
pub(crate) struct OwnedProtocol<T> {
334513
guid: r_efi::efi::Guid,
335514
handle: NonNull<crate::ffi::c_void>,

‎src/bootstrap/src/core/build_steps/dist.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2481,7 +2481,7 @@ impl Step for Gcc {
24812481
fn run(self, builder: &Builder<'_>) -> Self::Output {
24822482
let tarball = Tarball::new(builder, "gcc", &self.target.triple);
24832483
let output = builder.ensure(super::gcc::Gcc { target: self.target });
2484-
tarball.add_file(output.libgccjit, ".", 0o644);
2484+
tarball.add_file(output.libgccjit, "lib", 0o644);
24852485
tarball.generate()
24862486
}
24872487
}

‎src/bootstrap/src/core/build_steps/gcc.rs

+24-14
Original file line numberDiff line numberDiff line change
@@ -63,18 +63,23 @@ impl Step for Gcc {
6363
}
6464

6565
build_gcc(&metadata, builder, target);
66-
67-
let lib_alias = metadata.install_dir.join("lib/libgccjit.so.0");
68-
if !lib_alias.exists() {
69-
t!(builder.symlink_file(&libgccjit_path, lib_alias));
70-
}
66+
create_lib_alias(builder, &libgccjit_path);
7167

7268
t!(metadata.stamp.write());
7369

7470
GccOutput { libgccjit: libgccjit_path }
7571
}
7672
}
7773

74+
/// Creates a libgccjit.so.0 alias next to libgccjit.so if it does not
75+
/// already exist
76+
fn create_lib_alias(builder: &Builder<'_>, libgccjit: &PathBuf) {
77+
let lib_alias = libgccjit.parent().unwrap().join("libgccjit.so.0");
78+
if !lib_alias.exists() {
79+
t!(builder.symlink_file(libgccjit, lib_alias));
80+
}
81+
}
82+
7883
pub struct Meta {
7984
stamp: BuildStamp,
8085
out_dir: PathBuf,
@@ -109,8 +114,10 @@ fn try_download_gcc(builder: &Builder<'_>, target: TargetSelection) -> Option<Pa
109114
builder.config.download_ci_gcc(&sha, &root);
110115
t!(gcc_stamp.write());
111116
}
112-
// FIXME: put libgccjit.so into a lib directory in dist::Gcc
113-
Some(root.join("libgccjit.so"))
117+
118+
let libgccjit = root.join("lib").join("libgccjit.so");
119+
create_lib_alias(builder, &libgccjit);
120+
Some(libgccjit)
114121
}
115122

116123
#[cfg(test)]
@@ -177,6 +184,14 @@ fn libgccjit_built_path(install_dir: &Path) -> PathBuf {
177184
}
178185

179186
fn build_gcc(metadata: &Meta, builder: &Builder<'_>, target: TargetSelection) {
187+
if builder.build.cc_tool(target).is_like_clang()
188+
|| builder.build.cxx_tool(target).is_like_clang()
189+
{
190+
panic!(
191+
"Attempting to build GCC using Clang, which is known to misbehave. Please use GCC as the host C/C++ compiler. "
192+
);
193+
}
194+
180195
let Meta { stamp: _, out_dir, install_dir, root } = metadata;
181196

182197
t!(fs::create_dir_all(out_dir));
@@ -203,18 +218,13 @@ fn build_gcc(metadata: &Meta, builder: &Builder<'_>, target: TargetSelection) {
203218
let mut configure_cmd = command(src_dir.join("configure"));
204219
configure_cmd
205220
.current_dir(out_dir)
206-
// On CI, we compile GCC with Clang.
207-
// The -Wno-everything flag is needed to make GCC compile with Clang 19.
208-
// `-g -O2` are the default flags that are otherwise used by Make.
209-
// FIXME(kobzol): change the flags once we have [gcc] configuration in config.toml.
210-
.env("CXXFLAGS", "-Wno-everything -g -O2")
211-
.env("CFLAGS", "-Wno-everything -g -O2")
212221
.arg("--enable-host-shared")
213-
.arg("--enable-languages=jit")
222+
.arg("--enable-languages=c,jit,lto")
214223
.arg("--enable-checking=release")
215224
.arg("--disable-bootstrap")
216225
.arg("--disable-multilib")
217226
.arg(format!("--prefix={}", install_dir.display()));
227+
218228
let cc = builder.build.cc(target).display().to_string();
219229
let cc = builder
220230
.build

‎src/bootstrap/src/lib.rs

+11
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use std::{env, fs, io, str};
2727

2828
use build_helper::ci::gha;
2929
use build_helper::exit;
30+
use cc::Tool;
3031
use termcolor::{ColorChoice, StandardStream, WriteColor};
3132
use utils::build_stamp::BuildStamp;
3233
use utils::channel::GitInfo;
@@ -1218,6 +1219,16 @@ Executed at: {executed_at}"#,
12181219
self.cc.borrow()[&target].path().into()
12191220
}
12201221

1222+
/// Returns the internal `cc::Tool` for the C compiler.
1223+
fn cc_tool(&self, target: TargetSelection) -> Tool {
1224+
self.cc.borrow()[&target].clone()
1225+
}
1226+
1227+
/// Returns the internal `cc::Tool` for the C++ compiler.
1228+
fn cxx_tool(&self, target: TargetSelection) -> Tool {
1229+
self.cxx.borrow()[&target].clone()
1230+
}
1231+
12211232
/// Returns C flags that `cc-rs` thinks should be enabled for the
12221233
/// specified target by default.
12231234
fn cc_handled_clags(&self, target: TargetSelection, c: CLang) -> Vec<String> {

‎src/ci/citool/src/merge_report.rs

+138-77
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
use std::cmp::Reverse;
2-
use std::collections::HashMap;
1+
use std::collections::{HashMap, HashSet};
2+
use std::path::PathBuf;
33

44
use anyhow::Context;
5-
use build_helper::metrics::{JsonRoot, TestOutcome};
5+
use build_helper::metrics::{JsonRoot, TestOutcome, TestSuiteMetadata};
66

77
use crate::jobs::JobDatabase;
88
use crate::metrics::get_test_suites;
@@ -13,8 +13,10 @@ type JobName = String;
1313
/// Computes a post merge CI analysis report between the `parent` and `current` commits.
1414
pub fn post_merge_report(job_db: JobDatabase, parent: Sha, current: Sha) -> anyhow::Result<()> {
1515
let jobs = download_all_metrics(&job_db, &parent, &current)?;
16-
let diffs = aggregate_test_diffs(&jobs)?;
17-
report_test_changes(diffs);
16+
let aggregated_test_diffs = aggregate_test_diffs(&jobs)?;
17+
18+
println!("Comparing {parent} (base) -> {current} (this PR)\n");
19+
report_test_diffs(aggregated_test_diffs);
1820

1921
Ok(())
2022
}
@@ -54,7 +56,16 @@ Maybe it was newly added?"#,
5456
Ok(jobs)
5557
}
5658

59+
/// Downloads job metrics of the given job for the given commit.
60+
/// Caches the result on the local disk.
5761
fn download_job_metrics(job_name: &str, sha: &str) -> anyhow::Result<JsonRoot> {
62+
let cache_path = PathBuf::from(".citool-cache").join(sha).join(job_name).join("metrics.json");
63+
if let Some(cache_entry) =
64+
std::fs::read_to_string(&cache_path).ok().and_then(|data| serde_json::from_str(&data).ok())
65+
{
66+
return Ok(cache_entry);
67+
}
68+
5869
let url = get_metrics_url(job_name, sha);
5970
let mut response = ureq::get(&url).call()?;
6071
if !response.status().is_success() {
@@ -68,6 +79,13 @@ fn download_job_metrics(job_name: &str, sha: &str) -> anyhow::Result<JsonRoot> {
6879
.body_mut()
6980
.read_json()
7081
.with_context(|| anyhow::anyhow!("cannot deserialize metrics from {url}"))?;
82+
83+
// Ignore errors if cache cannot be created
84+
if std::fs::create_dir_all(cache_path.parent().unwrap()).is_ok() {
85+
if let Ok(serialized) = serde_json::to_string(&data) {
86+
let _ = std::fs::write(&cache_path, &serialized);
87+
}
88+
}
7189
Ok(data)
7290
}
7391

@@ -76,81 +94,80 @@ fn get_metrics_url(job_name: &str, sha: &str) -> String {
7694
format!("https://ci-artifacts.rust-lang.org/rustc-builds{suffix}/{sha}/metrics-{job_name}.json")
7795
}
7896

97+
/// Represents a difference in the outcome of tests between a base and a current commit.
98+
/// Maps test diffs to jobs that contained them.
99+
#[derive(Debug)]
100+
struct AggregatedTestDiffs {
101+
diffs: HashMap<TestDiff, Vec<JobName>>,
102+
}
103+
79104
fn aggregate_test_diffs(
80105
jobs: &HashMap<JobName, JobMetrics>,
81-
) -> anyhow::Result<Vec<AggregatedTestDiffs>> {
82-
let mut job_diffs = vec![];
106+
) -> anyhow::Result<AggregatedTestDiffs> {
107+
let mut diffs: HashMap<TestDiff, Vec<JobName>> = HashMap::new();
83108

84109
// Aggregate test suites
85110
for (name, metrics) in jobs {
86111
if let Some(parent) = &metrics.parent {
87112
let tests_parent = aggregate_tests(parent);
88113
let tests_current = aggregate_tests(&metrics.current);
89-
let test_diffs = calculate_test_diffs(tests_parent, tests_current);
90-
if !test_diffs.is_empty() {
91-
job_diffs.push((name.clone(), test_diffs));
114+
for diff in calculate_test_diffs(tests_parent, tests_current) {
115+
diffs.entry(diff).or_default().push(name.to_string());
92116
}
93117
}
94118
}
95119

96-
// Aggregate jobs with the same diff, as often the same diff will appear in many jobs
97-
let job_diffs: HashMap<Vec<(Test, TestOutcomeDiff)>, Vec<String>> =
98-
job_diffs.into_iter().fold(HashMap::new(), |mut acc, (job, diffs)| {
99-
acc.entry(diffs).or_default().push(job);
100-
acc
101-
});
120+
Ok(AggregatedTestDiffs { diffs })
121+
}
102122

103-
Ok(job_diffs
104-
.into_iter()
105-
.map(|(test_diffs, jobs)| AggregatedTestDiffs { jobs, test_diffs })
106-
.collect())
123+
#[derive(Eq, PartialEq, Hash, Debug)]
124+
enum TestOutcomeDiff {
125+
ChangeOutcome { before: TestOutcome, after: TestOutcome },
126+
Missing { before: TestOutcome },
127+
Added(TestOutcome),
107128
}
108129

109-
fn calculate_test_diffs(
110-
reference: TestSuiteData,
111-
current: TestSuiteData,
112-
) -> Vec<(Test, TestOutcomeDiff)> {
113-
let mut diffs = vec![];
130+
#[derive(Eq, PartialEq, Hash, Debug)]
131+
struct TestDiff {
132+
test: Test,
133+
diff: TestOutcomeDiff,
134+
}
135+
136+
fn calculate_test_diffs(parent: TestSuiteData, current: TestSuiteData) -> HashSet<TestDiff> {
137+
let mut diffs = HashSet::new();
114138
for (test, outcome) in &current.tests {
115-
match reference.tests.get(test) {
139+
match parent.tests.get(test) {
116140
Some(before) => {
117141
if before != outcome {
118-
diffs.push((
119-
test.clone(),
120-
TestOutcomeDiff::ChangeOutcome {
142+
diffs.insert(TestDiff {
143+
test: test.clone(),
144+
diff: TestOutcomeDiff::ChangeOutcome {
121145
before: before.clone(),
122146
after: outcome.clone(),
123147
},
124-
));
148+
});
125149
}
126150
}
127-
None => diffs.push((test.clone(), TestOutcomeDiff::Added(outcome.clone()))),
151+
None => {
152+
diffs.insert(TestDiff {
153+
test: test.clone(),
154+
diff: TestOutcomeDiff::Added(outcome.clone()),
155+
});
156+
}
128157
}
129158
}
130-
for (test, outcome) in &reference.tests {
159+
for (test, outcome) in &parent.tests {
131160
if !current.tests.contains_key(test) {
132-
diffs.push((test.clone(), TestOutcomeDiff::Missing { before: outcome.clone() }));
161+
diffs.insert(TestDiff {
162+
test: test.clone(),
163+
diff: TestOutcomeDiff::Missing { before: outcome.clone() },
164+
});
133165
}
134166
}
135167

136168
diffs
137169
}
138170

139-
/// Represents a difference in the outcome of tests between a base and a current commit.
140-
#[derive(Debug)]
141-
struct AggregatedTestDiffs {
142-
/// All jobs that had the exact same test diffs.
143-
jobs: Vec<String>,
144-
test_diffs: Vec<(Test, TestOutcomeDiff)>,
145-
}
146-
147-
#[derive(Eq, PartialEq, Hash, Debug)]
148-
enum TestOutcomeDiff {
149-
ChangeOutcome { before: TestOutcome, after: TestOutcome },
150-
Missing { before: TestOutcome },
151-
Added(TestOutcome),
152-
}
153-
154171
/// Aggregates test suite executions from all bootstrap invocations in a given CI job.
155172
#[derive(Default)]
156173
struct TestSuiteData {
@@ -160,6 +177,7 @@ struct TestSuiteData {
160177
#[derive(Hash, PartialEq, Eq, Debug, Clone)]
161178
struct Test {
162179
name: String,
180+
is_doctest: bool,
163181
}
164182

165183
/// Extracts all tests from the passed metrics and map them to their outcomes.
@@ -168,7 +186,10 @@ fn aggregate_tests(metrics: &JsonRoot) -> TestSuiteData {
168186
let test_suites = get_test_suites(&metrics);
169187
for suite in test_suites {
170188
for test in &suite.tests {
171-
let test_entry = Test { name: normalize_test_name(&test.name) };
189+
// Poor man's detection of doctests based on the "(line XYZ)" suffix
190+
let is_doctest = matches!(suite.metadata, TestSuiteMetadata::CargoPackage { .. })
191+
&& test.name.contains("(line");
192+
let test_entry = Test { name: normalize_test_name(&test.name), is_doctest };
172193
tests.insert(test_entry, test.outcome.clone());
173194
}
174195
}
@@ -181,16 +202,13 @@ fn normalize_test_name(name: &str) -> String {
181202
}
182203

183204
/// Prints test changes in Markdown format to stdout.
184-
fn report_test_changes(mut diffs: Vec<AggregatedTestDiffs>) {
205+
fn report_test_diffs(diff: AggregatedTestDiffs) {
185206
println!("## Test differences");
186-
if diffs.is_empty() {
207+
if diff.diffs.is_empty() {
187208
println!("No test diffs found");
188209
return;
189210
}
190211

191-
// Sort diffs in decreasing order by diff count
192-
diffs.sort_by_key(|entry| Reverse(entry.test_diffs.len()));
193-
194212
fn format_outcome(outcome: &TestOutcome) -> String {
195213
match outcome {
196214
TestOutcome::Passed => "pass".to_string(),
@@ -219,36 +237,79 @@ fn report_test_changes(mut diffs: Vec<AggregatedTestDiffs>) {
219237
}
220238
}
221239

222-
let max_diff_count = 10;
223-
let max_job_count = 5;
224-
let max_test_count = 10;
225-
226-
for diff in diffs.iter().take(max_diff_count) {
227-
let mut jobs = diff.jobs.clone();
228-
jobs.sort();
229-
230-
let jobs = jobs.iter().take(max_job_count).map(|j| format!("`{j}`")).collect::<Vec<_>>();
240+
fn format_job_group(group: u64) -> String {
241+
format!("**J{group}**")
242+
}
231243

232-
let extra_jobs = diff.jobs.len().saturating_sub(max_job_count);
233-
let suffix = if extra_jobs > 0 {
234-
format!(" (and {extra_jobs} {})", pluralize("other", extra_jobs))
235-
} else {
236-
String::new()
244+
// It would be quite noisy to repeat the jobs that contained the test changes after/next to
245+
// every test diff. At the same time, grouping the test diffs by
246+
// [unique set of jobs that contained them] also doesn't work well, because the test diffs
247+
// would have to be duplicated several times.
248+
// Instead, we create a set of unique job groups, and then print a job group after each test.
249+
// We then print the job groups at the end, as a sort of index.
250+
let mut grouped_diffs: Vec<(&TestDiff, u64)> = vec![];
251+
let mut job_list_to_group: HashMap<&[JobName], u64> = HashMap::new();
252+
let mut job_index: Vec<&[JobName]> = vec![];
253+
254+
let original_diff_count = diff.diffs.len();
255+
let diffs = diff
256+
.diffs
257+
.into_iter()
258+
.filter(|(diff, _)| !diff.test.is_doctest)
259+
.map(|(diff, mut jobs)| {
260+
jobs.sort();
261+
(diff, jobs)
262+
})
263+
.collect::<Vec<_>>();
264+
let doctest_count = original_diff_count.saturating_sub(diffs.len());
265+
266+
let max_diff_count = 100;
267+
for (diff, jobs) in diffs.iter().take(max_diff_count) {
268+
let jobs = &*jobs;
269+
let job_group = match job_list_to_group.get(jobs.as_slice()) {
270+
Some(id) => *id,
271+
None => {
272+
let id = job_index.len() as u64;
273+
job_index.push(jobs);
274+
job_list_to_group.insert(jobs, id);
275+
id
276+
}
237277
};
238-
println!("- {}{suffix}", jobs.join(","));
278+
grouped_diffs.push((diff, job_group));
279+
}
239280

240-
let extra_tests = diff.test_diffs.len().saturating_sub(max_test_count);
241-
for (test, outcome_diff) in diff.test_diffs.iter().take(max_test_count) {
242-
println!(" - {}: {}", test.name, format_diff(&outcome_diff));
243-
}
244-
if extra_tests > 0 {
245-
println!(" - (and {extra_tests} additional {})", pluralize("tests", extra_tests));
246-
}
281+
// Sort diffs by job group and test name
282+
grouped_diffs.sort_by(|(d1, g1), (d2, g2)| g1.cmp(&g2).then(d1.test.name.cmp(&d2.test.name)));
283+
284+
for (diff, job_group) in grouped_diffs {
285+
println!(
286+
"- `{}`: {} ({})",
287+
diff.test.name,
288+
format_diff(&diff.diff),
289+
format_job_group(job_group)
290+
);
247291
}
248292

249293
let extra_diffs = diffs.len().saturating_sub(max_diff_count);
250294
if extra_diffs > 0 {
251-
println!("\n(and {extra_diffs} additional {})", pluralize("diff", extra_diffs));
295+
println!("\n(and {extra_diffs} additional {})", pluralize("test diff", extra_diffs));
296+
}
297+
298+
if doctest_count > 0 {
299+
println!(
300+
"\nAdditionally, {doctest_count} doctest {} were found. These are ignored, as they are noisy.",
301+
pluralize("diff", doctest_count)
302+
);
303+
}
304+
305+
// Now print the job group index
306+
println!("\n**Job group index**\n");
307+
for (group, jobs) in job_index.into_iter().enumerate() {
308+
println!(
309+
"- {}: {}",
310+
format_job_group(group as u64),
311+
jobs.iter().map(|j| format!("`{j}`")).collect::<Vec<_>>().join(", ")
312+
);
252313
}
253314
}
254315

‎src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile

+3-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,9 @@ ENV SCRIPT python3 ../x.py build --set rust.debug=true opt-dist && \
101101
./build/$HOSTS/stage0-tools-bin/opt-dist linux-ci -- python3 ../x.py dist \
102102
--host $HOSTS --target $HOSTS \
103103
--include-default-paths \
104-
build-manifest bootstrap gcc
104+
build-manifest bootstrap && \
105+
# Use GCC for building GCC, as it seems to behave badly when built with Clang
106+
CC=/rustroot/bin/cc CXX=/rustroot/bin/c++ python3 ../x.py dist gcc
105107
ENV CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_LINKER=clang
106108

107109
# This is the only builder which will create source tarballs

‎src/tools/opt-dist/src/tests.rs

+52-29
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::path::Path;
2+
13
use anyhow::Context;
24
use camino::{Utf8Path, Utf8PathBuf};
35

@@ -86,36 +88,57 @@ llvm-config = "{llvm_config}"
8688
log::info!("Using following `config.toml` for running tests:\n{config_content}");
8789

8890
// Simulate a stage 0 compiler with the extracted optimized dist artifacts.
89-
std::fs::write("config.toml", config_content)?;
90-
91-
let x_py = env.checkout_path().join("x.py");
92-
let mut args = vec![
93-
env.python_binary(),
94-
x_py.as_str(),
95-
"test",
96-
"--build",
97-
env.host_tuple(),
98-
"--stage",
99-
"0",
100-
"tests/assembly",
101-
"tests/codegen",
102-
"tests/codegen-units",
103-
"tests/incremental",
104-
"tests/mir-opt",
105-
"tests/pretty",
106-
"tests/run-make/glibc-symbols-x86_64-unknown-linux-gnu",
107-
"tests/ui",
108-
"tests/crashes",
109-
];
110-
for test_path in env.skipped_tests() {
111-
args.extend(["--skip", test_path]);
91+
with_backed_up_file(Path::new("config.toml"), &config_content, || {
92+
let x_py = env.checkout_path().join("x.py");
93+
let mut args = vec![
94+
env.python_binary(),
95+
x_py.as_str(),
96+
"test",
97+
"--build",
98+
env.host_tuple(),
99+
"--stage",
100+
"0",
101+
"tests/assembly",
102+
"tests/codegen",
103+
"tests/codegen-units",
104+
"tests/incremental",
105+
"tests/mir-opt",
106+
"tests/pretty",
107+
"tests/run-make/glibc-symbols-x86_64-unknown-linux-gnu",
108+
"tests/ui",
109+
"tests/crashes",
110+
];
111+
for test_path in env.skipped_tests() {
112+
args.extend(["--skip", test_path]);
113+
}
114+
cmd(&args)
115+
.env("COMPILETEST_FORCE_STAGE0", "1")
116+
// Also run dist-only tests
117+
.env("COMPILETEST_ENABLE_DIST_TESTS", "1")
118+
.run()
119+
.context("Cannot execute tests")
120+
})
121+
}
122+
123+
/// Backup `path` (if it exists), then write `contents` into it, and then restore the original
124+
/// contents of the file.
125+
fn with_backed_up_file<F>(path: &Path, contents: &str, func: F) -> anyhow::Result<()>
126+
where
127+
F: FnOnce() -> anyhow::Result<()>,
128+
{
129+
let original_contents =
130+
if path.is_file() { Some(std::fs::read_to_string(path)?) } else { None };
131+
132+
// Overwrite it with new contents
133+
std::fs::write(path, contents)?;
134+
135+
let ret = func();
136+
137+
if let Some(original_contents) = original_contents {
138+
std::fs::write(path, original_contents)?;
112139
}
113-
cmd(&args)
114-
.env("COMPILETEST_FORCE_STAGE0", "1")
115-
// Also run dist-only tests
116-
.env("COMPILETEST_ENABLE_DIST_TESTS", "1")
117-
.run()
118-
.context("Cannot execute tests")
140+
141+
ret
119142
}
120143

121144
/// Tries to find the version of the dist artifacts (either nightly, beta, or 1.XY.Z).

‎tests/ui/crate_type_flag.rs

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
//@ compile-flags: --crate-type dynlib
2+
//@ error-pattern: unknown crate type: `dynlib`, expected one of: `lib`, `rlib`, `staticlib`, `dylib`, `cdylib`, `bin`, `proc-macro`
3+
4+
fn main() {}

‎tests/ui/crate_type_flag.stderr

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
error: unknown crate type: `dynlib`, expected one of: `lib`, `rlib`, `staticlib`, `dylib`, `cdylib`, `bin`, `proc-macro`
2+
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
error: unknown crate type: ``
1+
error: unknown crate type: ``, expected one of: `lib`, `rlib`, `staticlib`, `dylib`, `cdylib`, `bin`, `proc-macro`
22

Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
error: unknown crate type: `proc_macro`
1+
error: unknown crate type: `proc_macro`, expected one of: `lib`, `rlib`, `staticlib`, `dylib`, `cdylib`, `bin`, `proc-macro`
22

‎tests/ui/invalid-compile-flags/crate-type-flag.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,16 @@
4545
// `proc-macro` is accepted, but `proc_macro` is not.
4646
//@ revisions: proc_underscore_macro
4747
//@[proc_underscore_macro] compile-flags: --crate-type=proc_macro
48-
//@[proc_underscore_macro] error-pattern: "unknown crate type: `proc_macro`"
48+
//@[proc_underscore_macro] error-pattern: unknown crate type: `proc_macro`
4949

5050
// Empty `--crate-type` not accepted.
5151
//@ revisions: empty_crate_type
5252
//@[empty_crate_type] compile-flags: --crate-type=
53-
//@[empty_crate_type] error-pattern: "unknown crate type: ``"
53+
//@[empty_crate_type] error-pattern: unknown crate type: ``
5454

5555
// Random unknown crate type. Also check that we can handle non-ASCII.
5656
//@ revisions: unknown
5757
//@[unknown] compile-flags: --crate-type=🤡
58-
//@[unknown] error-pattern: "unknown crate type: `🤡`"
58+
//@[unknown] error-pattern: unknown crate type: `🤡`
5959

6060
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
error: unknown crate type: `🤡`
1+
error: unknown crate type: `🤡`, expected one of: `lib`, `rlib`, `staticlib`, `dylib`, `cdylib`, `bin`, `proc-macro`
22

‎tests/ui/lint/linker-warning.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ warning: unused attribute
1616
LL | #![allow(linker_messages)]
1717
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
1818
|
19-
= note: the `linker_warnings` lint can only be controlled at the root of a crate that needs to be linked
19+
= note: the `linker_messages` lint can only be controlled at the root of a crate that needs to be linked
2020

2121
warning: 2 warnings emitted
2222

0 commit comments

Comments
 (0)
Please sign in to comment.