Skip to content

Commit

Permalink
Rollup merge of rust-lang#131913 - jieyouxu:only_debug_assertions, r=…
Browse files Browse the repository at this point in the history
…onur-ozkan

Add `{ignore,needs}-{rustc,std}-debug-assertions` directive support

Add `{ignore,needs}-{rustc,std}-debug-assertions` compiletest directives and retire the old `{ignore,only}-debug` directives. The old `{ignore,only}-debug` directives were ambiguous because you could have std built with debug assertions but rustc not built with debug assertions or vice versa. If we want to support the use case of controlling test run based on if rustc was built with debug assertions, then having `{ignore,only}-debug` will be very confusing.

cc `@matthiaskrgr`

Closes rust-lang#123987.

r? bootstrap (or compiler tbh)
  • Loading branch information
GuillaumeGomez authored Nov 7, 2024
2 parents 3da2125 + 0d5cc8e commit 2a1792b
Show file tree
Hide file tree
Showing 22 changed files with 119 additions and 43 deletions.
10 changes: 7 additions & 3 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1938,9 +1938,13 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the

cmd.arg("--json");

if builder.config.rust_debug_assertions_std {
cmd.arg("--with-debug-assertions");
};
if builder.config.rustc_debug_assertions {
cmd.arg("--with-rustc-debug-assertions");
}

if builder.config.std_debug_assertions {
cmd.arg("--with-std-debug-assertions");
}

