Skip to content

Commit

Permalink
Rollup merge of #127322 - onur-ozkan:ci-rustc-incompatible-options, r…
Browse files Browse the repository at this point in the history
…=Mark-Simulacrum

handle ci-rustc incompatible options during config parse

This PR ensures that `config.toml` does not use CI rustc incompatible options when CI rustc is enabled (just like [ci-llvm checks](https://github.com/rust-lang/rust/blob/e2cf31a6148725bde4ea48acf1e4fe72675257a2/src/bootstrap/src/core/config/config.rs#L1809-L1836)). Some options can change compiler's behavior in certain scenarios. If we don't check these incompatible options, CI runners using CI rustc might ignore options we have explicitly set. This could be dangerous as we might think a rustc test passed with option T but in fact it wasn't tested with option T.

Later in #122709, I will disable CI rustc if any of those options were used (similar to [this approach](https://github.com/rust-lang/rust/blob/dd2c24aafddbd9cc170f32f5b447c7d3005c7412/src/ci/run.sh#L165-L169)). If CI runners fail because of these checks, it means the logic in run.sh isn't covering the incompatible options correctly (since any incompatible option should turn off CI rustc).

The list may not be complete, but should be a good first step as it's better than nothing!

Blocker for #122709
  • Loading branch information
matthiaskrgr authored Jul 14, 2024
2 parents 459120d + 335bdd4 commit 60e10e6
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 8 deletions.
2 changes: 1 addition & 1 deletion config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@

# Forces frame pointers to be used with `-Cforce-frame-pointers`.
# This can be helpful for profiling at a small performance cost.
# frame-pointers = false
#frame-pointers = false

# Indicates whether stack protectors should be used
# via the unstable option `-Zstack-protector`.
Expand Down
125 changes: 118 additions & 7 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ macro_rules! check_ci_llvm {
assert!(
$name.is_none(),
"setting {} is incompatible with download-ci-llvm.",
stringify!($name)
stringify!($name).replace("_", "-")
);
};
}
Expand Down Expand Up @@ -1568,7 +1568,15 @@ impl Config {
let mut lld_enabled = None;

let mut is_user_configured_rust_channel = false;

if let Some(rust) = toml.rust {
config.download_rustc_commit =
config.download_ci_rustc_commit(rust.download_rustc.clone());

if config.download_rustc_commit.is_some() {
check_incompatible_options_for_ci_rustc(&rust);
}

let Rust {
optimize: optimize_toml,
debug: debug_toml,
Expand Down Expand Up @@ -1616,7 +1624,7 @@ impl Config {
new_symbol_mangling,
profile_generate,
profile_use,
download_rustc,
download_rustc: _,
lto,
validate_mir_opts,
frame_pointers,
Expand All @@ -1626,11 +1634,7 @@ impl Config {
} = rust;

is_user_configured_rust_channel = channel.is_some();
set(&mut config.channel, channel);

config.download_rustc_commit = config.download_ci_rustc_commit(download_rustc);

// FIXME: handle download-rustc incompatible options.
set(&mut config.channel, channel.clone());

debug = debug_toml;
debug_assertions = debug_assertions_toml;
Expand Down Expand Up @@ -2608,6 +2612,113 @@ impl Config {
}
}

/// Checks the CI rustc incompatible options by destructuring the `Rust` instance
/// and makes sure that no rust options from config.toml are missed.
fn check_incompatible_options_for_ci_rustc(rust: &Rust) {
macro_rules! err {
($name:expr) => {
assert!(
$name.is_none(),
"ERROR: Setting `rust.{}` is incompatible with `rust.download-rustc`.",
stringify!($name).replace("_", "-")
);
};
}

macro_rules! warn {
($name:expr) => {
if $name.is_some() {
println!(
"WARNING: `rust.{}` has no effect with `rust.download-rustc`.",
stringify!($name).replace("_", "-")
);
}
};
}

let Rust {
// Following options are the CI rustc incompatible ones.
optimize,
debug_logging,
debuginfo_level_rustc,
llvm_tools,
llvm_bitcode_linker,
lto,
stack_protector,
strip,
lld_mode,
jemalloc,
rpath,
channel,
description,
incremental,
default_linker,

// Rest of the options can simply be ignored.
debug: _,
codegen_units: _,
codegen_units_std: _,
debug_assertions: _,
debug_assertions_std: _,
overflow_checks: _,
overflow_checks_std: _,
debuginfo_level: _,
debuginfo_level_std: _,
debuginfo_level_tools: _,
debuginfo_level_tests: _,
split_debuginfo: _,
backtrace: _,
parallel_compiler: _,
musl_root: _,
verbose_tests: _,
optimize_tests: _,
codegen_tests: _,
omit_git_hash: _,
dist_src: _,
save_toolstates: _,
codegen_backends: _,
lld: _,
deny_warnings: _,
backtrace_on_ice: _,
verify_llvm_ir: _,
thin_lto_import_instr_limit: _,
remap_debuginfo: _,
test_compare_mode: _,
llvm_libunwind: _,
control_flow_guard: _,
ehcont_guard: _,
new_symbol_mangling: _,
profile_generate: _,
profile_use: _,
download_rustc: _,
validate_mir_opts: _,
frame_pointers: _,
} = rust;

// There are two kinds of checks for CI rustc incompatible options:
// 1. Checking an option that may change the compiler behaviour/output.
// 2. Checking an option that have no effect on the compiler behaviour/output.
//
// If the option belongs to the first category, we call `err` macro for a hard error;
// otherwise, we just print a warning with `warn` macro.
err!(optimize);
err!(debug_logging);
err!(debuginfo_level_rustc);
err!(default_linker);
err!(rpath);
err!(strip);
err!(stack_protector);
err!(lld_mode);
err!(llvm_tools);
err!(llvm_bitcode_linker);
err!(jemalloc);
err!(lto);

warn!(channel);
warn!(description);
warn!(incremental);
}

fn set<T>(field: &mut T, val: Option<T>) {
if let Some(v) = val {
*field = v;
Expand Down

0 comments on commit 60e10e6

Please sign in to comment.