diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 23d3e71424b2b..2770bded30aec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -71,6 +71,11 @@ jobs: uses: actions/checkout@v3 with: fetch-depth: 2 + - name: "checkout the `master` branch for tidy" + uses: actions/checkout@v3 + with: + ref: master + fetch-depth: 1 - name: configure the PR in which the error message will be posted run: "echo \"[CI_PR_NUMBER=$num]\"" env: @@ -485,6 +490,11 @@ jobs: uses: actions/checkout@v3 with: fetch-depth: 2 + - name: "checkout the `master` branch for tidy" + uses: actions/checkout@v3 + with: + ref: master + fetch-depth: 1 - name: configure the PR in which the error message will be posted run: "echo \"[CI_PR_NUMBER=$num]\"" env: @@ -600,6 +610,11 @@ jobs: uses: actions/checkout@v3 with: fetch-depth: 2 + - name: "checkout the `master` branch for tidy" + uses: actions/checkout@v3 + with: + ref: master + fetch-depth: 1 - name: configure the PR in which the error message will be posted run: "echo \"[CI_PR_NUMBER=$num]\"" env: diff --git a/Cargo.lock b/Cargo.lock index 9581099c210ea..7a86a1012a761 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -259,6 +259,10 @@ dependencies = [ "toml", ] +[[package]] +name = "build_helper" +version = "0.1.0" + [[package]] name = "bump-stage0" version = "0.1.0" @@ -2675,9 +2679,9 @@ dependencies = [ [[package]] name = "owo-colors" -version = "3.4.0" +version = "3.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "decf7381921fea4dcb2549c5667eda59b3ec297ab7e2b5fc33eac69d2e7da87b" +checksum = "c1b04fb49957986fdce4d6ee7a65027d55d4b6d2265e5848bbb507b58ccfdb6f" [[package]] name = "packed_simd_2" @@ -5203,9 +5207,9 @@ dependencies = [ [[package]] name = "termcolor" -version = "1.1.2" +version = "1.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2dfed899f0eb03f32ee8c6a0aabdb8a7949659e3466561fc0adf54e26d88c5f4" +checksum = "bab24d30b911b2376f3a13cc2cd443142f0c81dda04c118693e35b3835757755" dependencies = [ "winapi-util", ] @@ -5304,11 +5308,13 @@ dependencies = [ name = "tidy" version = "0.1.0" dependencies = [ + "build_helper", "cargo_metadata 0.14.0", "ignore", "lazy_static", "miropt-test-tools", "regex", + "termcolor", "walkdir", ] diff --git a/Cargo.toml b/Cargo.toml index 1cec4a84480c4..ce08d4edb567d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,7 @@ members = [ "library/std", "library/test", "src/rustdoc-json-types", + "src/tools/build_helper", "src/tools/cargotest", "src/tools/clippy", "src/tools/clippy/clippy_dev", diff --git a/compiler/rustc_target/src/abi/call/m68k.rs b/compiler/rustc_target/src/abi/call/m68k.rs index c1e0f54af5f1e..1d4649ed8678e 100644 --- a/compiler/rustc_target/src/abi/call/m68k.rs +++ b/compiler/rustc_target/src/abi/call/m68k.rs @@ -10,7 +10,7 @@ fn classify_ret(ret: &mut ArgAbi<'_, Ty>) { fn classify_arg(arg: &mut ArgAbi<'_, Ty>) { if arg.layout.is_aggregate() { - arg.make_indirect_byval(); + arg.make_indirect_byval(None); } else { arg.extend_integer_width_to(32); } diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index a5ffaebea0b98..9e799282c4f31 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -500,9 +500,7 @@ impl<'a, Ty> ArgAbi<'a, Ty> { .set(ArgAttribute::NonNull) .set(ArgAttribute::NoUndef); attrs.pointee_size = layout.size; - // FIXME(eddyb) We should be doing this, but at least on - // i686-pc-windows-msvc, it results in wrong stack offsets. - // attrs.pointee_align = Some(layout.align.abi); + attrs.pointee_align = Some(layout.align.abi); let extra_attrs = layout.is_unsized().then_some(ArgAttributes::new()); @@ -519,11 +517,19 @@ impl<'a, Ty> ArgAbi<'a, Ty> { self.mode = Self::indirect_pass_mode(&self.layout); } - pub fn make_indirect_byval(&mut self) { + pub fn make_indirect_byval(&mut self, byval_align: Option) { self.make_indirect(); match self.mode { - PassMode::Indirect { attrs: _, extra_attrs: _, ref mut on_stack } => { + PassMode::Indirect { ref mut attrs, extra_attrs: _, ref mut on_stack } => { *on_stack = true; + + // Some platforms, like 32-bit x86, change the alignment of the type when passing + // `byval`. Account for that. + if let Some(byval_align) = byval_align { + // On all targets with byval align this is currently true, so let's assert it. + debug_assert!(byval_align >= Align::from_bytes(4).unwrap()); + attrs.pointee_align = Some(byval_align); + } } _ => unreachable!(), } @@ -660,7 +666,8 @@ impl<'a, Ty> FnAbi<'a, Ty> { { if abi == spec::abi::Abi::X86Interrupt { if let Some(arg) = self.args.first_mut() { - arg.make_indirect_byval(); + // FIXME(pcwalton): This probably should use the x86 `byval` ABI... + arg.make_indirect_byval(None); } return Ok(()); } diff --git a/compiler/rustc_target/src/abi/call/wasm.rs b/compiler/rustc_target/src/abi/call/wasm.rs index 44427ee5317c1..0eb2309ecb28b 100644 --- a/compiler/rustc_target/src/abi/call/wasm.rs +++ b/compiler/rustc_target/src/abi/call/wasm.rs @@ -36,7 +36,7 @@ where { arg.extend_integer_width_to(32); if arg.layout.is_aggregate() && !unwrap_trivial_aggregate(cx, arg) { - arg.make_indirect_byval(); + arg.make_indirect_byval(None); } } diff --git a/compiler/rustc_target/src/abi/call/x86.rs b/compiler/rustc_target/src/abi/call/x86.rs index 7c26335dcf4cd..58c0717b7d1b3 100644 --- a/compiler/rustc_target/src/abi/call/x86.rs +++ b/compiler/rustc_target/src/abi/call/x86.rs @@ -1,5 +1,5 @@ use crate::abi::call::{ArgAttribute, FnAbi, PassMode, Reg, RegKind}; -use crate::abi::{HasDataLayout, TyAbiInterface}; +use crate::abi::{Align, HasDataLayout, TyAbiInterface}; use crate::spec::HasTargetSpec; #[derive(PartialEq)] @@ -53,11 +53,38 @@ where if arg.is_ignore() { continue; } - if arg.layout.is_aggregate() { - arg.make_indirect_byval(); - } else { + if !arg.layout.is_aggregate() { arg.extend_integer_width_to(32); + continue; } + + // We need to compute the alignment of the `byval` argument. The rules can be found in + // `X86_32ABIInfo::getTypeStackAlignInBytes` in Clang's `TargetInfo.cpp`. Summarized here, + // they are: + // + // 1. If the natural alignment of the type is less than or equal to 4, the alignment is 4. + // + // 2. Otherwise, on Linux, the alignment of any vector type is the natural alignment. + // (This doesn't matter here because we ensure we have an aggregate with the check above.) + // + // 3. Otherwise, on Apple platforms, the alignment of anything that contains a vector type + // is 16. + // + // 4. If none of these conditions are true, the alignment is 4. + let t = cx.target_spec(); + let align_4 = Align::from_bytes(4).unwrap(); + let align_16 = Align::from_bytes(16).unwrap(); + let byval_align = if arg.layout.align.abi < align_4 { + align_4 + } else if t.is_like_osx && arg.layout.align.abi >= align_16 { + // FIXME(pcwalton): This is dubious--we should actually be looking inside the type to + // determine if it contains SIMD vector values--but I think it's fine? + align_16 + } else { + align_4 + }; + + arg.make_indirect_byval(Some(byval_align)); } if flavor == Flavor::FastcallOrVectorcall { diff --git a/compiler/rustc_target/src/abi/call/x86_64.rs b/compiler/rustc_target/src/abi/call/x86_64.rs index c0c071a614f50..54abf23c15054 100644 --- a/compiler/rustc_target/src/abi/call/x86_64.rs +++ b/compiler/rustc_target/src/abi/call/x86_64.rs @@ -213,7 +213,7 @@ where match cls_or_mem { Err(Memory) => { if is_arg { - arg.make_indirect_byval(); + arg.make_indirect_byval(None); } else { // `sret` parameter thus one less integer register available arg.make_indirect(); diff --git a/src/bootstrap/Cargo.lock b/src/bootstrap/Cargo.lock index efe8ae3169fc2..4a0ba5925773e 100644 --- a/src/bootstrap/Cargo.lock +++ b/src/bootstrap/Cargo.lock @@ -36,6 +36,7 @@ dependencies = [ name = "bootstrap" version = "0.0.0" dependencies = [ + "build_helper", "cc", "cmake", "fd-lock", @@ -70,6 +71,10 @@ dependencies = [ "regex-automata", ] +[[package]] +name = "build_helper" +version = "0.1.0" + [[package]] name = "cc" version = "1.0.73" diff --git a/src/bootstrap/Cargo.toml b/src/bootstrap/Cargo.toml index fafe82a9c128a..22ceeca941e93 100644 --- a/src/bootstrap/Cargo.toml +++ b/src/bootstrap/Cargo.toml @@ -30,6 +30,7 @@ path = "bin/sccache-plus-cl.rs" test = false [dependencies] +build_helper = { path = "../tools/build_helper" } cmake = "0.1.38" fd-lock = "3.0.8" filetime = "0.2" diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 707e4169002d9..66bc0f023b6c9 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -97,18 +97,36 @@ impl RunConfig<'_> { self.builder.build.build } - /// Return a `-p=x -p=y` string suitable for passing to a cargo invocation. + /// Return a list of crate names selected by `run.paths`. pub fn cargo_crates_in_set(&self) -> Interned> { let mut crates = Vec::new(); for krate in &self.paths { let path = krate.assert_single_path(); let crate_name = self.builder.crate_paths[&path.path]; - crates.push(format!("-p={crate_name}")); + crates.push(crate_name.to_string()); } INTERNER.intern_list(crates) } } +/// A description of the crates in this set, suitable for passing to `builder.info`. +/// +/// `crates` should be generated by [`RunConfig::cargo_crates_in_set`]. +pub fn crate_description(crates: &[impl AsRef]) -> String { + if crates.is_empty() { + return "".into(); + } + + let mut descr = String::from(" {"); + descr.push_str(crates[0].as_ref()); + for krate in &crates[1..] { + descr.push_str(", "); + descr.push_str(krate.as_ref()); + } + descr.push('}'); + descr +} + struct StepDescription { default: bool, only_hosts: bool, diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index 2e1bd8d6d1f6d..32e5d414061ec 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -99,19 +99,13 @@ impl Step for Std { cargo_subcommand(builder.kind), ); std_cargo(builder, target, compiler.stage, &mut cargo); + cargo.args(args(builder)); builder.info(&format!( - "Checking stage{} std artifacts ({} -> {})", + "Checking stage{} library artifacts ({} -> {})", builder.top_stage, &compiler.host, target )); - run_cargo( - builder, - cargo, - args(builder), - &libstd_stamp(builder, compiler, target), - vec![], - true, - ); + run_cargo(builder, cargo, &libstd_stamp(builder, compiler, target), vec![], true); // We skip populating the sysroot in non-zero stage because that'll lead // to rlib/rmeta conflicts if std gets built during this session. @@ -155,19 +149,13 @@ impl Step for Std { for krate in builder.in_tree_crates("test", Some(target)) { cargo.arg("-p").arg(krate.name); } + cargo.args(args(builder)); builder.info(&format!( - "Checking stage{} std test/bench/example targets ({} -> {})", + "Checking stage{} library test/bench/example targets ({} -> {})", builder.top_stage, &compiler.host, target )); - run_cargo( - builder, - cargo, - args(builder), - &libstd_test_stamp(builder, compiler, target), - vec![], - true, - ); + run_cargo(builder, cargo, &libstd_test_stamp(builder, compiler, target), vec![], true); } } @@ -231,19 +219,13 @@ impl Step for Rustc { for krate in builder.in_tree_crates("rustc-main", Some(target)) { cargo.arg("-p").arg(krate.name); } + cargo.args(args(builder)); builder.info(&format!( "Checking stage{} compiler artifacts ({} -> {})", builder.top_stage, &compiler.host, target )); - run_cargo( - builder, - cargo, - args(builder), - &librustc_stamp(builder, compiler, target), - vec![], - true, - ); + run_cargo(builder, cargo, &librustc_stamp(builder, compiler, target), vec![], true); let libdir = builder.sysroot_libdir(compiler, target); let hostdir = builder.sysroot_libdir(compiler, compiler.host); @@ -290,6 +272,7 @@ impl Step for CodegenBackend { .arg("--manifest-path") .arg(builder.src.join(format!("compiler/rustc_codegen_{}/Cargo.toml", backend))); rustc_cargo_env(builder, &mut cargo, target); + cargo.args(args(builder)); builder.info(&format!( "Checking stage{} {} artifacts ({} -> {})", @@ -299,7 +282,6 @@ impl Step for CodegenBackend { run_cargo( builder, cargo, - args(builder), &codegen_backend_stamp(builder, compiler, target, backend), vec![], true, @@ -355,11 +337,13 @@ impl Step for RustAnalyzer { cargo.arg("--benches"); } + cargo.args(args(builder)); + builder.info(&format!( "Checking stage{} {} artifacts ({} -> {})", compiler.stage, "rust-analyzer", &compiler.host.triple, target.triple )); - run_cargo(builder, cargo, args(builder), &stamp(builder, compiler, target), vec![], true); + run_cargo(builder, cargo, &stamp(builder, compiler, target), vec![], true); /// Cargo's output path in a given stage, compiled by a particular /// compiler for the specified target. @@ -413,6 +397,8 @@ macro_rules! tool_check_step { cargo.arg("--all-targets"); } + cargo.args(args(builder)); + // Enable internal lints for clippy and rustdoc // NOTE: this doesn't enable lints for any other tools unless they explicitly add `#![warn(rustc::internal)]` // See https://github.com/rust-lang/rust/pull/80573#issuecomment-754010776 @@ -428,7 +414,6 @@ macro_rules! tool_check_step { run_cargo( builder, cargo, - args(builder), &stamp(builder, compiler, target), vec![], true, diff --git a/src/bootstrap/clean.rs b/src/bootstrap/clean.rs index 8e363ee1290ee..468efc1114c43 100644 --- a/src/bootstrap/clean.rs +++ b/src/bootstrap/clean.rs @@ -9,11 +9,10 @@ use std::fs; use std::io::{self, ErrorKind}; use std::path::Path; -use crate::builder::{Builder, RunConfig, ShouldRun, Step}; +use crate::builder::{crate_description, Builder, RunConfig, ShouldRun, Step}; use crate::cache::Interned; -use crate::config::TargetSelection; use crate::util::t; -use crate::{Build, Mode, Subcommand}; +use crate::{Build, Compiler, Mode, Subcommand}; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct CleanAll {} @@ -40,7 +39,7 @@ macro_rules! clean_crate_tree { ( $( $name:ident, $mode:path, $root_crate:literal);+ $(;)? ) => { $( #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct $name { - target: TargetSelection, + compiler: Compiler, crates: Interned>, } @@ -54,22 +53,21 @@ macro_rules! clean_crate_tree { fn make_run(run: RunConfig<'_>) { let builder = run.builder; - if builder.top_stage != 0 { - panic!("non-stage-0 clean not supported for individual crates"); - } - builder.ensure(Self { crates: run.cargo_crates_in_set(), target: run.target }); + let compiler = builder.compiler(builder.top_stage, run.target); + builder.ensure(Self { crates: run.cargo_crates_in_set(), compiler }); } fn run(self, builder: &Builder<'_>) -> Self::Output { - let compiler = builder.compiler(0, self.target); - let mut cargo = builder.bare_cargo(compiler, $mode, self.target, "clean"); + let compiler = self.compiler; + let target = compiler.host; + let mut cargo = builder.bare_cargo(compiler, $mode, target, "clean"); for krate in &*self.crates { cargo.arg(krate); } builder.info(&format!( - "Cleaning stage{} {} artifacts ({} -> {})", - compiler.stage, stringify!($name).to_lowercase(), &compiler.host, self.target + "Cleaning{} stage{} {} artifacts ({} -> {})", + crate_description(&self.crates), compiler.stage, stringify!($name).to_lowercase(), &compiler.host, target, )); // NOTE: doesn't use `run_cargo` because we don't want to save a stamp file, diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index 1030247b890c3..f9a04f2e91dbf 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -18,6 +18,7 @@ use std::str; use serde::Deserialize; +use crate::builder::crate_description; use crate::builder::Cargo; use crate::builder::{Builder, Kind, RunConfig, ShouldRun, Step}; use crate::cache::{Interned, INTERNER}; @@ -110,7 +111,10 @@ impl Step for Std { let compiler_to_use = builder.compiler_for(compiler.stage, compiler.host, target); if compiler_to_use != compiler { builder.ensure(Std::new(compiler_to_use, target)); - builder.info(&format!("Uplifting stage1 std ({} -> {})", compiler_to_use.host, target)); + builder.info(&format!( + "Uplifting stage1 library ({} -> {})", + compiler_to_use.host, target + )); // Even if we're not building std this stage, the new sysroot must // still contain the third party objects needed by various targets. @@ -126,19 +130,18 @@ impl Step for Std { let mut cargo = builder.cargo(compiler, Mode::Std, SourceType::InTree, target, "build"); std_cargo(builder, target, compiler.stage, &mut cargo); + for krate in &*self.crates { + cargo.arg("-p").arg(krate); + } builder.info(&format!( - "Building stage{} std artifacts ({} -> {})", - compiler.stage, &compiler.host, target + "Building{} stage{} library artifacts ({} -> {})", + crate_description(&self.crates), + compiler.stage, + &compiler.host, + target, )); - run_cargo( - builder, - cargo, - self.crates.to_vec(), - &libstd_stamp(builder, compiler, target), - target_deps, - false, - ); + run_cargo(builder, cargo, &libstd_stamp(builder, compiler, target), target_deps, false); builder.ensure(StdLink::from_std( self, @@ -425,7 +428,7 @@ impl Step for StdLink { let target_compiler = self.target_compiler; let target = self.target; builder.info(&format!( - "Copying stage{} std from stage{} ({} -> {} / {})", + "Copying stage{} library from stage{} ({} -> {} / {})", target_compiler.stage, compiler.stage, &compiler.host, target_compiler.host, target )); let libdir = builder.sysroot_libdir(target_compiler, target); @@ -714,18 +717,18 @@ impl Step for Rustc { } } + for krate in &*self.crates { + cargo.arg("-p").arg(krate); + } + builder.info(&format!( - "Building stage{} compiler artifacts ({} -> {})", - compiler.stage, &compiler.host, target + "Building{} stage{} compiler artifacts ({} -> {})", + crate_description(&self.crates), + compiler.stage, + &compiler.host, + target, )); - run_cargo( - builder, - cargo, - self.crates.to_vec(), - &librustc_stamp(builder, compiler, target), - vec![], - false, - ); + run_cargo(builder, cargo, &librustc_stamp(builder, compiler, target), vec![], false); builder.ensure(RustcLink::from_rustc( self, @@ -981,7 +984,7 @@ impl Step for CodegenBackend { "Building stage{} codegen backend {} ({} -> {})", compiler.stage, backend, &compiler.host, target )); - let files = run_cargo(builder, cargo, vec![], &tmp_stamp, vec![], false); + let files = run_cargo(builder, cargo, &tmp_stamp, vec![], false); if builder.config.dry_run() { return; } @@ -1405,7 +1408,6 @@ pub fn add_to_sysroot( pub fn run_cargo( builder: &Builder<'_>, cargo: Cargo, - tail_args: Vec, stamp: &Path, additional_target_deps: Vec<(PathBuf, DependencyType)>, is_check: bool, @@ -1431,7 +1433,7 @@ pub fn run_cargo( // files we need to probe for later. let mut deps = Vec::new(); let mut toplevel = Vec::new(); - let ok = stream_cargo(builder, cargo, tail_args, &mut |msg| { + let ok = stream_cargo(builder, cargo, &mut |msg| { let (filenames, crate_types) = match msg { CargoMessage::CompilerArtifact { filenames, @@ -1546,7 +1548,6 @@ pub fn run_cargo( pub fn stream_cargo( builder: &Builder<'_>, cargo: Cargo, - tail_args: Vec, cb: &mut dyn FnMut(CargoMessage<'_>), ) -> bool { let mut cargo = Command::from(cargo); @@ -1566,10 +1567,6 @@ pub fn stream_cargo( } cargo.arg("--message-format").arg(message_format).stdout(Stdio::piped()); - for arg in tail_args { - cargo.arg(arg); - } - builder.verbose(&format!("running: {:?}", cargo)); let mut child = match cargo.spawn() { Ok(child) => child, diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index 3f43c68d2e082..9bad9046ecc2c 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -12,6 +12,7 @@ use std::fs; use std::io; use std::path::{Path, PathBuf}; +use crate::builder::crate_description; use crate::builder::{Builder, Compiler, Kind, RunConfig, ShouldRun, Step}; use crate::cache::{Interned, INTERNER}; use crate::compile; @@ -558,7 +559,8 @@ fn doc_std( requested_crates: &[String], ) { builder.info(&format!( - "Documenting stage{} std ({}) in {} format", + "Documenting{} stage{} library ({}) in {} format", + crate_description(requested_crates), stage, target, format.as_str() diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 3be7ad4075ed8..2c6d201d18fbe 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -352,32 +352,32 @@ To learn more about a subcommand, run `./x.py -h`", // fn usage() let usage = |exit_code: i32, opts: &Options, verbose: bool, subcommand_help: &str| -> ! { - // We have an unfortunate situation here: some Steps use `builder.in_tree_crates` to determine their paths. - // To determine those crates, we need to run `cargo metadata`, which means we need all submodules to be checked out. - // That takes a while to run, so only do it when paths were explicitly requested, not on all CLI errors. - // `Build::new` won't load submodules for the `setup` command. - let cmd = if verbose { - println!("note: updating submodules before printing available paths"); - "build" - } else { - "setup" - }; - let config = Config::parse(&[cmd.to_string()]); - let build = Build::new(config); - let paths = Builder::get_help(&build, subcommand); - println!("{}", opts.usage(subcommand_help)); - if let Some(s) = paths { - if verbose { + if verbose { + // We have an unfortunate situation here: some Steps use `builder.in_tree_crates` to determine their paths. + // To determine those crates, we need to run `cargo metadata`, which means we need all submodules to be checked out. + // That takes a while to run, so only do it when paths were explicitly requested, not on all CLI errors. + // `Build::new` won't load submodules for the `setup` command. + let cmd = if verbose { + println!("note: updating submodules before printing available paths"); + "build" + } else { + "setup" + }; + let config = Config::parse(&[cmd.to_string()]); + let build = Build::new(config); + let paths = Builder::get_help(&build, subcommand); + + if let Some(s) = paths { println!("{}", s); } else { - println!( - "Run `./x.py {} -h -v` to see a list of available paths.", - subcommand.as_str() - ); + panic!("No paths available for subcommand `{}`", subcommand.as_str()); } - } else if verbose { - panic!("No paths available for subcommand `{}`", subcommand.as_str()); + } else { + println!( + "Run `./x.py {} -h -v` to see a list of available paths.", + subcommand.as_str() + ); } crate::detail_exit(exit_code); }; diff --git a/src/bootstrap/format.rs b/src/bootstrap/format.rs index 1d57c6ecbbb8d..cca45f4a634e8 100644 --- a/src/bootstrap/format.rs +++ b/src/bootstrap/format.rs @@ -2,6 +2,7 @@ use crate::builder::Builder; use crate::util::{output, program_out_of_date, t}; +use build_helper::git::get_rust_lang_rust_remote; use ignore::WalkBuilder; use std::collections::VecDeque; use std::path::{Path, PathBuf}; @@ -79,56 +80,22 @@ fn update_rustfmt_version(build: &Builder<'_>) { /// /// Returns `None` if all files should be formatted. fn get_modified_rs_files(build: &Builder<'_>) -> Option> { - let Ok(remote) = get_rust_lang_rust_remote() else {return None;}; + let Ok(remote) = get_rust_lang_rust_remote() else { return None; }; if !verify_rustfmt_version(build) { return None; } + + let merge_base = + output(build.config.git().arg("merge-base").arg(&format!("{remote}/master")).arg("HEAD")); Some( - output( - build - .config - .git() - .arg("diff-index") - .arg("--name-only") - .arg("--merge-base") - .arg(&format!("{remote}/master")), - ) - .lines() - .map(|s| s.trim().to_owned()) - .filter(|f| Path::new(f).extension().map_or(false, |ext| ext == "rs")) - .collect(), + output(build.config.git().arg("diff-index").arg("--name-only").arg(merge_base.trim())) + .lines() + .map(|s| s.trim().to_owned()) + .filter(|f| Path::new(f).extension().map_or(false, |ext| ext == "rs")) + .collect(), ) } -/// Finds the remote for rust-lang/rust. -/// For example for these remotes it will return `upstream`. -/// ```text -/// origin https://github.com/Nilstrieb/rust.git (fetch) -/// origin https://github.com/Nilstrieb/rust.git (push) -/// upstream https://github.com/rust-lang/rust (fetch) -/// upstream https://github.com/rust-lang/rust (push) -/// ``` -fn get_rust_lang_rust_remote() -> Result { - let mut git = Command::new("git"); - git.args(["config", "--local", "--get-regex", "remote\\..*\\.url"]); - - let output = git.output().map_err(|err| format!("{err:?}"))?; - if !output.status.success() { - return Err("failed to execute git config command".to_owned()); - } - - let stdout = String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))?; - - let rust_lang_remote = stdout - .lines() - .find(|remote| remote.contains("rust-lang")) - .ok_or_else(|| "rust-lang/rust remote not found".to_owned())?; - - let remote_name = - rust_lang_remote.split('.').nth(1).ok_or_else(|| "remote name not found".to_owned())?; - Ok(remote_name.into()) -} - #[derive(serde::Deserialize)] struct RustfmtConfig { ignore: Vec, diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 5ea41d10bc817..d44b96cfb991e 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -113,6 +113,7 @@ use std::path::{Path, PathBuf}; use std::process::Command; use std::str; +use build_helper::ci::CiEnv; use channel::GitInfo; use config::{DryRun, Target}; use filetime::FileTime; @@ -121,7 +122,7 @@ use once_cell::sync::OnceCell; use crate::builder::Kind; use crate::config::{LlvmLibunwind, TargetSelection}; use crate::util::{ - exe, libdir, mtime, output, run, run_suppressed, symlink_dir, try_run_suppressed, CiEnv, + exe, libdir, mtime, output, run, run_suppressed, symlink_dir, try_run_suppressed, }; mod bolt; diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs index 4e503dfe864e2..bdc81b07b8d54 100644 --- a/src/bootstrap/native.rs +++ b/src/bootstrap/native.rs @@ -24,6 +24,8 @@ use crate::util::get_clang_cl_resource_dir; use crate::util::{self, exe, output, t, up_to_date}; use crate::{CLang, GitRepo}; +use build_helper::ci::CiEnv; + #[derive(Clone)] pub struct LlvmResult { /// Path to llvm-config binary. @@ -217,7 +219,7 @@ pub(crate) fn is_ci_llvm_available(config: &Config, asserts: bool) -> bool { return false; } - if crate::util::CiEnv::is_ci() { + if CiEnv::is_ci() { // We assume we have access to git, so it's okay to unconditionally pass // `true` here. let llvm_sha = detect_llvm_sha(config, true); diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index c8b4134391e5f..d5bec268a4567 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -11,6 +11,7 @@ use std::iter; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; +use crate::builder::crate_description; use crate::builder::{Builder, Compiler, Kind, RunConfig, ShouldRun, Step}; use crate::cache::Interned; use crate::compile; @@ -2154,8 +2155,12 @@ impl Step for Crate { } builder.info(&format!( - "{} {:?} stage{} ({} -> {})", - test_kind, self.crates, compiler.stage, &compiler.host, target + "{}{} stage{} ({} -> {})", + test_kind, + crate_description(&self.crates), + compiler.stage, + &compiler.host, + target )); let _time = util::timeit(&builder); try_run(builder, &mut cargo.into()); diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index 24b033cc0dc5e..63026bd44d475 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -72,7 +72,7 @@ impl Step for ToolBuild { builder.info(&format!("Building stage{} tool {} ({})", compiler.stage, tool, target)); let mut duplicates = Vec::new(); - let is_expected = compile::stream_cargo(builder, cargo, vec![], &mut |msg| { + let is_expected = compile::stream_cargo(builder, cargo, &mut |msg| { // Only care about big things like the RLS/Cargo for now match tool { "rls" | "cargo" | "clippy-driver" | "miri" | "rustfmt" => {} diff --git a/src/bootstrap/util.rs b/src/bootstrap/util.rs index 58220783228b2..8ce9a9ce38cc5 100644 --- a/src/bootstrap/util.rs +++ b/src/bootstrap/util.rs @@ -255,35 +255,6 @@ pub enum CiEnv { GitHubActions, } -impl CiEnv { - /// Obtains the current CI environment. - pub fn current() -> CiEnv { - if env::var("TF_BUILD").map_or(false, |e| e == "True") { - CiEnv::AzurePipelines - } else if env::var("GITHUB_ACTIONS").map_or(false, |e| e == "true") { - CiEnv::GitHubActions - } else { - CiEnv::None - } - } - - pub fn is_ci() -> bool { - Self::current() != CiEnv::None - } - - /// If in a CI environment, forces the command to run with colors. - pub fn force_coloring_in_ci(self, cmd: &mut Command) { - if self != CiEnv::None { - // Due to use of stamp/docker, the output stream of rustbuild is not - // a TTY in CI, so coloring is by-default turned off. - // The explicit `TERM=xterm` environment is needed for - // `--color always` to actually work. This env var was lost when - // compiling through the Makefile. Very strange. - cmd.env("TERM", "xterm").args(&["--color", "always"]); - } - } -} - pub fn forcing_clang_based_tests() -> bool { if let Some(var) = env::var_os("RUSTBUILD_FORCE_CLANG_BASED_TESTS") { match &var.to_string_lossy().to_lowercase()[..] { diff --git a/src/ci/github-actions/ci.yml b/src/ci/github-actions/ci.yml index d33396dcc80fe..d3b07419486b2 100644 --- a/src/ci/github-actions/ci.yml +++ b/src/ci/github-actions/ci.yml @@ -103,6 +103,12 @@ x--expand-yaml-anchors--remove: with: fetch-depth: 2 + - name: checkout the `master` branch for tidy + uses: actions/checkout@v3 + with: + ref: master + fetch-depth: 1 + # Rust Log Analyzer can't currently detect the PR number of a GitHub # Actions build on its own, so a hint in the log message is needed to # point it in the right direction. diff --git a/src/test/codegen/align-byval.rs b/src/test/codegen/align-byval.rs new file mode 100644 index 0000000000000..35b3b1adc2cdc --- /dev/null +++ b/src/test/codegen/align-byval.rs @@ -0,0 +1,56 @@ +// ignore-x86 +// ignore-aarch64 +// ignore-aarch64_be +// ignore-arm +// ignore-armeb +// ignore-avr +// ignore-bpfel +// ignore-bpfeb +// ignore-hexagon +// ignore-mips +// ignore-mips64 +// ignore-msp430 +// ignore-powerpc64 +// ignore-powerpc64le +// ignore-powerpc +// ignore-r600 +// ignore-amdgcn +// ignore-sparc +// ignore-sparcv9 +// ignore-sparcel +// ignore-s390x +// ignore-tce +// ignore-thumb +// ignore-thumbeb +// ignore-xcore +// ignore-nvptx +// ignore-nvptx64 +// ignore-le32 +// ignore-le64 +// ignore-amdil +// ignore-amdil64 +// ignore-hsail +// ignore-hsail64 +// ignore-spir +// ignore-spir64 +// ignore-kalimba +// ignore-shave +// +// Tests that `byval` alignment is properly specified (#80127). +// The only targets that use `byval` are m68k, wasm, x86-64, and x86. Note that +// x86 has special rules (see #103830), and it's therefore ignored here. + +#[repr(C)] +#[repr(align(16))] +struct Foo { + a: [i32; 16], +} + +extern "C" { + // CHECK: declare void @f({{.*}}byval(%Foo) align 16{{.*}}) + fn f(foo: Foo); +} + +pub fn main() { + unsafe { f(Foo { a: [1; 16] }) } +} diff --git a/src/test/codegen/function-arguments-noopt.rs b/src/test/codegen/function-arguments-noopt.rs index ff76405a4ea32..5b41ab7f8dc0e 100644 --- a/src/test/codegen/function-arguments-noopt.rs +++ b/src/test/codegen/function-arguments-noopt.rs @@ -36,7 +36,7 @@ pub fn borrow_call(x: &i32, f: fn(&i32) -> &i32) -> &i32 { f(x) } -// CHECK: void @struct_({{%S\*|ptr}} sret(%S){{( %0)?}}, {{%S\*|ptr}} %x) +// CHECK: void @struct_({{%S\*|ptr}} sret(%S) align 4{{( %0)?}}, {{%S\*|ptr}} align 4 %x) #[no_mangle] pub fn struct_(x: S) -> S { x @@ -45,7 +45,7 @@ pub fn struct_(x: S) -> S { // CHECK-LABEL: @struct_call #[no_mangle] pub fn struct_call(x: S, f: fn(S) -> S) -> S { - // CHECK: call void %f({{%S\*|ptr}} sret(%S){{( %0)?}}, {{%S\*|ptr}} %{{.+}}) + // CHECK: call void %f({{%S\*|ptr}} sret(%S) align 4{{( %0)?}}, {{%S\*|ptr}} align 4 %{{.+}}) f(x) } diff --git a/src/test/codegen/function-arguments.rs b/src/test/codegen/function-arguments.rs index 44fee952307f2..fd413bcf1d57f 100644 --- a/src/test/codegen/function-arguments.rs +++ b/src/test/codegen/function-arguments.rs @@ -127,7 +127,7 @@ pub fn mutable_notunpin_borrow(_: &mut NotUnpin) { pub fn notunpin_borrow(_: &NotUnpin) { } -// CHECK: @indirect_struct({{%S\*|ptr}} noalias nocapture noundef readonly dereferenceable(32) %_1) +// CHECK: @indirect_struct({{%S\*|ptr}} noalias nocapture noundef readonly align 4 dereferenceable(32) %_1) #[no_mangle] pub fn indirect_struct(_: S) { } @@ -151,7 +151,7 @@ pub fn _box(x: Box) -> Box { x } -// CHECK: @struct_return({{%S\*|ptr}} noalias nocapture noundef sret(%S) dereferenceable(32){{( %0)?}}) +// CHECK: @struct_return({{%S\*|ptr}} noalias nocapture noundef sret(%S) align 4 dereferenceable(32){{( %0)?}}) #[no_mangle] pub fn struct_return() -> S { S { diff --git a/src/test/codegen/issue-86106.rs b/src/test/codegen/issue-86106.rs new file mode 100644 index 0000000000000..e647dbe2514db --- /dev/null +++ b/src/test/codegen/issue-86106.rs @@ -0,0 +1,55 @@ +// min-llvm-version: 15.0 +// compile-flags: -C opt-level=3 + +// The below two functions ensure that both `String::new()` and `"".to_string()` +// produce the identical code. + +#![crate_type = "lib"] + +// CHECK-LABEL: @string_new = unnamed_addr alias void (ptr), ptr @empty_to_string +#[no_mangle] +pub fn string_new() -> String { + String::new() +} + +// CHECK-LABEL: define void @empty_to_string +#[no_mangle] +pub fn empty_to_string() -> String { + // CHECK-NOT: load i8 + // CHECK: store i{{32|64}} + // CHECK-NEXT: getelementptr + // CHECK-NEXT: store ptr + // CHECK-NEXT: getelementptr + // CHECK-NEXT: store i{{32|64}} + // CHECK-NEXT: ret void + "".to_string() +} + +// The below two functions ensure that both `vec![]` and `vec![].clone()` +// produce the identical code. + +// CHECK-LABEL: @empty_vec +#[no_mangle] +pub fn empty_vec() -> Vec { + // CHECK: store i{{32|64}} + // CHECK-NOT: load i8 + // CHECK-NEXT: getelementptr + // CHECK-NEXT: store ptr + // CHECK-NEXT: getelementptr + // CHECK-NEXT: store i{{32|64}} + // CHECK-NEXT: ret void + vec![] +} + +// CHECK-LABEL: @empty_vec_clone +#[no_mangle] +pub fn empty_vec_clone() -> Vec { + // CHECK: store i{{32|64}} + // CHECK-NOT: load i8 + // CHECK-NEXT: getelementptr + // CHECK-NEXT: store ptr + // CHECK-NEXT: getelementptr + // CHECK-NEXT: store i{{32|64}} + // CHECK-NEXT: ret void + vec![].clone() +} diff --git a/src/test/run-make-fulldeps/extern-fn-explicit-align/Makefile b/src/test/run-make-fulldeps/extern-fn-explicit-align/Makefile new file mode 100644 index 0000000000000..4f5d026f213f2 --- /dev/null +++ b/src/test/run-make-fulldeps/extern-fn-explicit-align/Makefile @@ -0,0 +1,5 @@ +include ../tools.mk + +all: $(call NATIVE_STATICLIB,test) + $(RUSTC) test.rs + $(call RUN,test) || exit 1 diff --git a/src/test/run-make-fulldeps/extern-fn-explicit-align/test.c b/src/test/run-make-fulldeps/extern-fn-explicit-align/test.c new file mode 100644 index 0000000000000..a015fc9aaf606 --- /dev/null +++ b/src/test/run-make-fulldeps/extern-fn-explicit-align/test.c @@ -0,0 +1,35 @@ +#include +#include +#include +#include + +struct TwoU64s +{ + uint64_t a; + uint64_t b; +} __attribute__((aligned(16))); + +struct BoolAndU32 +{ + bool a; + uint32_t b; +}; + +int32_t many_args( + void *a, + void *b, + const char *c, + uint64_t d, + bool e, + struct BoolAndU32 f, + void *g, + struct TwoU64s h, + void *i, + void *j, + void *k, + void *l, + const char *m) +{ + assert(strcmp(m, "Hello world") == 0); + return 0; +} diff --git a/src/test/run-make-fulldeps/extern-fn-explicit-align/test.rs b/src/test/run-make-fulldeps/extern-fn-explicit-align/test.rs new file mode 100644 index 0000000000000..ba6cc87bd1857 --- /dev/null +++ b/src/test/run-make-fulldeps/extern-fn-explicit-align/test.rs @@ -0,0 +1,61 @@ +// Issue #80127: Passing structs via FFI should work with explicit alignment. + +use std::ffi::CString; +use std::ptr::null_mut; + +#[derive(Clone, Copy, Debug, PartialEq)] +#[repr(C)] +#[repr(align(16))] +pub struct TwoU64s { + pub a: u64, + pub b: u64, +} + +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct BoolAndU32 { + pub a: bool, + pub b: u32, +} + +#[link(name = "test", kind = "static")] +extern "C" { + fn many_args( + a: *mut (), + b: *mut (), + c: *const i8, + d: u64, + e: bool, + f: BoolAndU32, + g: *mut (), + h: TwoU64s, + i: *mut (), + j: *mut (), + k: *mut (), + l: *mut (), + m: *const i8, + ) -> i32; +} + +fn main() { + let two_u64s = TwoU64s { a: 1, b: 2 }; + let bool_and_u32 = BoolAndU32 { a: true, b: 3 }; + let string = CString::new("Hello world").unwrap(); + unsafe { + many_args( + null_mut(), + null_mut(), + null_mut(), + 4, + true, + bool_and_u32, + null_mut(), + two_u64s, + null_mut(), + null_mut(), + null_mut(), + null_mut(), + string.as_ptr(), + ); + } +} diff --git a/src/test/rustdoc-gui/scrape-examples-layout.goml b/src/test/rustdoc-gui/scrape-examples-layout.goml index fde9a0ab0bc30..95102528ec11d 100644 --- a/src/test/rustdoc-gui/scrape-examples-layout.goml +++ b/src/test/rustdoc-gui/scrape-examples-layout.goml @@ -33,3 +33,17 @@ assert-property: ( ".more-scraped-examples .scraped-example:nth-child(6) .code-wrapper .src-line-numbers", {"clientWidth": |clientWidth|} ) + +// Check that for both mobile and desktop sizes, the buttons in scraped examples are displayed +// correctly. + +store-value: (offset_y, 4) + +// First with desktop +assert-position: (".scraped-example .code-wrapper", {"y": 255}) +assert-position: (".scraped-example .code-wrapper .prev", {"y": 255 + |offset_y|}) + +// Then with mobile +size: (600, 600) +assert-position: (".scraped-example .code-wrapper", {"y": 314}) +assert-position: (".scraped-example .code-wrapper .prev", {"y": 314 + |offset_y|}) diff --git a/src/tools/build_helper/Cargo.toml b/src/tools/build_helper/Cargo.toml new file mode 100644 index 0000000000000..99f6fea2ecf99 --- /dev/null +++ b/src/tools/build_helper/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "build_helper" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/src/tools/build_helper/src/ci.rs b/src/tools/build_helper/src/ci.rs new file mode 100644 index 0000000000000..9f113c72b9338 --- /dev/null +++ b/src/tools/build_helper/src/ci.rs @@ -0,0 +1,40 @@ +use std::process::Command; + +#[derive(Copy, Clone, PartialEq, Eq, Debug)] +pub enum CiEnv { + /// Not a CI environment. + None, + /// The Azure Pipelines environment, for Linux (including Docker), Windows, and macOS builds. + AzurePipelines, + /// The GitHub Actions environment, for Linux (including Docker), Windows and macOS builds. + GitHubActions, +} + +impl CiEnv { + /// Obtains the current CI environment. + pub fn current() -> CiEnv { + if std::env::var("TF_BUILD").map_or(false, |e| e == "True") { + CiEnv::AzurePipelines + } else if std::env::var("GITHUB_ACTIONS").map_or(false, |e| e == "true") { + CiEnv::GitHubActions + } else { + CiEnv::None + } + } + + pub fn is_ci() -> bool { + Self::current() != CiEnv::None + } + + /// If in a CI environment, forces the command to run with colors. + pub fn force_coloring_in_ci(self, cmd: &mut Command) { + if self != CiEnv::None { + // Due to use of stamp/docker, the output stream of rustbuild is not + // a TTY in CI, so coloring is by-default turned off. + // The explicit `TERM=xterm` environment is needed for + // `--color always` to actually work. This env var was lost when + // compiling through the Makefile. Very strange. + cmd.env("TERM", "xterm").args(&["--color", "always"]); + } + } +} diff --git a/src/tools/build_helper/src/git.rs b/src/tools/build_helper/src/git.rs new file mode 100644 index 0000000000000..e9638206e6729 --- /dev/null +++ b/src/tools/build_helper/src/git.rs @@ -0,0 +1,30 @@ +use std::process::Command; + +/// Finds the remote for rust-lang/rust. +/// For example for these remotes it will return `upstream`. +/// ```text +/// origin https://github.com/Nilstrieb/rust.git (fetch) +/// origin https://github.com/Nilstrieb/rust.git (push) +/// upstream https://github.com/rust-lang/rust (fetch) +/// upstream https://github.com/rust-lang/rust (push) +/// ``` +pub fn get_rust_lang_rust_remote() -> Result { + let mut git = Command::new("git"); + git.args(["config", "--local", "--get-regex", "remote\\..*\\.url"]); + + let output = git.output().map_err(|err| format!("{err:?}"))?; + if !output.status.success() { + return Err("failed to execute git config command".to_owned()); + } + + let stdout = String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))?; + + let rust_lang_remote = stdout + .lines() + .find(|remote| remote.contains("rust-lang")) + .ok_or_else(|| "rust-lang/rust remote not found".to_owned())?; + + let remote_name = + rust_lang_remote.split('.').nth(1).ok_or_else(|| "remote name not found".to_owned())?; + Ok(remote_name.into()) +} diff --git a/src/tools/build_helper/src/lib.rs b/src/tools/build_helper/src/lib.rs new file mode 100644 index 0000000000000..d3d2323db853a --- /dev/null +++ b/src/tools/build_helper/src/lib.rs @@ -0,0 +1,2 @@ +pub mod ci; +pub mod git; diff --git a/src/tools/tidy/Cargo.toml b/src/tools/tidy/Cargo.toml index 97d038da702d5..bf291dd681475 100644 --- a/src/tools/tidy/Cargo.toml +++ b/src/tools/tidy/Cargo.toml @@ -5,12 +5,14 @@ edition = "2021" autobins = false [dependencies] +build_helper = { path = "../build_helper" } cargo_metadata = "0.14" regex = "1" miropt-test-tools = { path = "../miropt-test-tools" } lazy_static = "1" walkdir = "2" ignore = "0.4.18" +termcolor = "1.1.3" [[bin]] name = "rust-tidy" diff --git a/src/tools/tidy/src/error_codes_check.rs b/src/tools/tidy/src/error_codes_check.rs index fd870b0997ce0..40a46c630d70a 100644 --- a/src/tools/tidy/src/error_codes_check.rs +++ b/src/tools/tidy/src/error_codes_check.rs @@ -80,15 +80,6 @@ fn check_if_error_code_is_test_in_explanation(f: &str, err_code: &str) -> bool { ignore_found } -macro_rules! some_or_continue { - ($e:expr) => { - match $e { - Some(e) => e, - None => continue, - } - }; -} - fn extract_error_codes( f: &str, error_codes: &mut HashMap, @@ -122,10 +113,16 @@ fn extract_error_codes( Some((file_name, _)) => file_name, }, }; - let path = some_or_continue!(path.parent()) + + let Some(parent) = path.parent() else { + continue; + }; + + let path = parent .join(md_file_name) .canonicalize() .expect("failed to canonicalize error explanation file path"); + match read_to_string(&path) { Ok(content) => { let has_test = check_if_error_code_is_test_in_explanation(&content, &err_code); @@ -215,8 +212,6 @@ pub fn check(paths: &[&Path], bad: &mut bool) { // * #[error = "E0111"] let regex = Regex::new(r#"[(,"\s](E\d{4})[,)"]"#).unwrap(); - println!("Checking which error codes lack tests..."); - for path in paths { walk(path, &mut filter_dirs, &mut |entry, contents| { let file_name = entry.file_name(); @@ -245,20 +240,15 @@ pub fn check(paths: &[&Path], bad: &mut bool) { }); } if found_explanations == 0 { - eprintln!("No error code explanation was tested!"); - *bad = true; + tidy_error!(bad, "No error code explanation was tested!"); } if found_tests == 0 { - eprintln!("No error code was found in compilation errors!"); - *bad = true; + tidy_error!(bad, "No error code was found in compilation errors!"); } if explanations.is_empty() { - eprintln!("No error code explanation was found!"); - *bad = true; + tidy_error!(bad, "No error code explanation was found!"); } if errors.is_empty() { - println!("Found {} error codes", error_codes.len()); - for (err_code, error_status) in &error_codes { if !error_status.has_test && !EXEMPTED_FROM_TEST.contains(&err_code.as_str()) { errors.push(format!("Error code {err_code} needs to have at least one UI test!")); @@ -310,11 +300,6 @@ pub fn check(paths: &[&Path], bad: &mut bool) { } errors.sort(); for err in &errors { - eprintln!("{err}"); - } - println!("Found {} error(s) in error codes", errors.len()); - if !errors.is_empty() { - *bad = true; + tidy_error!(bad, "{err}"); } - println!("Done!"); } diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index 698e4850bea9b..54f11c48b545e 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -3,6 +3,10 @@ //! This library contains the tidy lints and exposes it //! to be used by tools. +use std::fmt::Display; + +use termcolor::WriteColor; + /// A helper macro to `unwrap` a result except also print out details like: /// /// * The expression that failed @@ -26,18 +30,27 @@ macro_rules! t { } macro_rules! tidy_error { - ($bad:expr, $fmt:expr) => ({ - *$bad = true; - eprint!("tidy error: "); - eprintln!($fmt); - }); - ($bad:expr, $fmt:expr, $($arg:tt)*) => ({ - *$bad = true; - eprint!("tidy error: "); - eprintln!($fmt, $($arg)*); + ($bad:expr, $($fmt:tt)*) => ({ + $crate::tidy_error($bad, format_args!($($fmt)*)).expect("failed to output error"); }); } +fn tidy_error(bad: &mut bool, args: impl Display) -> std::io::Result<()> { + use std::io::Write; + use termcolor::{Color, ColorChoice, ColorSpec, StandardStream}; + + *bad = true; + + let mut stderr = StandardStream::stdout(ColorChoice::Auto); + stderr.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?; + + write!(&mut stderr, "tidy error")?; + stderr.set_color(&ColorSpec::new())?; + + writeln!(&mut stderr, ": {args}")?; + Ok(()) +} + pub mod alphabetical; pub mod bins; pub mod debug_artifacts; @@ -48,6 +61,7 @@ pub mod errors; pub mod extdeps; pub mod features; pub mod mir_opt_tests; +pub mod no_merge; pub mod pal; pub mod primitive_docs; pub mod style; diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 6714c63ee62a1..65f0fe9743f80 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -107,6 +107,8 @@ fn main() { check!(alphabetical, &compiler_path); check!(alphabetical, &library_path); + check!(no_merge, ()); + let collected = { drain_handles(&mut handles); diff --git a/src/tools/tidy/src/no_merge.rs b/src/tools/tidy/src/no_merge.rs new file mode 100644 index 0000000000000..362d6f6f4d77f --- /dev/null +++ b/src/tools/tidy/src/no_merge.rs @@ -0,0 +1,72 @@ +//! This check makes sure that no accidental merge commits are introduced to the repository. +//! It forbids all merge commits that are not caused by rollups/bors or subtree syncs. + +use std::process::Command; + +use build_helper::ci::CiEnv; +use build_helper::git::get_rust_lang_rust_remote; + +macro_rules! try_unwrap_in_ci { + ($expr:expr) => { + match $expr { + Ok(value) => value, + Err(err) if CiEnv::is_ci() => { + panic!("Encountered error while testing Git status: {:?}", err) + } + Err(_) => return, + } + }; +} + +pub fn check(_: (), bad: &mut bool) { + let remote = try_unwrap_in_ci!(get_rust_lang_rust_remote()); + let merge_commits = try_unwrap_in_ci!(find_merge_commits(&remote)); + + let mut bad_merge_commits = merge_commits.lines().filter(|commit| { + !( + // Bors is the ruler of merge commits. + commit.starts_with("Auto merge of") || commit.starts_with("Rollup merge of") + ) + }); + + if let Some(merge) = bad_merge_commits.next() { + tidy_error!( + bad, + "found a merge commit in the history: `{merge}`. +To resolve the issue, see this: https://rustc-dev-guide.rust-lang.org/git.html#i-made-a-merge-commit-by-accident. +If you're doing a subtree sync, add your tool to the list in the code that emitted this error." + ); + } +} + +/// Runs `git log --merges --format=%s $REMOTE/master..HEAD` and returns all commits +fn find_merge_commits(remote: &str) -> Result { + let mut git = Command::new("git"); + git.args([ + "log", + "--merges", + "--format=%s", + &format!("{remote}/master..HEAD"), + // Ignore subtree syncs. Add your new subtrees here. + ":!src/tools/miri", + ":!src/tools/rust-analyzer", + ":!compiler/rustc_smir", + ":!library/portable-simd", + ":!compiler/rustc_codegen_gcc", + ":!src/tools/rustfmt", + ":!compiler/rustc_codegen_cranelift", + ":!src/tools/clippy", + ]); + + let output = git.output().map_err(|err| format!("{err:?}"))?; + if !output.status.success() { + return Err(format!( + "failed to execute git log command: {}", + String::from_utf8_lossy(&output.stderr) + )); + } + + let stdout = String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))?; + + Ok(stdout) +}