From 25e13fd2b74a08bd49f52aa44fa247c6ba0293d7 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 16 Nov 2024 07:13:16 +0100 Subject: [PATCH 01/18] Bootstrap: Don't duplicate cc-rs flags Commands that end up invoking cc-rs, i.e. Cargo (through build scripts) and cmake-rs don't need the CFLAGS from cc-rs itself, as they will just end up as duplicates. --- src/bootstrap/src/core/build_steps/compile.rs | 3 ++- src/bootstrap/src/core/build_steps/llvm.rs | 4 ++-- src/bootstrap/src/core/build_steps/test.rs | 8 +++++-- src/bootstrap/src/core/builder/cargo.rs | 4 ++-- src/bootstrap/src/lib.rs | 21 ++++++++++++------- src/bootstrap/src/utils/cc_detect.rs | 6 ++++-- 6 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 24be705f48194..e013b3843807c 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -1564,7 +1564,8 @@ pub fn compiler_file( return PathBuf::new(); } let mut cmd = command(compiler); - cmd.args(builder.cflags(target, GitRepo::Rustc, c)); + cmd.args(builder.default_cflags(target, c)); + cmd.args(builder.extra_cflags(target, GitRepo::Rustc, c)); cmd.arg(format!("-print-file-name={file}")); let out = cmd.run_capture_stdout(builder).stdout(); PathBuf::from(out.trim()) diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index ffb7d9a9e0e74..3cae5407d0ef9 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -758,7 +758,7 @@ fn configure_cmake( cfg.build_arg("-j").build_arg(builder.jobs().to_string()); let mut cflags: OsString = builder - .cflags(target, GitRepo::Llvm, CLang::C) + .extra_cflags(target, GitRepo::Llvm, CLang::C) .into_iter() .filter(|flag| { !suppressed_compiler_flag_prefixes @@ -778,7 +778,7 @@ fn configure_cmake( } cfg.define("CMAKE_C_FLAGS", cflags); let mut cxxflags: OsString = builder - .cflags(target, GitRepo::Llvm, CLang::Cxx) + .extra_cflags(target, GitRepo::Llvm, CLang::Cxx) .into_iter() .filter(|flag| { !suppressed_compiler_flag_prefixes diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 532c8f767eb10..bb03a00e6584b 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2012,14 +2012,18 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the // Only pass correct values for these flags for the `run-make` suite as it // requires that a C++ compiler was configured which isn't always the case. if !builder.config.dry_run() && mode == "run-make" { + let mut cflags = builder.default_cflags(target, CLang::C); + cflags.extend(builder.extra_cflags(target, GitRepo::Rustc, CLang::C)); + let mut cxxflags = builder.default_cflags(target, CLang::Cxx); + cxxflags.extend(builder.extra_cflags(target, GitRepo::Rustc, CLang::Cxx)); cmd.arg("--cc") .arg(builder.cc(target)) .arg("--cxx") .arg(builder.cxx(target).unwrap()) .arg("--cflags") - .arg(builder.cflags(target, GitRepo::Rustc, CLang::C).join(" ")) + .arg(cflags.join(" ")) .arg("--cxxflags") - .arg(builder.cflags(target, GitRepo::Rustc, CLang::Cxx).join(" ")); + .arg(cxxflags.join(" ")); copts_passed = true; if let Some(ar) = builder.ar(target) { cmd.arg("--ar").arg(ar); diff --git a/src/bootstrap/src/core/builder/cargo.rs b/src/bootstrap/src/core/builder/cargo.rs index 0688a1d689282..cb9b28de5e55e 100644 --- a/src/bootstrap/src/core/builder/cargo.rs +++ b/src/bootstrap/src/core/builder/cargo.rs @@ -307,7 +307,7 @@ impl Cargo { let cc = ccacheify(&builder.cc(target)); self.command.env(format!("CC_{triple_underscored}"), &cc); - let cflags = builder.cflags(target, GitRepo::Rustc, CLang::C).join(" "); + let cflags = builder.extra_cflags(target, GitRepo::Rustc, CLang::C).join(" "); self.command.env(format!("CFLAGS_{triple_underscored}"), &cflags); if let Some(ar) = builder.ar(target) { @@ -319,7 +319,7 @@ impl Cargo { if let Ok(cxx) = builder.cxx(target) { let cxx = ccacheify(&cxx); - let cxxflags = builder.cflags(target, GitRepo::Rustc, CLang::Cxx).join(" "); + let cxxflags = builder.extra_cflags(target, GitRepo::Rustc, CLang::Cxx).join(" "); self.command .env(format!("CXX_{triple_underscored}"), &cxx) .env(format!("CXXFLAGS_{triple_underscored}"), cxxflags); diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index c384fd6bf4351..b71cc1db6acd9 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -250,6 +250,7 @@ impl Mode { } } +#[derive(Debug, Hash, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub enum CLang { C, Cxx, @@ -1187,9 +1188,9 @@ Executed at: {executed_at}"#, self.cc.borrow()[&target].path().into() } - /// Returns a list of flags to pass to the C compiler for the target - /// specified. - fn cflags(&self, target: TargetSelection, which: GitRepo, c: CLang) -> Vec { + /// Returns C flags that `cc-rs` thinks should be enabled for the + /// specified target by default. + fn default_cflags(&self, target: TargetSelection, c: CLang) -> Vec { if self.config.dry_run() { return Vec::new(); } @@ -1198,14 +1199,18 @@ Executed at: {executed_at}"#, CLang::Cxx => self.cxx.borrow()[&target].clone(), }; - // Filter out -O and /O (the optimization flags) that we picked up from - // cc-rs because the build scripts will determine that for themselves. - let mut base = base - .args() + // Filter out -O and /O (the optimization flags) that we picked up + // from cc-rs, that's up to the caller to figure out. + base.args() .iter() .map(|s| s.to_string_lossy().into_owned()) .filter(|s| !s.starts_with("-O") && !s.starts_with("/O")) - .collect::>(); + .collect::>() + } + + /// Returns extra C flags that `cc-rs` doesn't handle. + fn extra_cflags(&self, target: TargetSelection, which: GitRepo, c: CLang) -> Vec { + let mut base = Vec::new(); // If we're compiling C++ on macOS then we add a flag indicating that // we want libc++ (more filled out than libstdc++), ensuring that diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index 0df0046945220..d20651f843b6b 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/src/bootstrap/src/utils/cc_detect.rs @@ -132,7 +132,8 @@ pub fn find_target(build: &Build, target: TargetSelection) { }; build.cc.borrow_mut().insert(target, compiler.clone()); - let cflags = build.cflags(target, GitRepo::Rustc, CLang::C); + let mut cflags = build.default_cflags(target, CLang::C); + cflags.extend(build.extra_cflags(target, GitRepo::Rustc, CLang::C)); // If we use llvm-libunwind, we will need a C++ compiler as well for all targets // We'll need one anyways if the target triple is also a host triple @@ -158,7 +159,8 @@ pub fn find_target(build: &Build, target: TargetSelection) { build.verbose(|| println!("CC_{} = {:?}", target.triple, build.cc(target))); build.verbose(|| println!("CFLAGS_{} = {cflags:?}", target.triple)); if let Ok(cxx) = build.cxx(target) { - let cxxflags = build.cflags(target, GitRepo::Rustc, CLang::Cxx); + let mut cxxflags = build.default_cflags(target, CLang::Cxx); + cxxflags.extend(build.extra_cflags(target, GitRepo::Rustc, CLang::Cxx)); build.verbose(|| println!("CXX_{} = {cxx:?}", target.triple)); build.verbose(|| println!("CXXFLAGS_{} = {cxxflags:?}", target.triple)); } From 8729ba31d31d834a59a6fa3573d56dab73a16af1 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 16 Nov 2024 07:24:07 +0100 Subject: [PATCH 02/18] Remove outdated workaround for -mmacosx-version-min= This didn't actually activate properly, since cmake-rs calls cc-rs internally, and as such the flag was set anyhow. --- src/bootstrap/src/core/build_steps/llvm.rs | 48 ++++------------------ 1 file changed, 8 insertions(+), 40 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 3cae5407d0ef9..e778f2559593e 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -534,7 +534,7 @@ impl Step for Llvm { cfg.define("LLVM_VERSION_SUFFIX", suffix); } - configure_cmake(builder, target, &mut cfg, true, ldflags, &[]); + configure_cmake(builder, target, &mut cfg, true, ldflags); configure_llvm(builder, target, &mut cfg); for (key, val) in &builder.config.llvm_build_config { @@ -623,7 +623,6 @@ fn configure_cmake( cfg: &mut cmake::Config, use_compiler_launcher: bool, mut ldflags: LdFlags, - suppressed_compiler_flag_prefixes: &[&str], ) { // Do not print installation messages for up-to-date files. // LLVM and LLD builds can produce a lot of those and hit CI limits on log size. @@ -757,17 +756,8 @@ fn configure_cmake( } cfg.build_arg("-j").build_arg(builder.jobs().to_string()); - let mut cflags: OsString = builder - .extra_cflags(target, GitRepo::Llvm, CLang::C) - .into_iter() - .filter(|flag| { - !suppressed_compiler_flag_prefixes - .iter() - .any(|suppressed_prefix| flag.starts_with(suppressed_prefix)) - }) - .collect::>() - .join(" ") - .into(); + let mut cflags: OsString = + builder.extra_cflags(target, GitRepo::Llvm, CLang::C).join(" ").into(); if let Some(ref s) = builder.config.llvm_cflags { cflags.push(" "); cflags.push(s); @@ -777,17 +767,8 @@ fn configure_cmake( cflags.push(format!(" --target={target}")); } cfg.define("CMAKE_C_FLAGS", cflags); - let mut cxxflags: OsString = builder - .extra_cflags(target, GitRepo::Llvm, CLang::Cxx) - .into_iter() - .filter(|flag| { - !suppressed_compiler_flag_prefixes - .iter() - .any(|suppressed_prefix| flag.starts_with(suppressed_prefix)) - }) - .collect::>() - .join(" ") - .into(); + let mut cxxflags: OsString = + builder.extra_cflags(target, GitRepo::Llvm, CLang::Cxx).join(" ").into(); if let Some(ref s) = builder.config.llvm_cxxflags { cxxflags.push(" "); cxxflags.push(s); @@ -950,7 +931,7 @@ impl Step for Enzyme { // FIXME(ZuseZ4): Find a nicer way to use Enzyme Debug builds //cfg.profile("Debug"); //cfg.define("CMAKE_BUILD_TYPE", "Debug"); - configure_cmake(builder, target, &mut cfg, true, LdFlags::default(), &[]); + configure_cmake(builder, target, &mut cfg, true, LdFlags::default()); // Re-use the same flags as llvm to control the level of debug information // generated for lld. @@ -1065,7 +1046,7 @@ impl Step for Lld { ldflags.push_all("-Wl,-rpath,'$ORIGIN/../../../'"); } - configure_cmake(builder, target, &mut cfg, true, ldflags, &[]); + configure_cmake(builder, target, &mut cfg, true, ldflags); configure_llvm(builder, target, &mut cfg); // Re-use the same flags as llvm to control the level of debug information @@ -1164,20 +1145,7 @@ impl Step for Sanitizers { // Unfortunately sccache currently lacks support to build them successfully. // Disable compiler launcher on Darwin targets to avoid potential issues. let use_compiler_launcher = !self.target.contains("apple-darwin"); - // Since v1.0.86, the cc crate adds -mmacosx-version-min to the default - // flags on MacOS. A long-standing bug in the CMake rules for compiler-rt - // causes architecture detection to be skipped when this flag is present, - // and compilation fails. https://github.com/llvm/llvm-project/issues/88780 - let suppressed_compiler_flag_prefixes: &[&str] = - if self.target.contains("apple-darwin") { &["-mmacosx-version-min="] } else { &[] }; - configure_cmake( - builder, - self.target, - &mut cfg, - use_compiler_launcher, - LdFlags::default(), - suppressed_compiler_flag_prefixes, - ); + configure_cmake(builder, self.target, &mut cfg, use_compiler_launcher, LdFlags::default()); t!(fs::create_dir_all(&out_dir)); cfg.out_dir(out_dir); From f816d33c8386b1e0d2f34f369ff518a8282bbdfa Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 16 Nov 2024 04:51:27 +0100 Subject: [PATCH 03/18] Always set the deployment target when building std --- src/bootstrap/src/core/build_steps/compile.rs | 51 ++++++++++++++++++- src/bootstrap/src/core/builder/cargo.rs | 4 ++ .../run-make/apple-deployment-target/rmake.rs | 23 ++++++++- 3 files changed, 75 insertions(+), 3 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index e013b3843807c..7665ee7b462e3 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -462,8 +462,55 @@ fn compiler_rt_for_profiler(builder: &Builder<'_>) -> PathBuf { /// Configure cargo to compile the standard library, adding appropriate env vars /// and such. pub fn std_cargo(builder: &Builder<'_>, target: TargetSelection, stage: u32, cargo: &mut Cargo) { - if let Some(target) = env::var_os("MACOSX_STD_DEPLOYMENT_TARGET") { - cargo.env("MACOSX_DEPLOYMENT_TARGET", target); + // rustc already ensures that it builds with the minimum deployment + // target, so ideally we shouldn't need to do anything here. + // + // However, `cc` currently defaults to a higher version for backwards + // compatibility, which means that compiler-rt, which is built via + // compiler-builtins' build script, gets built with a higher deployment + // target. This in turn causes warnings while linking, and is generally + // a compatibility hazard. + // + // So, at least until https://github.com/rust-lang/cc-rs/issues/1171, or + // perhaps https://github.com/rust-lang/cargo/issues/13115 is resolved, we + // explicitly set the deployment target environment variables to avoid + // this issue. + // + // This place also serves as an extension point if we ever wanted to raise + // rustc's default deployment target while keeping the prebuilt `std` at + // a lower version, so it's kinda nice to have in any case. + if target.contains("apple") && !builder.config.dry_run() { + // Query rustc for the deployment target. + let mut cmd = command(builder.rustc(cargo.compiler())); + cmd.arg("--target").arg(target.rustc_target_arg()); + cmd.arg("--print=deployment-target"); + let output = cmd.run_capture_stdout(builder).stdout(); + + let value = output.split('=').last().unwrap().trim(); + + // FIXME: Simplify after https://github.com/rust-lang/rust/pull/133041 + let env_var = if target.contains("apple-darwin") { + "MACOSX_DEPLOYMENT_TARGET" + } else if target.contains("apple-ios") { + "IPHONEOS_DEPLOYMENT_TARGET" + } else if target.contains("apple-tvos") { + "TVOS_DEPLOYMENT_TARGET" + } else if target.contains("apple-watchos") { + "WATCHOS_DEPLOYMENT_TARGET" + } else if target.contains("apple-visionos") { + "XROS_DEPLOYMENT_TARGET" + } else { + panic!("unknown target OS for apple target"); + }; + + // Unconditionally set the env var (if it was set in the environment + // already, rustc should've picked that up). + cargo.env(env_var, value); + + // Allow CI to override the deployment target for `std`. + if let Some(target) = env::var_os("MACOSX_STD_DEPLOYMENT_TARGET") { + cargo.env("MACOSX_DEPLOYMENT_TARGET", target); + } } // Paths needed by `library/profiler_builtins/build.rs`. diff --git a/src/bootstrap/src/core/builder/cargo.rs b/src/bootstrap/src/core/builder/cargo.rs index cb9b28de5e55e..fca46ff443f3a 100644 --- a/src/bootstrap/src/core/builder/cargo.rs +++ b/src/bootstrap/src/core/builder/cargo.rs @@ -122,6 +122,10 @@ impl Cargo { cargo } + pub fn compiler(&self) -> Compiler { + self.compiler + } + pub fn into_cmd(self) -> BootstrapCommand { self.into() } diff --git a/tests/run-make/apple-deployment-target/rmake.rs b/tests/run-make/apple-deployment-target/rmake.rs index fed6d310770c0..6fe6bee422552 100644 --- a/tests/run-make/apple-deployment-target/rmake.rs +++ b/tests/run-make/apple-deployment-target/rmake.rs @@ -7,7 +7,11 @@ //@ only-apple -use run_make_support::{apple_os, cmd, run_in_tmpdir, rustc, target}; +use std::collections::HashSet; + +use run_make_support::{ + apple_os, cmd, has_extension, path, regex, run_in_tmpdir, rustc, shallow_find_files, target, +}; /// Run vtool to check the `minos` field in LC_BUILD_VERSION. /// @@ -156,4 +160,21 @@ fn main() { rustc().env_remove(env_var).run(); minos("foo.o", default_version); }); + + // Test that all binaries in rlibs produced by `rustc` have the same version. + // Regression test for https://github.com/rust-lang/rust/issues/128419. + let sysroot = rustc().print("sysroot").run().stdout_utf8(); + let target_sysroot = path(sysroot.trim()).join("lib/rustlib").join(target()).join("lib"); + let rlibs = shallow_find_files(&target_sysroot, |path| has_extension(path, "rlib")); + + let output = cmd("otool").arg("-l").args(rlibs).run().stdout_utf8(); + let re = regex::Regex::new(r"(minos|version) ([0-9.]*)").unwrap(); + let mut versions = HashSet::new(); + for (_, [_, version]) in re.captures_iter(&output).map(|c| c.extract()) { + versions.insert(version); + } + // FIXME(madsmtm): See above for aarch64-apple-watchos. + if versions.len() != 1 && target() != "aarch64-apple-watchos" { + panic!("std rlibs contained multiple different deployment target versions: {versions:?}"); + } } From 673b3d380fb2e0607bfdd875eafd0bffa81e0a91 Mon Sep 17 00:00:00 2001 From: Walnut <39544927+Walnut356@users.noreply.github.com> Date: Sun, 17 Nov 2024 09:18:42 -0600 Subject: [PATCH 04/18] restrict synthetic types to standard library types --- src/etc/lldb_commands | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/etc/lldb_commands b/src/etc/lldb_commands index 4be2dba34f6f8..8a2ed0835b758 100644 --- a/src/etc/lldb_commands +++ b/src/etc/lldb_commands @@ -1,4 +1,23 @@ -type synthetic add -l lldb_lookup.synthetic_lookup -x ".*" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(alloc::([a-z_]+::)+)String$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^&(mut )?str$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^&(mut )?\\[.+\\]$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(std::ffi::([a-z_]+::)+)OsString$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(alloc::([a-z_]+::)+)Vec<.+>$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(alloc::([a-z_]+::)+)VecDeque<.+>$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(alloc::([a-z_]+::)+)BTreeSet<.+>$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(alloc::([a-z_]+::)+)BTreeMap<.+>$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(std::collections::([a-z_]+::)+)HashMap<.+>$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(std::collections::([a-z_]+::)+)HashSet<.+>$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(alloc::([a-z_]+::)+)Rc<.+>$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(alloc::([a-z_]+::)+)Arc<.+>$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(core::([a-z_]+::)+)Cell<.+>$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(core::([a-z_]+::)+)Ref<.+>$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(core::([a-z_]+::)+)RefMut<.+>$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(core::([a-z_]+::)+)RefCell<.+>$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(core::([a-z_]+::)+)NonZero<.+>$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^core::num::([a-z_]+::)*NonZero.+$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^(std::([a-z_]+::)+)PathBuf$" --category Rust +type synthetic add -l lldb_lookup.synthetic_lookup -x "^&(mut )?(std::([a-z_]+::)+)Path$" --category Rust type summary add -F lldb_lookup.summary_lookup -e -x -h "^(alloc::([a-z_]+::)+)String$" --category Rust type summary add -F lldb_lookup.summary_lookup -e -x -h "^&(mut )?str$" --category Rust type summary add -F lldb_lookup.summary_lookup -e -x -h "^&(mut )?\\[.+\\]$" --category Rust From ff3dab47fb0a6dd0d6cdc41dbb0e8cd3ed2dffc9 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 25 Nov 2024 08:00:22 +0100 Subject: [PATCH 05/18] Explain why MACOSX_STD_DEPLOYMENT_TARGET exist --- src/bootstrap/src/core/build_steps/compile.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 7665ee7b462e3..71f0f2144c9f5 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -507,7 +507,15 @@ pub fn std_cargo(builder: &Builder<'_>, target: TargetSelection, stage: u32, car // already, rustc should've picked that up). cargo.env(env_var, value); - // Allow CI to override the deployment target for `std`. + // Allow CI to override the deployment target for `std` on macOS. + // + // This is useful because we might want the host tooling LLVM, `rustc` + // and Cargo to have a different deployment target than `std` itself + // (currently, these two versions are the same, but in the past, we + // supported macOS 10.7 for user code and macOS 10.8 in host tooling). + // + // It is not necessary on the other platforms, since only macOS has + // support for host tooling. if let Some(target) = env::var_os("MACOSX_STD_DEPLOYMENT_TARGET") { cargo.env("MACOSX_DEPLOYMENT_TARGET", target); } From 8ecac88d7a999afbcb6cf9f35ee4ef0589095e55 Mon Sep 17 00:00:00 2001 From: Walnut <39544927+Walnut356@users.noreply.github.com> Date: Thu, 28 Nov 2024 08:09:45 -0600 Subject: [PATCH 06/18] force expanded formatting for non-synthetic types --- src/etc/lldb_commands | 1 + 1 file changed, 1 insertion(+) diff --git a/src/etc/lldb_commands b/src/etc/lldb_commands index 8a2ed0835b758..2811a00d89839 100644 --- a/src/etc/lldb_commands +++ b/src/etc/lldb_commands @@ -18,6 +18,7 @@ type synthetic add -l lldb_lookup.synthetic_lookup -x "^(core::([a-z_]+::)+)NonZ type synthetic add -l lldb_lookup.synthetic_lookup -x "^core::num::([a-z_]+::)*NonZero.+$" --category Rust type synthetic add -l lldb_lookup.synthetic_lookup -x "^(std::([a-z_]+::)+)PathBuf$" --category Rust type synthetic add -l lldb_lookup.synthetic_lookup -x "^&(mut )?(std::([a-z_]+::)+)Path$" --category Rust +type summary add -F _ -e -x -h "^.*$" --category Rust type summary add -F lldb_lookup.summary_lookup -e -x -h "^(alloc::([a-z_]+::)+)String$" --category Rust type summary add -F lldb_lookup.summary_lookup -e -x -h "^&(mut )?str$" --category Rust type summary add -F lldb_lookup.summary_lookup -e -x -h "^&(mut )?\\[.+\\]$" --category Rust From ba7316655645c020263e207ee9c036131b511d45 Mon Sep 17 00:00:00 2001 From: Julian Frimmel Date: Sun, 6 Oct 2024 12:33:25 +0200 Subject: [PATCH 07/18] Support `clobber_abi` for AVR inline assembly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds the relevant registers to the list of clobbered regis- ters (part of #93335). This follows the [ABI documentation] of AVR-GCC: > The [...] call-clobbered general purpose registers (GPRs) are > registers that might be destroyed (clobbered) by a function call. > > - **R18–R27, R30, R31** > > These GPRs are call clobbered. An ordinary function may use them > without restoring the contents. [...] > > - **R0, T-Flag** > > The temporary register and the T-flag in SREG are also call- > clobbered, but this knowledge is not exposed explicitly to the > compiler (R0 is a fixed register). Therefore this commit lists the aforementioned registers `r18–r27`, `r30` and `r31` as clobbered registers. Since the `r0` register (listed above as well) is not available in inline assembly at all (potentially because the AVR-GCC considers it a fixed register causing the register to never be used in register allocation and LLVM adopting this), there is no need to list it in the clobber list (the `r0`-variant is not even available). A comment was added to ensure, that the `r0` gets added to the clobber-list once the register gets usable in inline ASM. Since the SREG is normally considered clobbered anyways (unless the user supplies the `preserve_flags`-option), there is no need to explicitly list a bit in this register (which is not possible to list anyways). Note, that this commit completely ignores the case of interrupts (that are described in the ABI-specification), since every register touched in an ISR need to be saved anyways. [ABI documentation]: https://gcc.gnu.org/wiki/avr-gcc#Call-Used_Registers --- compiler/rustc_target/src/asm/avr.rs | 5 +++++ compiler/rustc_target/src/asm/mod.rs | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/compiler/rustc_target/src/asm/avr.rs b/compiler/rustc_target/src/asm/avr.rs index 276f376b297e6..9adcbecdf3c10 100644 --- a/compiler/rustc_target/src/asm/avr.rs +++ b/compiler/rustc_target/src/asm/avr.rs @@ -106,6 +106,11 @@ def_regs! { "the stack pointer cannot be used as an operand for inline asm", #error = ["r0", "r1", "r1r0"] => "r0 and r1 are not available due to an issue in LLVM", + // If this changes within LLVM, the compiler might use the registers + // in the future. This must be reflected in the set of clobbered + // registers, else the clobber ABI implementation is *unsound*, as + // this generates invalid code (register is not marked as clobbered + // but may change the register content). } } diff --git a/compiler/rustc_target/src/asm/mod.rs b/compiler/rustc_target/src/asm/mod.rs index 9fe733e063cf5..204d5d5336102 100644 --- a/compiler/rustc_target/src/asm/mod.rs +++ b/compiler/rustc_target/src/asm/mod.rs @@ -928,6 +928,7 @@ pub enum InlineAsmClobberAbi { AArch64, AArch64NoX18, Arm64EC, + Avr, RiscV, RiscVE, LoongArch, @@ -986,6 +987,10 @@ impl InlineAsmClobberAbi { }), _ => Err(&["C", "system", "efiapi"]), }, + InlineAsmArch::Avr => match name { + "C" | "system" => Ok(InlineAsmClobberAbi::Avr), + _ => Err(&["C", "system"]), + }, InlineAsmArch::LoongArch64 => match name { "C" | "system" => Ok(InlineAsmClobberAbi::LoongArch), _ => Err(&["C", "system"]), @@ -1133,6 +1138,23 @@ impl InlineAsmClobberAbi { d24, d25, d26, d27, d28, d29, d30, d31, } }, + InlineAsmClobberAbi::Avr => clobbered_regs! { + Avr AvrInlineAsmReg { + // The list of "Call-Used Registers" according to + // https://gcc.gnu.org/wiki/avr-gcc#Call-Used_Registers + + // Clobbered registers available in inline assembly + r18, r19, r20, r21, r22, r23, r24, r25, r26, r27, r30, r31, + // As per the AVR-GCC-ABI documentation linked above, the R0 + // register is a clobbered register as well. Since we don't + // allow the usage of R0 in inline assembly, nothing has to + // be done here. + // Likewise, the T-flag in the SREG should be clobbered, but + // this is not necessary to be listed here, since the SREG + // is considered clobbered anyways unless `preserve_flags` + // is used. + } + }, InlineAsmClobberAbi::RiscV => clobbered_regs! { RiscV RiscVInlineAsmReg { // ra From d7e0a3eee0c5e79134664293eea3c46b91f0bec7 Mon Sep 17 00:00:00 2001 From: Julian Frimmel Date: Sun, 6 Oct 2024 13:09:29 +0200 Subject: [PATCH 08/18] Add test case for the clobber options --- tests/codegen/asm-clobber_abi-avr.rs | 43 ++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 tests/codegen/asm-clobber_abi-avr.rs diff --git a/tests/codegen/asm-clobber_abi-avr.rs b/tests/codegen/asm-clobber_abi-avr.rs new file mode 100644 index 0000000000000..6e0c75368e23f --- /dev/null +++ b/tests/codegen/asm-clobber_abi-avr.rs @@ -0,0 +1,43 @@ +//@ assembly-output: emit-asm +//@ compile-flags: --target avr-unknown-gnu-atmega328 +//@ needs-llvm-components: avr + +#![crate_type = "rlib"] +#![feature(no_core, rustc_attrs, lang_items, asm_experimental_arch)] +#![no_core] + +#[lang = "sized"] +trait Sized {} + +#[rustc_builtin_macro] +macro_rules! asm { + () => {}; +} + +// CHECK-LABEL: @sreg_is_clobbered +// CHECK: void asm sideeffect "", "~{sreg}"() +#[no_mangle] +pub unsafe fn sreg_is_clobbered() { + asm!("", options(nostack, nomem)); +} + +// CHECK-LABEL: @sreg_is_not_clobbered_if_preserve_flags_is_used +// CHECK: void asm sideeffect "", ""() +#[no_mangle] +pub unsafe fn sreg_is_not_clobbered_if_preserve_flags_is_used() { + asm!("", options(nostack, nomem, preserves_flags)); +} + +// CHECK-LABEL: @clobber_abi +// CHECK: asm sideeffect "", "={r18},={r19},={r20},={r21},={r22},={r23},={r24},={r25},={r26},={r27},={r30},={r31},~{sreg}"() +#[no_mangle] +pub unsafe fn clobber_abi() { + asm!("", clobber_abi("C"), options(nostack, nomem)); +} + +// CHECK-LABEL: @clobber_abi_with_preserved_flags +// CHECK: asm sideeffect "", "={r18},={r19},={r20},={r21},={r22},={r23},={r24},={r25},={r26},={r27},={r30},={r31}"() +#[no_mangle] +pub unsafe fn clobber_abi_with_preserved_flags() { + asm!("", clobber_abi("C"), options(nostack, nomem, preserves_flags)); +} From 2bd3bbb2e0132d6d9c3766f35c2efc34414a8a66 Mon Sep 17 00:00:00 2001 From: Julian Frimmel Date: Sat, 2 Nov 2024 08:25:53 +0100 Subject: [PATCH 09/18] Move & rename test case to match naming of #132456 --- tests/codegen/{asm-clobber_abi-avr.rs => asm/avr-clobbers.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/codegen/{asm-clobber_abi-avr.rs => asm/avr-clobbers.rs} (100%) diff --git a/tests/codegen/asm-clobber_abi-avr.rs b/tests/codegen/asm/avr-clobbers.rs similarity index 100% rename from tests/codegen/asm-clobber_abi-avr.rs rename to tests/codegen/asm/avr-clobbers.rs From 67d2f3f6857a1d28113f1a63eb003b53d17088c9 Mon Sep 17 00:00:00 2001 From: Julian Frimmel Date: Thu, 28 Nov 2024 12:57:42 +0100 Subject: [PATCH 10/18] Reword error message of reserved AVR registers Those are reserved as per the GCC (and thus LLVM) ABI, which is distinct from an issue. The rewording was requested in this [review]. [review]: https://github.com/rust-lang/rust/pull/131323#issuecomment-2479178721 --- compiler/rustc_target/src/asm/avr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_target/src/asm/avr.rs b/compiler/rustc_target/src/asm/avr.rs index 9adcbecdf3c10..55d393c81d3ec 100644 --- a/compiler/rustc_target/src/asm/avr.rs +++ b/compiler/rustc_target/src/asm/avr.rs @@ -105,7 +105,7 @@ def_regs! { #error = ["SP", "SPL", "SPH"] => "the stack pointer cannot be used as an operand for inline asm", #error = ["r0", "r1", "r1r0"] => - "r0 and r1 are not available due to an issue in LLVM", + "LLVM reserves r0 (scratch register) and r1 (zero register)", // If this changes within LLVM, the compiler might use the registers // in the future. This must be reflected in the set of clobbered // registers, else the clobber ABI implementation is *unsound*, as From 76adf05cfbffd78fc3f9e123634e992a70798e8f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 29 Nov 2024 06:05:56 +1100 Subject: [PATCH 11/18] Rename `-Zparse-only`. I was surprised to find that running with `-Zparse-only` only parses the crate root file. Other files aren't parsed because that happens later during expansion. This commit renames the option and updates the help message to make this clearer. --- compiler/rustc_driver_impl/src/lib.rs | 4 +++- compiler/rustc_interface/src/tests.rs | 2 +- compiler/rustc_session/src/config.rs | 4 ++-- compiler/rustc_session/src/options.rs | 5 +++-- tests/ui/generic-associated-types/parse/in-trait-impl.rs | 2 +- tests/ui/generic-associated-types/parse/in-trait.rs | 2 +- tests/ui/impl-trait/impl-trait-plus-priority.rs | 2 +- tests/ui/parser/assoc/assoc-oddities-1.rs | 2 +- tests/ui/parser/assoc/assoc-oddities-2.rs | 2 +- tests/ui/parser/bounds-type.rs | 2 +- tests/ui/parser/impl-qpath.rs | 2 +- tests/ui/parser/issues/issue-17904.rs | 2 +- tests/ui/traits/const-traits/syntax.rs | 2 +- tests/ui/traits/const-traits/tilde-const-syntax.rs | 2 +- tests/ui/traits/const-traits/tilde-twice.rs | 2 +- 15 files changed, 20 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 256266d2965d4..ee5a05a0b9de9 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -425,7 +425,9 @@ fn run_compiler( return early_exit(); } - if sess.opts.unstable_opts.parse_only || sess.opts.unstable_opts.show_span.is_some() { + if sess.opts.unstable_opts.parse_crate_root_only + || sess.opts.unstable_opts.show_span.is_some() + { return early_exit(); } diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 6beae14100d9a..c1b2d8562522a 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -712,7 +712,7 @@ fn test_unstable_options_tracking_hash() { untracked!(no_analysis, true); untracked!(no_leak_check, true); untracked!(no_parallel_backend, true); - untracked!(parse_only, true); + untracked!(parse_crate_root_only, true); // `pre_link_arg` is omitted because it just forwards to `pre_link_args`. untracked!(pre_link_args, vec![String::from("abc"), String::from("def")]); untracked!(print_codegen_stats, true); diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 0124397ea46f3..cb6d539cdf940 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -1208,7 +1208,7 @@ impl Options { /// Returns `true` if there will be an output file generated. pub fn will_create_output_file(&self) -> bool { - !self.unstable_opts.parse_only && // The file is just being parsed + !self.unstable_opts.parse_crate_root_only && // The file is just being parsed self.unstable_opts.ls.is_empty() // The file is just being queried } @@ -1864,7 +1864,7 @@ fn parse_output_types( matches: &getopts::Matches, ) -> OutputTypes { let mut output_types = BTreeMap::new(); - if !unstable_opts.parse_only { + if !unstable_opts.parse_crate_root_only { for list in matches.opt_strs("emit") { for output_type in list.split(',') { let (shorthand, path) = split_out_file_name(output_type); diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index a2d75917c8266..47c191e67baac 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1937,8 +1937,9 @@ options! { "support compiling tests with panic=abort (default: no)"), panic_in_drop: PanicStrategy = (PanicStrategy::Unwind, parse_panic_strategy, [TRACKED], "panic strategy for panics in drops"), - parse_only: bool = (false, parse_bool, [UNTRACKED], - "parse only; do not compile, assemble, or link (default: no)"), + parse_crate_root_only: bool = (false, parse_bool, [UNTRACKED], + "parse the crate root file only; do not parse other files, compile, assemble, or link \ + (default: no)"), patchable_function_entry: PatchableFunctionEntry = (PatchableFunctionEntry::default(), parse_patchable_function_entry, [TRACKED], "nop padding at function entry"), plt: Option = (None, parse_opt_bool, [TRACKED], diff --git a/tests/ui/generic-associated-types/parse/in-trait-impl.rs b/tests/ui/generic-associated-types/parse/in-trait-impl.rs index 5ba42be358312..ef67fb18228fb 100644 --- a/tests/ui/generic-associated-types/parse/in-trait-impl.rs +++ b/tests/ui/generic-associated-types/parse/in-trait-impl.rs @@ -1,5 +1,5 @@ //@ check-pass -//@ compile-flags: -Z parse-only +//@ compile-flags: -Z parse-crate-root-only impl Baz for T where T: Foo { type Quux<'a> = ::Bar<'a, 'static>; diff --git a/tests/ui/generic-associated-types/parse/in-trait.rs b/tests/ui/generic-associated-types/parse/in-trait.rs index 913eceec0dacf..2add908d727db 100644 --- a/tests/ui/generic-associated-types/parse/in-trait.rs +++ b/tests/ui/generic-associated-types/parse/in-trait.rs @@ -1,5 +1,5 @@ //@ check-pass -//@ compile-flags: -Z parse-only +//@ compile-flags: -Z parse-crate-root-only use std::ops::Deref; use std::fmt::Debug; diff --git a/tests/ui/impl-trait/impl-trait-plus-priority.rs b/tests/ui/impl-trait/impl-trait-plus-priority.rs index 5441a015ac0ac..5575493a17d36 100644 --- a/tests/ui/impl-trait/impl-trait-plus-priority.rs +++ b/tests/ui/impl-trait/impl-trait-plus-priority.rs @@ -1,4 +1,4 @@ -//@ compile-flags: -Z parse-only +//@ compile-flags: -Z parse-crate-root-only fn f() -> impl A + {} // OK fn f() -> impl A + B {} // OK diff --git a/tests/ui/parser/assoc/assoc-oddities-1.rs b/tests/ui/parser/assoc/assoc-oddities-1.rs index 246546ac03420..c1b305a4eeb24 100644 --- a/tests/ui/parser/assoc/assoc-oddities-1.rs +++ b/tests/ui/parser/assoc/assoc-oddities-1.rs @@ -1,4 +1,4 @@ -//@ compile-flags: -Z parse-only +//@ compile-flags: -Z parse-crate-root-only fn main() { // following lines below parse and must not fail diff --git a/tests/ui/parser/assoc/assoc-oddities-2.rs b/tests/ui/parser/assoc/assoc-oddities-2.rs index aee2af41d62a6..82cf7d79c0d57 100644 --- a/tests/ui/parser/assoc/assoc-oddities-2.rs +++ b/tests/ui/parser/assoc/assoc-oddities-2.rs @@ -1,4 +1,4 @@ -//@ compile-flags: -Z parse-only +//@ compile-flags: -Z parse-crate-root-only fn main() { // see assoc-oddities-1 for explanation diff --git a/tests/ui/parser/bounds-type.rs b/tests/ui/parser/bounds-type.rs index 7cee6def32f83..ec0e83c314e1d 100644 --- a/tests/ui/parser/bounds-type.rs +++ b/tests/ui/parser/bounds-type.rs @@ -1,4 +1,4 @@ -//@ compile-flags: -Z parse-only +//@ compile-flags: -Z parse-crate-root-only //@ edition: 2021 struct S< diff --git a/tests/ui/parser/impl-qpath.rs b/tests/ui/parser/impl-qpath.rs index d7c4989b6e4c4..fed026792c9d1 100644 --- a/tests/ui/parser/impl-qpath.rs +++ b/tests/ui/parser/impl-qpath.rs @@ -1,5 +1,5 @@ //@ check-pass -//@ compile-flags: -Z parse-only +//@ compile-flags: -Z parse-crate-root-only impl <*const u8>::AssocTy {} // OK impl ::AssocTy {} // OK diff --git a/tests/ui/parser/issues/issue-17904.rs b/tests/ui/parser/issues/issue-17904.rs index 6f77d4bb086fb..99a3b13989885 100644 --- a/tests/ui/parser/issues/issue-17904.rs +++ b/tests/ui/parser/issues/issue-17904.rs @@ -1,4 +1,4 @@ -//@ compile-flags: -Zparse-only +//@ compile-flags: -Zparse-crate-root-only struct Baz where U: Eq(U); //This is parsed as the new Fn* style parenthesis syntax. struct Baz where U: Eq(U) -> R; // Notice this parses as well. diff --git a/tests/ui/traits/const-traits/syntax.rs b/tests/ui/traits/const-traits/syntax.rs index 1064713ac592c..cfac6e0a93e38 100644 --- a/tests/ui/traits/const-traits/syntax.rs +++ b/tests/ui/traits/const-traits/syntax.rs @@ -1,4 +1,4 @@ -//@ compile-flags: -Z parse-only +//@ compile-flags: -Z parse-crate-root-only //@ check-pass #![feature(const_trait_bound_opt_out)] diff --git a/tests/ui/traits/const-traits/tilde-const-syntax.rs b/tests/ui/traits/const-traits/tilde-const-syntax.rs index d65ecae3d067c..f9944c426ccee 100644 --- a/tests/ui/traits/const-traits/tilde-const-syntax.rs +++ b/tests/ui/traits/const-traits/tilde-const-syntax.rs @@ -1,4 +1,4 @@ -//@ compile-flags: -Z parse-only +//@ compile-flags: -Z parse-crate-root-only //@ check-pass #![feature(const_trait_impl)] diff --git a/tests/ui/traits/const-traits/tilde-twice.rs b/tests/ui/traits/const-traits/tilde-twice.rs index c3f9f8e676480..d341513b8a819 100644 --- a/tests/ui/traits/const-traits/tilde-twice.rs +++ b/tests/ui/traits/const-traits/tilde-twice.rs @@ -1,4 +1,4 @@ -//@ compile-flags: -Z parse-only +//@ compile-flags: -Z parse-crate-root-only #![feature(const_trait_impl)] From accdfa1e526af99c4c60412a12a9a8253eab5984 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 29 Nov 2024 06:08:48 +1100 Subject: [PATCH 12/18] Update `-Zshow-span` help message. To clarify how it works. --- compiler/rustc_session/src/options.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 47c191e67baac..25f75ae12e8e5 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -2037,7 +2037,7 @@ written to standard error output)"), shell_argfiles: bool = (false, parse_bool, [UNTRACKED], "allow argument files to be specified with POSIX \"shell-style\" argument quoting"), show_span: Option = (None, parse_opt_string, [TRACKED], - "show spans for compiler debugging (expr|pat|ty)"), + "show spans in the crate root file, for compiler debugging (expr|pat|ty)"), simulate_remapped_rust_src_base: Option = (None, parse_opt_pathbuf, [TRACKED], "simulate the effect of remap-debuginfo = true at bootstrapping by remapping path \ to rust's source base directory. only meant for testing purposes"), From c52d952a6aae3c850dba192a79662b990eb8dc11 Mon Sep 17 00:00:00 2001 From: Maybe Lapkin Date: Thu, 28 Nov 2024 20:31:08 +0100 Subject: [PATCH 13/18] add instructions for generating `flake.lock` to `envrc-flake` Previous setup instructions did not work without. (i.e. the envrc would not do anything, `nix flake show..` would provide unhelpful error) --- src/tools/nix-dev-shell/envrc-flake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/nix-dev-shell/envrc-flake b/src/tools/nix-dev-shell/envrc-flake index 218d88d8721fa..9def420f05cb4 100644 --- a/src/tools/nix-dev-shell/envrc-flake +++ b/src/tools/nix-dev-shell/envrc-flake @@ -1,7 +1,7 @@ # If you want to use this as an .envrc file to create a shell with necessery components # to develop rustc, use the following command in the root of the rusr checkout: # -# ln -s ./src/tools/nix-dev-shell/envrc-flake ./.envrc && echo .envrc >> .git/info/exclude +# ln -s ./src/tools/nix-dev-shell/envrc-flake ./.envrc && nix flake update --flake ./src/tools/nix-dev-shell && echo .envrc >> .git/info/exclude if nix flake show path:./src/tools/nix-dev-shell &> /dev/null; then use flake path:./src/tools/nix-dev-shell From 33f13f2ab575bb9d702217f25b9ea45fc527bbf2 Mon Sep 17 00:00:00 2001 From: Maybe Lapkin Date: Thu, 28 Nov 2024 20:34:30 +0100 Subject: [PATCH 14/18] ignore `/build` instead of `build/` in gitignore this does two things: 1. allows making `build` a symlink (which is not considered a directory by git, thus removal of trailing `/`). 2. removes the need to special case `rustc_mir_build/src/build` (leading `/` makes git only ignore the `build` in the root) --- .gitignore | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 948133cd76e4c..ce9db8b861d04 100644 --- a/.gitignore +++ b/.gitignore @@ -46,8 +46,7 @@ no_llvm_build /inst/ /llvm/ /mingw-build/ -build/ -!/compiler/rustc_mir_build/src/build/ +/build /build-rust-analyzer/ /dist/ /unicode-downloads From 8e7734978245522cbbd14e53e08e888faf031ded Mon Sep 17 00:00:00 2001 From: Maybe Lapkin Date: Thu, 28 Nov 2024 21:54:27 +0100 Subject: [PATCH 15/18] fix a comment with uneven number of backticks in rustc_mir_build this is funny though! apparently tidy parsed `.gitignore`, but did not recognize unignore lines (`!...`), so tidy was ignoring `rustc_mir_build` this whole time (at least for some lints?). --- compiler/rustc_mir_build/src/build/expr/as_rvalue.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs index 3f89e337781e5..c66af118453e5 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -231,7 +231,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { if range.start <= range.end { BinOp::BitAnd } else { BinOp::BitOr }; let mut comparer = |range: u128, bin_op: BinOp| -> Place<'tcx> { - // We can use `ty::TypingEnv::fully_monomorphized()`` here + // We can use `ty::TypingEnv::fully_monomorphized()` here // as we only need it to compute the layout of a primitive. let range_val = Const::from_bits( this.tcx, From b36dcc1a382d1a05099b80a8fe59279656ffed12 Mon Sep 17 00:00:00 2001 From: Orion Gonzalez Date: Wed, 27 Nov 2024 15:06:40 +0100 Subject: [PATCH 16/18] Improve the diagnostic of fn item in variadic fn --- compiler/rustc_hir_typeck/messages.ftl | 5 +++++ compiler/rustc_hir_typeck/src/errors.rs | 10 ++++++++++ compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs | 12 +++++++++--- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_hir_typeck/messages.ftl b/compiler/rustc_hir_typeck/messages.ftl index 3669100ed9167..6001816ffbec6 100644 --- a/compiler/rustc_hir_typeck/messages.ftl +++ b/compiler/rustc_hir_typeck/messages.ftl @@ -79,6 +79,11 @@ hir_typeck_field_multiply_specified_in_initializer = .label = used more than once .previous_use_label = first use of `{$ident}` +hir_typeck_fn_item_to_variadic_function = can't pass a function item to a variadic function + .suggestion = use a function pointer instead + .help = a function item is zero-sized and needs to be cast into a function pointer to be used in FFI + .note = for more information on function items, visit https://doc.rust-lang.org/reference/types/function-item.html + hir_typeck_fru_expr = this expression does not end in a comma... hir_typeck_fru_expr2 = ... so this is interpreted as a `..` range expression, instead of functional record update syntax hir_typeck_fru_note = this expression may have been misinterpreted as a `..` range expression diff --git a/compiler/rustc_hir_typeck/src/errors.rs b/compiler/rustc_hir_typeck/src/errors.rs index 4f579b05d83b6..936f80c75fa45 100644 --- a/compiler/rustc_hir_typeck/src/errors.rs +++ b/compiler/rustc_hir_typeck/src/errors.rs @@ -797,3 +797,13 @@ pub(crate) struct PassToVariadicFunction<'a, 'tcx> { #[note(hir_typeck_teach_help)] pub(crate) teach: bool, } + +#[derive(Diagnostic)] +#[diag(hir_typeck_fn_item_to_variadic_function, code = E0617)] +pub(crate) struct PassFnItemToVariadicFunction { + #[primary_span] + pub span: Span, + #[suggestion(code = " as {replace}", applicability = "machine-applicable", style = "verbose")] + pub sugg_span: Span, + pub replace: String, +} diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index f8f6564cf14d9..63777f82f1ae3 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -472,9 +472,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { variadic_error(tcx.sess, arg.span, arg_ty, "c_uint"); } ty::FnDef(..) => { - let ptr_ty = Ty::new_fn_ptr(self.tcx, arg_ty.fn_sig(self.tcx)); - let ptr_ty = self.resolve_vars_if_possible(ptr_ty); - variadic_error(tcx.sess, arg.span, arg_ty, &ptr_ty.to_string()); + let fn_ptr = Ty::new_fn_ptr(self.tcx, arg_ty.fn_sig(self.tcx)); + let fn_ptr = self.resolve_vars_if_possible(fn_ptr).to_string(); + + let fn_item_spa = arg.span; + tcx.sess.dcx().emit_err(errors::PassFnItemToVariadicFunction { + span: fn_item_spa, + sugg_span: fn_item_spa.shrink_to_hi(), + replace: fn_ptr, + }); } _ => {} } From 1e4817cd33a59f1e0ddb1a3e66bdc0794d0785be Mon Sep 17 00:00:00 2001 From: Orion Gonzalez Date: Wed, 27 Nov 2024 15:37:27 +0100 Subject: [PATCH 17/18] bless the tests and add a new one --- compiler/rustc_hir_typeck/src/errors.rs | 2 ++ .../c-variadic/fn-item-diagnostic-issue-69232.rs | 13 +++++++++++++ .../fn-item-diagnostic-issue-69232.stderr | 16 ++++++++++++++++ tests/ui/c-variadic/issue-32201.rs | 5 +++-- tests/ui/c-variadic/issue-32201.stderr | 11 +++++++++-- tests/ui/error-codes/E0617.rs | 5 +++-- tests/ui/error-codes/E0617.stderr | 8 +++++--- 7 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 tests/ui/c-variadic/fn-item-diagnostic-issue-69232.rs create mode 100644 tests/ui/c-variadic/fn-item-diagnostic-issue-69232.stderr diff --git a/compiler/rustc_hir_typeck/src/errors.rs b/compiler/rustc_hir_typeck/src/errors.rs index 936f80c75fa45..fa27abd270ff2 100644 --- a/compiler/rustc_hir_typeck/src/errors.rs +++ b/compiler/rustc_hir_typeck/src/errors.rs @@ -800,6 +800,8 @@ pub(crate) struct PassToVariadicFunction<'a, 'tcx> { #[derive(Diagnostic)] #[diag(hir_typeck_fn_item_to_variadic_function, code = E0617)] +#[help] +#[note] pub(crate) struct PassFnItemToVariadicFunction { #[primary_span] pub span: Span, diff --git a/tests/ui/c-variadic/fn-item-diagnostic-issue-69232.rs b/tests/ui/c-variadic/fn-item-diagnostic-issue-69232.rs new file mode 100644 index 0000000000000..d0ef91b22b234 --- /dev/null +++ b/tests/ui/c-variadic/fn-item-diagnostic-issue-69232.rs @@ -0,0 +1,13 @@ +// https://github.com/rust-lang/rust/issues/69232 + +extern "C" { + fn foo(x: usize, ...); +} + +fn test() -> u8 { + 127 +} + +fn main() { + unsafe { foo(1, test) }; //~ ERROR can't pass a function item to a variadic function +} diff --git a/tests/ui/c-variadic/fn-item-diagnostic-issue-69232.stderr b/tests/ui/c-variadic/fn-item-diagnostic-issue-69232.stderr new file mode 100644 index 0000000000000..6aa1c8a109159 --- /dev/null +++ b/tests/ui/c-variadic/fn-item-diagnostic-issue-69232.stderr @@ -0,0 +1,16 @@ +error[E0617]: can't pass a function item to a variadic function + --> $DIR/fn-item-diagnostic-issue-69232.rs:12:21 + | +LL | unsafe { foo(1, test) }; + | ^^^^ + | + = help: a function item is zero-sized and needs to be cast into a function pointer to be used in FFI + = note: for more information on function items, visit https://doc.rust-lang.org/reference/types/function-item.html +help: use a function pointer instead + | +LL | unsafe { foo(1, test as fn() -> u8) }; + | +++++++++++++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0617`. diff --git a/tests/ui/c-variadic/issue-32201.rs b/tests/ui/c-variadic/issue-32201.rs index f27bb1c2eb5b6..434711b75236e 100644 --- a/tests/ui/c-variadic/issue-32201.rs +++ b/tests/ui/c-variadic/issue-32201.rs @@ -7,7 +7,8 @@ fn bar(_: *const u8) {} fn main() { unsafe { foo(0, bar); - //~^ ERROR can't pass `fn(*const u8) {bar}` to variadic function - //~| HELP cast the value to `fn(*const u8)` + //~^ ERROR can't pass a function item to a variadic function + //~| HELP a function item is zero-sized and needs to be cast into a function pointer to be used in FFI + ////~| HELP use a function pointer instead } } diff --git a/tests/ui/c-variadic/issue-32201.stderr b/tests/ui/c-variadic/issue-32201.stderr index 352db9f62f6d8..1cd85d7f07af3 100644 --- a/tests/ui/c-variadic/issue-32201.stderr +++ b/tests/ui/c-variadic/issue-32201.stderr @@ -1,8 +1,15 @@ -error[E0617]: can't pass `fn(*const u8) {bar}` to variadic function +error[E0617]: can't pass a function item to a variadic function --> $DIR/issue-32201.rs:9:16 | LL | foo(0, bar); - | ^^^ help: cast the value to `fn(*const u8)`: `bar as fn(*const u8)` + | ^^^ + | + = help: a function item is zero-sized and needs to be cast into a function pointer to be used in FFI + = note: for more information on function items, visit https://doc.rust-lang.org/reference/types/function-item.html +help: use a function pointer instead + | +LL | foo(0, bar as fn(*const u8)); + | ++++++++++++++++ error: aborting due to 1 previous error diff --git a/tests/ui/error-codes/E0617.rs b/tests/ui/error-codes/E0617.rs index b71ba0ed88b9f..4a38174bc6fef 100644 --- a/tests/ui/error-codes/E0617.rs +++ b/tests/ui/error-codes/E0617.rs @@ -20,7 +20,8 @@ fn main() { //~^ ERROR can't pass `u16` to variadic function //~| HELP cast the value to `c_uint` printf(::std::ptr::null(), printf); - //~^ ERROR can't pass `unsafe extern "C" fn(*const i8, ...) {printf}` to variadic function - //~| HELP cast the value to `unsafe extern "C" fn(*const i8, ...)` + //~^ ERROR can't pass a function item to a variadic function + //~| HELP a function item is zero-sized and needs to be cast into a function pointer to be used in FFI + //~| HELP use a function pointer instead } } diff --git a/tests/ui/error-codes/E0617.stderr b/tests/ui/error-codes/E0617.stderr index ea91ad0829230..7193463e02831 100644 --- a/tests/ui/error-codes/E0617.stderr +++ b/tests/ui/error-codes/E0617.stderr @@ -28,16 +28,18 @@ error[E0617]: can't pass `u16` to variadic function LL | printf(::std::ptr::null(), 0u16); | ^^^^ help: cast the value to `c_uint`: `0u16 as c_uint` -error[E0617]: can't pass `unsafe extern "C" fn(*const i8, ...) {printf}` to variadic function +error[E0617]: can't pass a function item to a variadic function --> $DIR/E0617.rs:22:36 | LL | printf(::std::ptr::null(), printf); | ^^^^^^ | -help: cast the value to `unsafe extern "C" fn(*const i8, ...)` + = help: a function item is zero-sized and needs to be cast into a function pointer to be used in FFI + = note: for more information on function items, visit https://doc.rust-lang.org/reference/types/function-item.html +help: use a function pointer instead | LL | printf(::std::ptr::null(), printf as unsafe extern "C" fn(*const i8, ...)); - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + | +++++++++++++++++++++++++++++++++++++++ error: aborting due to 6 previous errors From 9461f4296ffcbd75440ec211ead7bff6fb86012c Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 29 Nov 2024 14:52:41 +1100 Subject: [PATCH 18/18] Revert "Rollup merge of #133418 - Zalathar:spans, r=jieyouxu" This reverts commit adf9b5fcd1de43eaf0a779e10612caee8b47bede, reversing changes made to af1ca153d4aed5ffe22445273aa388a8d3f8f4ae. Reverting due to . --- .../src/coverageinfo/ffi.rs | 36 ++-- .../src/coverageinfo/map_data.rs | 14 +- .../src/coverageinfo/mapgen.rs | 129 +++++++------- .../src/coverageinfo/mapgen/spans.rs | 124 ------------- compiler/rustc_codegen_llvm/src/lib.rs | 1 - compiler/rustc_middle/src/mir/coverage.rs | 18 +- compiler/rustc_middle/src/mir/pretty.rs | 4 +- .../rustc_mir_transform/src/coverage/mod.rs | 166 ++++++++++++++++-- ...ch_match_arms.main.InstrumentCoverage.diff | 12 +- ...ument_coverage.bar.InstrumentCoverage.diff | 2 +- ...ment_coverage.main.InstrumentCoverage.diff | 10 +- ...rage_cleanup.main.CleanupPostBorrowck.diff | 10 +- ...erage_cleanup.main.InstrumentCoverage.diff | 10 +- 13 files changed, 285 insertions(+), 251 deletions(-) delete mode 100644 compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index 19d6726002c41..a6e07ea2a60ec 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -1,4 +1,6 @@ -use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId}; +use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId, SourceRegion}; + +use crate::coverageinfo::mapgen::LocalFileId; /// Must match the layout of `LLVMRustCounterKind`. #[derive(Copy, Clone, Debug)] @@ -124,23 +126,37 @@ pub(crate) struct CoverageSpan { /// Local index into the function's local-to-global file ID table. /// The value at that index is itself an index into the coverage filename /// table in the CGU's `__llvm_covmap` section. - pub(crate) file_id: u32, + file_id: u32, /// 1-based starting line of the source code span. - pub(crate) start_line: u32, + start_line: u32, /// 1-based starting column of the source code span. - pub(crate) start_col: u32, + start_col: u32, /// 1-based ending line of the source code span. - pub(crate) end_line: u32, + end_line: u32, /// 1-based ending column of the source code span. High bit must be unset. - pub(crate) end_col: u32, + end_col: u32, +} + +impl CoverageSpan { + pub(crate) fn from_source_region( + local_file_id: LocalFileId, + code_region: &SourceRegion, + ) -> Self { + let file_id = local_file_id.as_u32(); + let &SourceRegion { start_line, start_col, end_line, end_col } = code_region; + // Internally, LLVM uses the high bit of `end_col` to distinguish between + // code regions and gap regions, so it can't be used by the column number. + assert!(end_col & (1u32 << 31) == 0, "high bit of `end_col` must be unset: {end_col:#X}"); + Self { file_id, start_line, start_col, end_line, end_col } + } } /// Must match the layout of `LLVMRustCoverageCodeRegion`. #[derive(Clone, Debug)] #[repr(C)] pub(crate) struct CodeRegion { - pub(crate) cov_span: CoverageSpan, + pub(crate) span: CoverageSpan, pub(crate) counter: Counter, } @@ -148,7 +164,7 @@ pub(crate) struct CodeRegion { #[derive(Clone, Debug)] #[repr(C)] pub(crate) struct BranchRegion { - pub(crate) cov_span: CoverageSpan, + pub(crate) span: CoverageSpan, pub(crate) true_counter: Counter, pub(crate) false_counter: Counter, } @@ -157,7 +173,7 @@ pub(crate) struct BranchRegion { #[derive(Clone, Debug)] #[repr(C)] pub(crate) struct MCDCBranchRegion { - pub(crate) cov_span: CoverageSpan, + pub(crate) span: CoverageSpan, pub(crate) true_counter: Counter, pub(crate) false_counter: Counter, pub(crate) mcdc_branch_params: mcdc::BranchParameters, @@ -167,6 +183,6 @@ pub(crate) struct MCDCBranchRegion { #[derive(Clone, Debug)] #[repr(C)] pub(crate) struct MCDCDecisionRegion { - pub(crate) cov_span: CoverageSpan, + pub(crate) span: CoverageSpan, pub(crate) mcdc_decision_params: mcdc::DecisionParameters, } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs index 95746b88cedbf..e3c0df278836e 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs @@ -3,9 +3,9 @@ use rustc_data_structures::fx::FxIndexSet; use rustc_index::bit_set::BitSet; use rustc_middle::mir::coverage::{ CounterId, CovTerm, Expression, ExpressionId, FunctionCoverageInfo, Mapping, MappingKind, Op, + SourceRegion, }; use rustc_middle::ty::Instance; -use rustc_span::Span; use tracing::{debug, instrument}; use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind}; @@ -220,16 +220,16 @@ impl<'tcx> FunctionCoverage<'tcx> { }) } - /// Yields all this function's coverage mappings, after simplifying away - /// unused counters and counter expressions. - pub(crate) fn mapping_spans( + /// Converts this function's coverage mappings into an intermediate form + /// that will be used by `mapgen` when preparing for FFI. + pub(crate) fn counter_regions( &self, - ) -> impl Iterator + ExactSizeIterator + Captures<'_> { + ) -> impl Iterator + ExactSizeIterator { self.function_coverage_info.mappings.iter().map(move |mapping| { - let &Mapping { ref kind, span } = mapping; + let Mapping { kind, source_region } = mapping; let kind = kind.map_terms(|term| if self.is_zero_term(term) { CovTerm::Zero } else { term }); - (kind, span) + (kind, source_region) }) } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index ed881418cb003..b582dd967a706 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -1,14 +1,12 @@ -mod spans; - use std::ffi::CString; -use std::sync::Arc; +use std::iter; use itertools::Itertools as _; use rustc_abi::Align; use rustc_codegen_ssa::traits::{ BaseTypeCodegenMethods, ConstCodegenMethods, StaticCodegenMethods, }; -use rustc_data_structures::fx::{FxHashSet, FxIndexMap}; +use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_index::IndexVec; use rustc_middle::mir::coverage::MappingKind; @@ -17,7 +15,7 @@ use rustc_middle::{bug, mir}; use rustc_session::RemapFileNameExt; use rustc_session::config::RemapPathScopeComponents; use rustc_span::def_id::DefIdSet; -use rustc_span::{SourceFile, StableSourceFileId}; +use rustc_span::{Span, Symbol}; use rustc_target::spec::HasTargetSpec; use tracing::debug; @@ -74,11 +72,11 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { .map(|(instance, function_coverage)| (instance, function_coverage.into_finished())) .collect::>(); - let all_files = function_coverage_entries + let all_file_names = function_coverage_entries .iter() .map(|(_, fn_cov)| fn_cov.function_coverage_info.body_span) - .map(|span| tcx.sess.source_map().lookup_source_file(span.lo())); - let global_file_table = GlobalFileTable::new(all_files); + .map(|span| span_file_name(tcx, span)); + let global_file_table = GlobalFileTable::new(all_file_names); // Encode all filenames referenced by coverage mappings in this CGU. let filenames_buffer = global_file_table.make_filenames_buffer(tcx); @@ -105,8 +103,15 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { encode_mappings_for_function(tcx, &global_file_table, &function_coverage); if coverage_mapping_buffer.is_empty() { - debug!("function has no mappings to embed; skipping"); - continue; + if function_coverage.is_used() { + bug!( + "A used function should have had coverage mapping data but did not: {}", + mangled_function_name + ); + } else { + debug!("unused function had no coverage mapping data: {}", mangled_function_name); + continue; + } } if !is_used { @@ -143,34 +148,29 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { } } -/// Maps "global" (per-CGU) file ID numbers to their underlying source files. +/// Maps "global" (per-CGU) file ID numbers to their underlying filenames. struct GlobalFileTable { - /// This "raw" table doesn't include the working dir, so a file's + /// This "raw" table doesn't include the working dir, so a filename's /// global ID is its index in this set **plus one**. - raw_file_table: FxIndexMap>, + raw_file_table: FxIndexSet, } impl GlobalFileTable { - fn new(all_files: impl IntoIterator>) -> Self { - // Collect all of the files into a set. Files usually come in contiguous - // runs, so we can dedup adjacent ones to save work. - let mut raw_file_table = all_files - .into_iter() - .dedup_by(|a, b| a.stable_id == b.stable_id) - .map(|f| (f.stable_id, f)) - .collect::>>(); + fn new(all_file_names: impl IntoIterator) -> Self { + // Collect all of the filenames into a set. Filenames usually come in + // contiguous runs, so we can dedup adjacent ones to save work. + let mut raw_file_table = all_file_names.into_iter().dedup().collect::>(); - // Sort the file table by its underlying filenames. - raw_file_table.sort_unstable_by(|_, a, _, b| { - Ord::cmp(&a.name, &b.name).then_with(|| Ord::cmp(&a.stable_id, &b.stable_id)) - }); + // Sort the file table by its actual string values, not the arbitrary + // ordering of its symbols. + raw_file_table.sort_unstable_by(|a, b| a.as_str().cmp(b.as_str())); Self { raw_file_table } } - fn global_file_id_for_file(&self, file: &SourceFile) -> GlobalFileId { - let raw_id = self.raw_file_table.get_index_of(&file.stable_id).unwrap_or_else(|| { - bug!("file not found in prepared global file table: {:?}", file.name); + fn global_file_id_for_file_name(&self, file_name: Symbol) -> GlobalFileId { + let raw_id = self.raw_file_table.get_index_of(&file_name).unwrap_or_else(|| { + bug!("file name not found in prepared global file table: {file_name}"); }); // The raw file table doesn't include an entry for the working dir // (which has ID 0), so add 1 to get the correct ID. @@ -178,27 +178,24 @@ impl GlobalFileTable { } fn make_filenames_buffer(&self, tcx: TyCtxt<'_>) -> Vec { - let mut table = Vec::with_capacity(self.raw_file_table.len() + 1); - // LLVM Coverage Mapping Format version 6 (zero-based encoded as 5) // requires setting the first filename to the compilation directory. // Since rustc generates coverage maps with relative paths, the // compilation directory can be combined with the relative paths // to get absolute paths, if needed. - table.push( - tcx.sess - .opts - .working_dir - .for_scope(tcx.sess, RemapPathScopeComponents::MACRO) - .to_string_lossy(), - ); - - // Add the regular entries after the base directory. - table.extend(self.raw_file_table.values().map(|file| { - file.name.for_scope(tcx.sess, RemapPathScopeComponents::MACRO).to_string_lossy() - })); - - llvm_cov::write_filenames_to_buffer(table.iter().map(|f| f.as_ref())) + use rustc_session::RemapFileNameExt; + use rustc_session::config::RemapPathScopeComponents; + let working_dir: &str = &tcx + .sess + .opts + .working_dir + .for_scope(tcx.sess, RemapPathScopeComponents::MACRO) + .to_string_lossy(); + + // Insert the working dir at index 0, before the other filenames. + let filenames = + iter::once(working_dir).chain(self.raw_file_table.iter().map(Symbol::as_str)); + llvm_cov::write_filenames_to_buffer(filenames) } } @@ -211,7 +208,7 @@ rustc_index::newtype_index! { /// An index into a function's list of global file IDs. That underlying list /// of local-to-global mappings will be embedded in the function's record in /// the `__llvm_covfun` linker section. - struct LocalFileId {} + pub(crate) struct LocalFileId {} } /// Holds a mapping from "local" (per-function) file IDs to "global" (per-CGU) @@ -237,6 +234,13 @@ impl VirtualFileMapping { } } +fn span_file_name(tcx: TyCtxt<'_>, span: Span) -> Symbol { + let source_file = tcx.sess.source_map().lookup_source_file(span.lo()); + let name = + source_file.name.for_scope(tcx.sess, RemapPathScopeComponents::MACRO).to_string_lossy(); + Symbol::intern(&name) +} + /// Using the expressions and counter regions collected for a single function, /// generate the variable-sized payload of its corresponding `__llvm_covfun` /// entry. The payload is returned as a vector of bytes. @@ -247,13 +251,11 @@ fn encode_mappings_for_function( global_file_table: &GlobalFileTable, function_coverage: &FunctionCoverage<'_>, ) -> Vec { - let mapping_spans = function_coverage.mapping_spans(); - if mapping_spans.is_empty() { + let counter_regions = function_coverage.counter_regions(); + if counter_regions.is_empty() { return Vec::new(); } - let fn_cov_info = function_coverage.function_coverage_info; - let expressions = function_coverage.counter_expressions().collect::>(); let mut virtual_file_mapping = VirtualFileMapping::default(); @@ -263,39 +265,34 @@ fn encode_mappings_for_function( let mut mcdc_decision_regions = vec![]; // Currently a function's mappings must all be in the same file as its body span. - let source_map = tcx.sess.source_map(); - let source_file = source_map.lookup_source_file(fn_cov_info.body_span.lo()); + let file_name = span_file_name(tcx, function_coverage.function_coverage_info.body_span); - // Look up the global file ID for that file. - let global_file_id = global_file_table.global_file_id_for_file(&source_file); + // Look up the global file ID for that filename. + let global_file_id = global_file_table.global_file_id_for_file_name(file_name); // Associate that global file ID with a local file ID for this function. let local_file_id = virtual_file_mapping.local_id_for_global(global_file_id); + debug!(" file id: {local_file_id:?} => {global_file_id:?} = '{file_name:?}'"); - let make_cov_span = |span| { - spans::make_coverage_span(local_file_id, source_map, fn_cov_info, &source_file, span) - }; - - // For each coverage mapping span in this function+file, convert it to a + // For each counter/region pair in this function+file, convert it to a // form suitable for FFI. - for (mapping_kind, span) in mapping_spans { - debug!("Adding counter {mapping_kind:?} to map for {span:?}"); - let Some(cov_span) = make_cov_span(span) else { continue }; + for (mapping_kind, region) in counter_regions { + debug!("Adding counter {mapping_kind:?} to map for {region:?}"); + let span = ffi::CoverageSpan::from_source_region(local_file_id, region); match mapping_kind { MappingKind::Code(term) => { - code_regions - .push(ffi::CodeRegion { cov_span, counter: ffi::Counter::from_term(term) }); + code_regions.push(ffi::CodeRegion { span, counter: ffi::Counter::from_term(term) }); } MappingKind::Branch { true_term, false_term } => { branch_regions.push(ffi::BranchRegion { - cov_span, + span, true_counter: ffi::Counter::from_term(true_term), false_counter: ffi::Counter::from_term(false_term), }); } MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => { mcdc_branch_regions.push(ffi::MCDCBranchRegion { - cov_span, + span, true_counter: ffi::Counter::from_term(true_term), false_counter: ffi::Counter::from_term(false_term), mcdc_branch_params: ffi::mcdc::BranchParameters::from(mcdc_params), @@ -303,7 +300,7 @@ fn encode_mappings_for_function( } MappingKind::MCDCDecision(mcdc_decision_params) => { mcdc_decision_regions.push(ffi::MCDCDecisionRegion { - cov_span, + span, mcdc_decision_params: ffi::mcdc::DecisionParameters::from(mcdc_decision_params), }); } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs deleted file mode 100644 index 4a7721879fd0d..0000000000000 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs +++ /dev/null @@ -1,124 +0,0 @@ -use rustc_middle::mir::coverage::FunctionCoverageInfo; -use rustc_span::source_map::SourceMap; -use rustc_span::{BytePos, Pos, SourceFile, Span}; -use tracing::debug; - -use crate::coverageinfo::ffi; -use crate::coverageinfo::mapgen::LocalFileId; - -/// Converts the span into its start line and column, and end line and column. -/// -/// Line numbers and column numbers are 1-based. Unlike most column numbers emitted by -/// the compiler, these column numbers are denoted in **bytes**, because that's what -/// LLVM's `llvm-cov` tool expects to see in coverage maps. -/// -/// Returns `None` if the conversion failed for some reason. This shouldn't happen, -/// but it's hard to rule out entirely (especially in the presence of complex macros -/// or other expansions), and if it does happen then skipping a span or function is -/// better than an ICE or `llvm-cov` failure that the user might have no way to avoid. -pub(crate) fn make_coverage_span( - file_id: LocalFileId, - source_map: &SourceMap, - fn_cov_info: &FunctionCoverageInfo, - file: &SourceFile, - span: Span, -) -> Option { - let span = ensure_non_empty_span(source_map, fn_cov_info, span)?; - - let lo = span.lo(); - let hi = span.hi(); - - // Column numbers need to be in bytes, so we can't use the more convenient - // `SourceMap` methods for looking up file coordinates. - let line_and_byte_column = |pos: BytePos| -> Option<(usize, usize)> { - let rpos = file.relative_position(pos); - let line_index = file.lookup_line(rpos)?; - let line_start = file.lines()[line_index]; - // Line numbers and column numbers are 1-based, so add 1 to each. - Some((line_index + 1, (rpos - line_start).to_usize() + 1)) - }; - - let (mut start_line, start_col) = line_and_byte_column(lo)?; - let (mut end_line, end_col) = line_and_byte_column(hi)?; - - // Apply an offset so that code in doctests has correct line numbers. - // FIXME(#79417): Currently we have no way to offset doctest _columns_. - start_line = source_map.doctest_offset_line(&file.name, start_line); - end_line = source_map.doctest_offset_line(&file.name, end_line); - - check_coverage_span(ffi::CoverageSpan { - file_id: file_id.as_u32(), - start_line: start_line as u32, - start_col: start_col as u32, - end_line: end_line as u32, - end_col: end_col as u32, - }) -} - -fn ensure_non_empty_span( - source_map: &SourceMap, - fn_cov_info: &FunctionCoverageInfo, - span: Span, -) -> Option { - if !span.is_empty() { - return Some(span); - } - - let lo = span.lo(); - let hi = span.hi(); - - // The span is empty, so try to expand it to cover an adjacent '{' or '}', - // but only within the bounds of the body span. - let try_next = hi < fn_cov_info.body_span.hi(); - let try_prev = fn_cov_info.body_span.lo() < lo; - if !(try_next || try_prev) { - return None; - } - - source_map - .span_to_source(span, |src, start, end| try { - // We're only checking for specific ASCII characters, so we don't - // have to worry about multi-byte code points. - if try_next && src.as_bytes()[end] == b'{' { - Some(span.with_hi(hi + BytePos(1))) - } else if try_prev && src.as_bytes()[start - 1] == b'}' { - Some(span.with_lo(lo - BytePos(1))) - } else { - None - } - }) - .ok()? -} - -/// If `llvm-cov` sees a source region that is improperly ordered (end < start), -/// it will immediately exit with a fatal error. To prevent that from happening, -/// discard regions that are improperly ordered, or might be interpreted in a -/// way that makes them improperly ordered. -fn check_coverage_span(cov_span: ffi::CoverageSpan) -> Option { - let ffi::CoverageSpan { file_id: _, start_line, start_col, end_line, end_col } = cov_span; - - // Line/column coordinates are supposed to be 1-based. If we ever emit - // coordinates of 0, `llvm-cov` might misinterpret them. - let all_nonzero = [start_line, start_col, end_line, end_col].into_iter().all(|x| x != 0); - // Coverage mappings use the high bit of `end_col` to indicate that a - // region is actually a "gap" region, so make sure it's unset. - let end_col_has_high_bit_unset = (end_col & (1 << 31)) == 0; - // If a region is improperly ordered (end < start), `llvm-cov` will exit - // with a fatal error, which is inconvenient for users and hard to debug. - let is_ordered = (start_line, start_col) <= (end_line, end_col); - - if all_nonzero && end_col_has_high_bit_unset && is_ordered { - Some(cov_span) - } else { - debug!( - ?cov_span, - ?all_nonzero, - ?end_col_has_high_bit_unset, - ?is_ordered, - "Skipping source region that would be misinterpreted or rejected by LLVM" - ); - // If this happens in a debug build, ICE to make it easier to notice. - debug_assert!(false, "Improper source region: {cov_span:?}"); - None - } -} diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 9f398107fc610..3dfb86d422dd2 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -17,7 +17,6 @@ #![feature(iter_intersperse)] #![feature(let_chains)] #![feature(rustdoc_internals)] -#![feature(try_blocks)] #![warn(unreachable_pub)] // tidy-alphabetical-end diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 33f9a2bdca6b4..11a4e7f89a7c7 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -155,6 +155,22 @@ impl Debug for CoverageKind { } } +#[derive(Clone, TyEncodable, TyDecodable, Hash, HashStable, PartialEq, Eq, PartialOrd, Ord)] +#[derive(TypeFoldable, TypeVisitable)] +pub struct SourceRegion { + pub start_line: u32, + pub start_col: u32, + pub end_line: u32, + pub end_col: u32, +} + +impl Debug for SourceRegion { + fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { + let &Self { start_line, start_col, end_line, end_col } = self; + write!(fmt, "{start_line}:{start_col} - {end_line}:{end_col}") + } +} + #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable)] #[derive(TyEncodable, TyDecodable, TypeFoldable, TypeVisitable)] pub enum Op { @@ -216,7 +232,7 @@ impl MappingKind { #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub struct Mapping { pub kind: MappingKind, - pub span: Span, + pub source_region: SourceRegion, } /// Stores per-function coverage information attached to a `mir::Body`, diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index ece468947c25d..2bfcd0a62274d 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -603,8 +603,8 @@ fn write_function_coverage_info( for (id, expression) in expressions.iter_enumerated() { writeln!(w, "{INDENT}coverage {id:?} => {expression:?};")?; } - for coverage::Mapping { kind, span } in mappings { - writeln!(w, "{INDENT}coverage {kind:?} => {span:?};")?; + for coverage::Mapping { kind, source_region } in mappings { + writeln!(w, "{INDENT}coverage {kind:?} => {source_region:?};")?; } writeln!(w)?; diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 02fd289b6ef6d..e8b3d80be0212 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -13,15 +13,16 @@ use rustc_hir::intravisit::{Visitor, walk_expr}; use rustc_middle::hir::map::Map; use rustc_middle::hir::nested_filter; use rustc_middle::mir::coverage::{ - CoverageKind, DecisionInfo, FunctionCoverageInfo, Mapping, MappingKind, + CoverageKind, DecisionInfo, FunctionCoverageInfo, Mapping, MappingKind, SourceRegion, }; use rustc_middle::mir::{ self, BasicBlock, BasicBlockData, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, }; use rustc_middle::ty::TyCtxt; -use rustc_span::Span; use rustc_span::def_id::LocalDefId; +use rustc_span::source_map::SourceMap; +use rustc_span::{BytePos, Pos, SourceFile, Span}; use tracing::{debug, debug_span, trace}; use crate::coverage::counters::{CounterIncrementSite, CoverageCounters}; @@ -96,7 +97,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: let coverage_counters = CoverageCounters::make_bcb_counters(&basic_coverage_blocks, &bcbs_with_counter_mappings); - let mappings = create_mappings(&extracted_mappings, &coverage_counters); + let mappings = create_mappings(tcx, &hir_info, &extracted_mappings, &coverage_counters); if mappings.is_empty() { // No spans could be converted into valid mappings, so skip this function. debug!("no spans could be converted into valid mappings; skipping"); @@ -135,12 +136,18 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: /// /// Precondition: All BCBs corresponding to those spans have been given /// coverage counters. -fn create_mappings( +fn create_mappings<'tcx>( + tcx: TyCtxt<'tcx>, + hir_info: &ExtractedHirInfo, extracted_mappings: &ExtractedMappings, coverage_counters: &CoverageCounters, ) -> Vec { + let source_map = tcx.sess.source_map(); + let file = source_map.lookup_source_file(hir_info.body_span.lo()); + let term_for_bcb = |bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters"); + let region_for_span = |span: Span| make_source_region(source_map, hir_info, &file, span); // Fully destructure the mappings struct to make sure we don't miss any kinds. let ExtractedMappings { @@ -153,20 +160,22 @@ fn create_mappings( } = extracted_mappings; let mut mappings = Vec::new(); - mappings.extend(code_mappings.iter().map( + mappings.extend(code_mappings.iter().filter_map( // Ordinary code mappings are the simplest kind. |&mappings::CodeMapping { span, bcb }| { + let source_region = region_for_span(span)?; let kind = MappingKind::Code(term_for_bcb(bcb)); - Mapping { kind, span } + Some(Mapping { kind, source_region }) }, )); - mappings.extend(branch_pairs.iter().map( + mappings.extend(branch_pairs.iter().filter_map( |&mappings::BranchPair { span, true_bcb, false_bcb }| { let true_term = term_for_bcb(true_bcb); let false_term = term_for_bcb(false_bcb); let kind = MappingKind::Branch { true_term, false_term }; - Mapping { kind, span } + let source_region = region_for_span(span)?; + Some(Mapping { kind, source_region }) }, )); @@ -174,7 +183,7 @@ fn create_mappings( |bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters"); // MCDC branch mappings are appended with their decisions in case decisions were ignored. - mappings.extend(mcdc_degraded_branches.iter().map( + mappings.extend(mcdc_degraded_branches.iter().filter_map( |&mappings::MCDCBranch { span, true_bcb, @@ -183,9 +192,10 @@ fn create_mappings( true_index: _, false_index: _, }| { + let source_region = region_for_span(span)?; let true_term = term_for_bcb(true_bcb); let false_term = term_for_bcb(false_bcb); - Mapping { kind: MappingKind::Branch { true_term, false_term }, span } + Some(Mapping { kind: MappingKind::Branch { true_term, false_term }, source_region }) }, )); @@ -193,7 +203,7 @@ fn create_mappings( let num_conditions = branches.len() as u16; let conditions = branches .into_iter() - .map( + .filter_map( |&mappings::MCDCBranch { span, true_bcb, @@ -202,29 +212,31 @@ fn create_mappings( true_index: _, false_index: _, }| { + let source_region = region_for_span(span)?; let true_term = term_for_bcb(true_bcb); let false_term = term_for_bcb(false_bcb); - Mapping { + Some(Mapping { kind: MappingKind::MCDCBranch { true_term, false_term, mcdc_params: condition_info, }, - span, - } + source_region, + }) }, ) .collect::>(); - if conditions.len() == num_conditions as usize { + if conditions.len() == num_conditions as usize + && let Some(source_region) = region_for_span(decision.span) + { // LLVM requires end index for counter mapping regions. let kind = MappingKind::MCDCDecision(DecisionInfo { bitmap_idx: (decision.bitmap_idx + decision.num_test_vectors) as u32, num_conditions, }); mappings.extend( - std::iter::once(Mapping { kind, span: decision.span }) - .chain(conditions.into_iter()), + std::iter::once(Mapping { kind, source_region }).chain(conditions.into_iter()), ); } else { mappings.extend(conditions.into_iter().map(|mapping| { @@ -233,7 +245,10 @@ fn create_mappings( else { unreachable!("all mappings here are MCDCBranch as shown above"); }; - Mapping { kind: MappingKind::Branch { true_term, false_term }, span: mapping.span } + Mapping { + kind: MappingKind::Branch { true_term, false_term }, + source_region: mapping.source_region, + } })) } } @@ -376,6 +391,121 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb data.statements.insert(0, statement); } +fn ensure_non_empty_span( + source_map: &SourceMap, + hir_info: &ExtractedHirInfo, + span: Span, +) -> Option { + if !span.is_empty() { + return Some(span); + } + + let lo = span.lo(); + let hi = span.hi(); + + // The span is empty, so try to expand it to cover an adjacent '{' or '}', + // but only within the bounds of the body span. + let try_next = hi < hir_info.body_span.hi(); + let try_prev = hir_info.body_span.lo() < lo; + if !(try_next || try_prev) { + return None; + } + + source_map + .span_to_source(span, |src, start, end| try { + // We're only checking for specific ASCII characters, so we don't + // have to worry about multi-byte code points. + if try_next && src.as_bytes()[end] == b'{' { + Some(span.with_hi(hi + BytePos(1))) + } else if try_prev && src.as_bytes()[start - 1] == b'}' { + Some(span.with_lo(lo - BytePos(1))) + } else { + None + } + }) + .ok()? +} + +/// Converts the span into its start line and column, and end line and column. +/// +/// Line numbers and column numbers are 1-based. Unlike most column numbers emitted by +/// the compiler, these column numbers are denoted in **bytes**, because that's what +/// LLVM's `llvm-cov` tool expects to see in coverage maps. +/// +/// Returns `None` if the conversion failed for some reason. This shouldn't happen, +/// but it's hard to rule out entirely (especially in the presence of complex macros +/// or other expansions), and if it does happen then skipping a span or function is +/// better than an ICE or `llvm-cov` failure that the user might have no way to avoid. +fn make_source_region( + source_map: &SourceMap, + hir_info: &ExtractedHirInfo, + file: &SourceFile, + span: Span, +) -> Option { + let span = ensure_non_empty_span(source_map, hir_info, span)?; + + let lo = span.lo(); + let hi = span.hi(); + + // Column numbers need to be in bytes, so we can't use the more convenient + // `SourceMap` methods for looking up file coordinates. + let line_and_byte_column = |pos: BytePos| -> Option<(usize, usize)> { + let rpos = file.relative_position(pos); + let line_index = file.lookup_line(rpos)?; + let line_start = file.lines()[line_index]; + // Line numbers and column numbers are 1-based, so add 1 to each. + Some((line_index + 1, (rpos - line_start).to_usize() + 1)) + }; + + let (mut start_line, start_col) = line_and_byte_column(lo)?; + let (mut end_line, end_col) = line_and_byte_column(hi)?; + + // Apply an offset so that code in doctests has correct line numbers. + // FIXME(#79417): Currently we have no way to offset doctest _columns_. + start_line = source_map.doctest_offset_line(&file.name, start_line); + end_line = source_map.doctest_offset_line(&file.name, end_line); + + check_source_region(SourceRegion { + start_line: start_line as u32, + start_col: start_col as u32, + end_line: end_line as u32, + end_col: end_col as u32, + }) +} + +/// If `llvm-cov` sees a source region that is improperly ordered (end < start), +/// it will immediately exit with a fatal error. To prevent that from happening, +/// discard regions that are improperly ordered, or might be interpreted in a +/// way that makes them improperly ordered. +fn check_source_region(source_region: SourceRegion) -> Option { + let SourceRegion { start_line, start_col, end_line, end_col } = source_region; + + // Line/column coordinates are supposed to be 1-based. If we ever emit + // coordinates of 0, `llvm-cov` might misinterpret them. + let all_nonzero = [start_line, start_col, end_line, end_col].into_iter().all(|x| x != 0); + // Coverage mappings use the high bit of `end_col` to indicate that a + // region is actually a "gap" region, so make sure it's unset. + let end_col_has_high_bit_unset = (end_col & (1 << 31)) == 0; + // If a region is improperly ordered (end < start), `llvm-cov` will exit + // with a fatal error, which is inconvenient for users and hard to debug. + let is_ordered = (start_line, start_col) <= (end_line, end_col); + + if all_nonzero && end_col_has_high_bit_unset && is_ordered { + Some(source_region) + } else { + debug!( + ?source_region, + ?all_nonzero, + ?end_col_has_high_bit_unset, + ?is_ordered, + "Skipping source region that would be misinterpreted or rejected by LLVM" + ); + // If this happens in a debug build, ICE to make it easier to notice. + debug_assert!(false, "Improper source region: {source_region:?}"); + None + } +} + /// Function information extracted from HIR by the coverage instrumentor. #[derive(Debug)] struct ExtractedHirInfo { diff --git a/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff index 39d52ba698a43..cbb11d50f7974 100644 --- a/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff @@ -33,12 +33,12 @@ + coverage ExpressionId(3) => Expression { lhs: Counter(3), op: Add, rhs: Counter(2) }; + coverage ExpressionId(4) => Expression { lhs: Expression(3), op: Add, rhs: Counter(1) }; + coverage ExpressionId(5) => Expression { lhs: Expression(4), op: Add, rhs: Expression(2) }; -+ coverage Code(Counter(0)) => $DIR/branch_match_arms.rs:14:1: 15:21 (#0); -+ coverage Code(Counter(3)) => $DIR/branch_match_arms.rs:16:17: 16:33 (#0); -+ coverage Code(Counter(2)) => $DIR/branch_match_arms.rs:17:17: 17:33 (#0); -+ coverage Code(Counter(1)) => $DIR/branch_match_arms.rs:18:17: 18:33 (#0); -+ coverage Code(Expression(2)) => $DIR/branch_match_arms.rs:19:17: 19:33 (#0); -+ coverage Code(Expression(5)) => $DIR/branch_match_arms.rs:21:2: 21:2 (#0); ++ coverage Code(Counter(0)) => 14:1 - 15:21; ++ coverage Code(Counter(3)) => 16:17 - 16:33; ++ coverage Code(Counter(2)) => 17:17 - 17:33; ++ coverage Code(Counter(1)) => 18:17 - 18:33; ++ coverage Code(Expression(2)) => 19:17 - 19:33; ++ coverage Code(Expression(5)) => 21:1 - 21:2; + bb0: { + Coverage::CounterIncrement(0); diff --git a/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff index 148ff86354b57..2efb1fd0a17a3 100644 --- a/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff @@ -5,7 +5,7 @@ let mut _0: bool; + coverage body span: $DIR/instrument_coverage.rs:19:18: 21:2 (#0) -+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:19:1: 21:2 (#0); ++ coverage Code(Counter(0)) => 19:1 - 21:2; + bb0: { + Coverage::CounterIncrement(0); diff --git a/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff index b480d1ac13a76..a179824d6c736 100644 --- a/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff @@ -9,11 +9,11 @@ + coverage body span: $DIR/instrument_coverage.rs:10:11: 16:2 (#0) + coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Add, rhs: Counter(1) }; -+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:10:1: 10:11 (#0); -+ coverage Code(Expression(0)) => $DIR/instrument_coverage.rs:12:12: 12:17 (#0); -+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:13:13: 13:18 (#0); -+ coverage Code(Counter(1)) => $DIR/instrument_coverage.rs:14:10: 14:10 (#0); -+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:16:2: 16:2 (#0); ++ coverage Code(Counter(0)) => 10:1 - 10:11; ++ coverage Code(Expression(0)) => 12:12 - 12:17; ++ coverage Code(Counter(0)) => 13:13 - 13:18; ++ coverage Code(Counter(1)) => 14:9 - 14:10; ++ coverage Code(Counter(0)) => 16:1 - 16:2; + bb0: { + Coverage::CounterIncrement(0); diff --git a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff index 2c7ec6e85eb0a..082539369f707 100644 --- a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff +++ b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff @@ -9,11 +9,11 @@ coverage body span: $DIR/instrument_coverage_cleanup.rs:13:11: 15:2 (#0) coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Subtract, rhs: Counter(1) }; - coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:13:1: 14:36 (#0); - coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0); - coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0); - coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0); - coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0); + coverage Code(Counter(0)) => 13:1 - 14:36; + coverage Code(Expression(0)) => 14:37 - 14:39; + coverage Code(Counter(1)) => 14:38 - 14:39; + coverage Code(Counter(0)) => 15:1 - 15:2; + coverage Branch { true_term: Expression(0), false_term: Counter(1) } => 14:8 - 14:36; bb0: { Coverage::CounterIncrement(0); diff --git a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff index c08265eb0e953..8635818c6a7a1 100644 --- a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff @@ -9,11 +9,11 @@ + coverage body span: $DIR/instrument_coverage_cleanup.rs:13:11: 15:2 (#0) + coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Subtract, rhs: Counter(1) }; -+ coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:13:1: 14:36 (#0); -+ coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0); -+ coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0); -+ coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0); -+ coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0); ++ coverage Code(Counter(0)) => 13:1 - 14:36; ++ coverage Code(Expression(0)) => 14:37 - 14:39; ++ coverage Code(Counter(1)) => 14:38 - 14:39; ++ coverage Code(Counter(0)) => 15:1 - 15:2; ++ coverage Branch { true_term: Expression(0), false_term: Counter(1) } => 14:8 - 14:36; + bb0: { + Coverage::CounterIncrement(0);