let mut llvm_components_passed = false;
let mut copts_passed = false;
Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/src/core/builder/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -833,9 +833,9 @@ impl Builder<'_> {
cargo.env(
profile_var("DEBUG_ASSERTIONS"),
if mode == Mode::Std {
self.config.rust_debug_assertions_std.to_string()
self.config.std_debug_assertions.to_string()
} else {
self.config.rust_debug_assertions.to_string()
self.config.rustc_debug_assertions.to_string()
},
);
cargo.env(
Expand Down
33 changes: 17 additions & 16 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,10 @@ pub struct Config {
pub rust_optimize: RustOptimize,
pub rust_codegen_units: Option<u32>,
pub rust_codegen_units_std: Option<u32>,
pub rust_debug_assertions: bool,
pub rust_debug_assertions_std: bool,

pub rustc_debug_assertions: bool,
pub std_debug_assertions: bool,

pub rust_overflow_checks: bool,
pub rust_overflow_checks_std: bool,
pub rust_debug_logging: bool,
Expand Down Expand Up @@ -1115,9 +1117,9 @@ define_config! {
debug: Option<bool> = "debug",
codegen_units: Option<u32> = "codegen-units",
codegen_units_std: Option<u32> = "codegen-units-std",
debug_assertions: Option<bool> = "debug-assertions",
rustc_debug_assertions: Option<bool> = "debug-assertions",
randomize_layout: Option<bool> = "randomize-layout",
debug_assertions_std: Option<bool> = "debug-assertions-std",
std_debug_assertions: Option<bool> = "debug-assertions-std",
overflow_checks: Option<bool> = "overflow-checks",
overflow_checks_std: Option<bool> = "overflow-checks-std",
debug_logging: Option<bool> = "debug-logging",
Expand Down Expand Up @@ -1652,8 +1654,8 @@ impl Config {
let mut llvm_offload = None;
let mut llvm_plugins = None;
let mut debug = None;
let mut debug_assertions = None;
let mut debug_assertions_std = None;
let mut rustc_debug_assertions = None;
let mut std_debug_assertions = None;
let mut overflow_checks = None;
let mut overflow_checks_std = None;
let mut debug_logging = None;
Expand All @@ -1675,8 +1677,8 @@ impl Config {
debug: debug_toml,
codegen_units,
codegen_units_std,
debug_assertions: debug_assertions_toml,
debug_assertions_std: debug_assertions_std_toml,
rustc_debug_assertions: rustc_debug_assertions_toml,
std_debug_assertions: std_debug_assertions_toml,
overflow_checks: overflow_checks_toml,
overflow_checks_std: overflow_checks_std_toml,
debug_logging: debug_logging_toml,
Expand Down Expand Up @@ -1734,8 +1736,8 @@ impl Config {
config.download_ci_rustc_commit(download_rustc, config.llvm_assertions);

debug = debug_toml;
debug_assertions = debug_assertions_toml;
debug_assertions_std = debug_assertions_std_toml;
rustc_debug_assertions = rustc_debug_assertions_toml;
std_debug_assertions = std_debug_assertions_toml;
overflow_checks = overflow_checks_toml;
overflow_checks_std = overflow_checks_std_toml;
debug_logging = debug_logging_toml;
Expand Down Expand Up @@ -2148,14 +2150,13 @@ impl Config {
config.rust_std_features = std_features.unwrap_or(default_std_features);

let default = debug == Some(true);
config.rust_debug_assertions = debug_assertions.unwrap_or(default);
config.rust_debug_assertions_std =
debug_assertions_std.unwrap_or(config.rust_debug_assertions);
config.rustc_debug_assertions = rustc_debug_assertions.unwrap_or(default);
config.std_debug_assertions = std_debug_assertions.unwrap_or(config.rustc_debug_assertions);
config.rust_overflow_checks = overflow_checks.unwrap_or(default);
config.rust_overflow_checks_std =
overflow_checks_std.unwrap_or(config.rust_overflow_checks);

config.rust_debug_logging = debug_logging.unwrap_or(config.rust_debug_assertions);
config.rust_debug_logging = debug_logging.unwrap_or(config.rustc_debug_assertions);

let with_defaults = |debuginfo_level_specific: Option<_>| {
debuginfo_level_specific.or(debuginfo_level).unwrap_or(if debug == Some(true) {
Expand Down Expand Up @@ -3075,8 +3076,8 @@ fn check_incompatible_options_for_ci_rustc(
debug: _,
codegen_units: _,
codegen_units_std: _,
debug_assertions: _,
debug_assertions_std: _,
rustc_debug_assertions: _,
std_debug_assertions: _,
overflow_checks: _,
overflow_checks_std: _,
debuginfo_level: _,
Expand Down
7 changes: 5 additions & 2 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,11 @@ pub struct Config {
/// Run ignored tests
pub run_ignored: bool,

/// Whether to run tests with `ignore-debug` header
pub with_debug_assertions: bool,
/// Whether rustc was built with debug assertions.
pub with_rustc_debug_assertions: bool,

/// Whether std was built with debug assertions.
pub with_std_debug_assertions: bool,

/// Only run tests that match these filters
pub filters: Vec<String>,
Expand Down
5 changes: 4 additions & 1 deletion src/tools/compiletest/src/directive-list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"ignore-coverage-map",
"ignore-coverage-run",
"ignore-cross-compile",
"ignore-debug",
"ignore-eabi",
"ignore-emscripten",
"ignore-endian-big",
Expand Down Expand Up @@ -82,13 +81,15 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"ignore-powerpc",
"ignore-remote",
"ignore-riscv64",
"ignore-rustc-debug-assertions",
"ignore-s390x",
"ignore-sgx",
"ignore-sparc64",
"ignore-spirv",
"ignore-stable",
"ignore-stage1",
"ignore-stage2",
"ignore-std-debug-assertions",
"ignore-test",
"ignore-thumb",
"ignore-thumbv8m.base-none-eabi",
Expand Down Expand Up @@ -135,6 +136,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"needs-relocation-model-pic",
"needs-run-enabled",
"needs-rust-lld",
"needs-rustc-debug-assertions",
"needs-sanitizer-address",
"needs-sanitizer-cfi",
"needs-sanitizer-dataflow",
Expand All @@ -147,6 +149,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"needs-sanitizer-shadow-call-stack",
"needs-sanitizer-support",
"needs-sanitizer-thread",
"needs-std-debug-assertions",
"needs-symlink",
"needs-threads",
"needs-unwind",
Expand Down
11 changes: 8 additions & 3 deletions src/tools/compiletest/src/header/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,14 @@ pub(super) fn parse_cfg_name_directive<'a>(
message: "when running tests remotely",
}
condition! {
name: "debug",
condition: config.with_debug_assertions,
message: "when running tests with `ignore-debug` header",
name: "rustc-debug-assertions",
condition: config.with_rustc_debug_assertions,
message: "when rustc is built with debug assertions",
}
condition! {
name: "std-debug-assertions",
condition: config.with_std_debug_assertions,
message: "when std is built with debug assertions",
}
condition! {
name: config.debugger.as_ref().map(|d| d.to_str()),
Expand Down
10 changes: 10 additions & 0 deletions src/tools/compiletest/src/header/needs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,16 @@ pub(super) fn handle_needs(
condition: cache.llvm_zstd,
ignore_reason: "ignored if LLVM wasn't build with zstd for ELF section compression",
},
Need {
name: "needs-rustc-debug-assertions",
condition: config.with_rustc_debug_assertions,
ignore_reason: "ignored if rustc wasn't built with debug assertions",
},
Need {
name: "needs-std-debug-assertions",
condition: config.with_std_debug_assertions,
ignore_reason: "ignored if std wasn't built with debug assertions",
},
];

let (name, comment) = match ln.split_once([':', ' ']) {
Expand Down
44 changes: 44 additions & 0 deletions src/tools/compiletest/src/header/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ struct ConfigBuilder {
git_hash: bool,
system_llvm: bool,
profiler_runtime: bool,
rustc_debug_assertions: bool,
std_debug_assertions: bool,
}

impl ConfigBuilder {
Expand Down Expand Up @@ -122,6 +124,16 @@ impl ConfigBuilder {
self
}

fn rustc_debug_assertions(&mut self, is_enabled: bool) -> &mut Self {
self.rustc_debug_assertions = is_enabled;
self
}

fn std_debug_assertions(&mut self, is_enabled: bool) -> &mut Self {
self.std_debug_assertions = is_enabled;
self
}

fn build(&mut self) -> Config {
let args = &[
"compiletest",
Expand Down Expand Up @@ -170,6 +182,12 @@ impl ConfigBuilder {
if self.profiler_runtime {
args.push("--profiler-runtime".to_owned());
}
if self.rustc_debug_assertions {
args.push("--with-rustc-debug-assertions".to_owned());
}
if self.std_debug_assertions {
args.push("--with-std-debug-assertions".to_owned());
}

args.push("--rustc-path".to_string());
// This is a subtle/fragile thing. On rust-lang CI, there is no global
Expand Down Expand Up @@ -314,6 +332,32 @@ fn only_target() {
assert!(!check_ignore(&config, "//@ only-64bit"));
}

#[test]
fn rustc_debug_assertions() {
let config: Config = cfg().rustc_debug_assertions(false).build();

assert!(check_ignore(&config, "//@ needs-rustc-debug-assertions"));
assert!(!check_ignore(&config, "//@ ignore-rustc-debug-assertions"));

let config: Config = cfg().rustc_debug_assertions(true).build();

assert!(!check_ignore(&config, "//@ needs-rustc-debug-assertions"));
assert!(check_ignore(&config, "//@ ignore-rustc-debug-assertions"));
}

#[test]
fn std_debug_assertions() {
let config: Config = cfg().std_debug_assertions(false).build();

assert!(check_ignore(&config, "//@ needs-std-debug-assertions"));
assert!(!check_ignore(&config, "//@ ignore-std-debug-assertions"));

let config: Config = cfg().std_debug_assertions(true).build();

assert!(!check_ignore(&config, "//@ needs-std-debug-assertions"));
assert!(check_ignore(&config, "//@ ignore-std-debug-assertions"));
}

#[test]
fn stage() {
let config: Config = cfg().stage_id("stage1-x86_64-unknown-linux-gnu").build();
Expand Down
9 changes: 6 additions & 3 deletions src/tools/compiletest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ pub fn parse_config(args: Vec<String>) -> Config {
.optopt("", "run", "whether to execute run-* tests", "auto | always | never")
.optflag("", "ignored", "run tests marked as ignored")
.optflag("", "has-enzyme", "run tests that require enzyme")
.optflag("", "with-debug-assertions", "whether to run tests with `ignore-debug` header")
.optflag("", "with-rustc-debug-assertions", "whether rustc was built with debug assertions")
.optflag("", "with-std-debug-assertions", "whether std was built with debug assertions")
.optmulti(
"",
"skip",
Expand Down Expand Up @@ -235,7 +236,8 @@ pub fn parse_config(args: Vec<String>) -> Config {

let src_base = opt_path(matches, "src-base");
let run_ignored = matches.opt_present("ignored");
let with_debug_assertions = matches.opt_present("with-debug-assertions");
let with_rustc_debug_assertions = matches.opt_present("with-rustc-debug-assertions");
let with_std_debug_assertions = matches.opt_present("with-std-debug-assertions");
let mode = matches.opt_str("mode").unwrap().parse().expect("invalid mode");
let has_html_tidy = if mode == Mode::Rustdoc {
Command::new("tidy")
Expand Down Expand Up @@ -293,7 +295,8 @@ pub fn parse_config(args: Vec<String>) -> Config {
suite: matches.opt_str("suite").unwrap(),
debugger: None,
run_ignored,
with_debug_assertions,
with_rustc_debug_assertions,
with_std_debug_assertions,
filters,
skip: matches.opt_strs("skip"),
filter_exact: matches.opt_present("exact"),
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/binary-heap-peek-mut-pop-no-panic.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//@ compile-flags: -O
//@ ignore-debug
//@ ignore-std-debug-assertions
#![crate_type = "lib"]

use std::collections::binary_heap::PeekMut;
Expand Down
3 changes: 2 additions & 1 deletion tests/codegen/mem-replace-big-type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
// known to be `1` after inlining).

//@ compile-flags: -C no-prepopulate-passes -Zinline-mir=no
//@ ignore-debug: precondition checks in ptr::read make them a bad candidate for MIR inlining
//@ ignore-std-debug-assertions
// Reason: precondition checks in ptr::read make them a bad candidate for MIR inlining
//@ needs-deterministic-layouts

#![crate_type = "lib"]
Expand Down
3 changes: 2 additions & 1 deletion tests/codegen/mem-replace-simple-type.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//@ compile-flags: -O -C no-prepopulate-passes
//@ only-x86_64 (to not worry about usize differing)
//@ ignore-debug: precondition checks make mem::replace not a candidate for MIR inlining
//@ ignore-std-debug-assertions
// Reason: precondition checks make mem::replace not a candidate for MIR inlining

#![crate_type = "lib"]

Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/slice-reverse.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ compile-flags: -O
//@ only-x86_64
//@ ignore-debug: debug assertions prevent generating shufflevector
//@ ignore-std-debug-assertions (debug assertions prevent generating shufflevector)

#![crate_type = "lib"]

Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/vec-in-place.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//@ ignore-debug: FIXME: checks for call detect scoped noalias metadata
//@ ignore-std-debug-assertions (FIXME: checks for call detect scoped noalias metadata)
//@ compile-flags: -O -Z merge-functions=disabled
#![crate_type = "lib"]

Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/vec-shrink-panik.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// LLVM 17 realizes double panic is not possible and doesn't generate calls
// to panic_cannot_unwind.
//@ compile-flags: -O
//@ ignore-debug: plain old debug assertions
//@ ignore-std-debug-assertions (plain old debug assertions)
//@ needs-unwind
#![crate_type = "lib"]
#![feature(shrink_to)]
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/vec-with-capacity.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//@ compile-flags: -O
//@ ignore-debug
//@ ignore-std-debug-assertions
// (with debug assertions turned on, `assert_unchecked` generates a real assertion)

#![crate_type = "lib"]
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/vecdeque-drain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

//@ compile-flags: -O
//@ needs-deterministic-layouts
//@ ignore-debug: FIXME: checks for call detect scoped noalias metadata
//@ ignore-std-debug-assertions (FIXME: checks for call detect scoped noalias metadata)

#![crate_type = "lib"]

Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/vecdeque_no_panic.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// This test checks that `VecDeque::front[_mut]()` and `VecDeque::back[_mut]()` can't panic.

//@ compile-flags: -O
//@ ignore-debug: plain old debug assertions
//@ ignore-std-debug-assertions (plain old debug assertions)

#![crate_type = "lib"]

Expand Down
3 changes: 2 additions & 1 deletion tests/mir-opt/pre-codegen/mem_replace.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// skip-filecheck
//@ compile-flags: -O -C debuginfo=0 -Zmir-opt-level=2 -Zinline-mir
//@ ignore-debug: precondition checks on ptr::read/write are under cfg(debug_assertions)
//@ ignore-std-debug-assertions
// Reason: precondition checks on ptr::read/write are under cfg(debug_assertions)
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY

#![crate_type = "lib"]
Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/pre-codegen/ptr_offset.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// skip-filecheck
//@ compile-flags: -O -C debuginfo=0 -Zmir-opt-level=2 -Zinline-mir
//@ ignore-debug: precondition checks are under cfg(debug_assertions)
//@ ignore-std-debug-assertions (precondition checks are under cfg(debug_assertions))
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY

#![crate_type = "lib"]
Expand Down
Loading

0 comments on commit 2a1792b

Please sign in to comment.