From def6b99b4a8e1362c9271e39de68e63cacac11ec Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sun, 19 May 2024 16:52:19 -0700 Subject: [PATCH 1/3] move rustc-rust-log test into ui/rustc-env --- .../auxiliary/rust-log-aux.rs} | 0 tests/ui/{rustc-rust-log.rs => rustc-env/rust-log.rs} | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename tests/ui/{auxiliary/rustc-rust-log-aux.rs => rustc-env/auxiliary/rust-log-aux.rs} (100%) rename tests/ui/{rustc-rust-log.rs => rustc-env/rust-log.rs} (88%) diff --git a/tests/ui/auxiliary/rustc-rust-log-aux.rs b/tests/ui/rustc-env/auxiliary/rust-log-aux.rs similarity index 100% rename from tests/ui/auxiliary/rustc-rust-log-aux.rs rename to tests/ui/rustc-env/auxiliary/rust-log-aux.rs diff --git a/tests/ui/rustc-rust-log.rs b/tests/ui/rustc-env/rust-log.rs similarity index 88% rename from tests/ui/rustc-rust-log.rs rename to tests/ui/rustc-env/rust-log.rs index 299b6c40a56..10040754593 100644 --- a/tests/ui/rustc-rust-log.rs +++ b/tests/ui/rustc-env/rust-log.rs @@ -5,7 +5,7 @@ //@ dont-check-compiler-stdout //@ dont-check-compiler-stderr //@ compile-flags: --error-format human -//@ aux-build: rustc-rust-log-aux.rs +//@ aux-build: rust-log-aux.rs //@ rustc-env:RUSTC_LOG=debug fn main() {} From 9985821b2fe20a969b799ec920563170fcc45d8f Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sun, 19 May 2024 17:25:25 -0700 Subject: [PATCH 2/3] defrost RUST_MIN_STACK=ice rustc hello.rs An earlier commit included the change for a suggestion here. Unfortunately, it also used unwrap instead of dying properly. Roll out the ~~rice paper~~ EarlyDiagCtxt before we do anything that might leave a mess. --- compiler/rustc_interface/src/interface.rs | 1 + compiler/rustc_interface/src/util.rs | 35 ++++++++++++++++------ tests/ui/rustc-env/README.md | 6 ++++ tests/ui/rustc-env/min-stack-banana.rs | 2 ++ tests/ui/rustc-env/min-stack-banana.stderr | 2 ++ 5 files changed, 37 insertions(+), 9 deletions(-) create mode 100644 tests/ui/rustc-env/README.md create mode 100644 tests/ui/rustc-env/min-stack-banana.rs create mode 100644 tests/ui/rustc-env/min-stack-banana.stderr diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 55304bbbd92..d43be6cebcb 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -389,6 +389,7 @@ pub fn run_compiler(config: Config, f: impl FnOnce(&Compiler) -> R + Se let hash_kind = config.opts.unstable_opts.src_hash_algorithm(&target); util::run_in_thread_pool_with_globals( + &early_dcx, config.opts.edition, config.opts.unstable_opts.threads, SourceMapInputs { file_loader, path_mapping, hash_kind }, diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index ce4d3825015..f549ae49012 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -51,20 +51,32 @@ pub fn add_configuration(cfg: &mut Cfg, sess: &mut Session, codegen_backend: &dy pub static STACK_SIZE: OnceLock = OnceLock::new(); pub const DEFAULT_STACK_SIZE: usize = 8 * 1024 * 1024; -fn init_stack_size() -> usize { +fn init_stack_size(early_dcx: &EarlyDiagCtxt) -> usize { // Obey the environment setting or default *STACK_SIZE.get_or_init(|| { env::var_os("RUST_MIN_STACK") - .map(|os_str| os_str.to_string_lossy().into_owned()) - // ignore if it is set to nothing - .filter(|s| s.trim() != "") - .map(|s| s.trim().parse::().unwrap()) + .as_ref() + .map(|os_str| os_str.to_string_lossy()) + // if someone finds out `export RUST_MIN_STACK=640000` isn't enough stack + // they might try to "unset" it by running `RUST_MIN_STACK= rustc code.rs` + // this is wrong, but std would nonetheless "do what they mean", so let's do likewise + .filter(|s| !s.trim().is_empty()) + // rustc is a batch program, so error early on inputs which are unlikely to be intended + // so no one thinks we parsed them setting `RUST_MIN_STACK="64 megabytes"` + // FIXME: we could accept `RUST_MIN_STACK=64MB`, perhaps? + .map(|s| { + s.trim().parse::().unwrap_or_else(|_| { + #[allow(rustc::untranslatable_diagnostic)] + early_dcx.early_fatal("`RUST_MIN_STACK` should be unset or a number of bytes") + }) + }) // otherwise pick a consistent default .unwrap_or(DEFAULT_STACK_SIZE) }) } fn run_in_thread_with_globals R + Send, R: Send>( + thread_stack_size: usize, edition: Edition, sm_inputs: SourceMapInputs, f: F, @@ -75,7 +87,7 @@ fn run_in_thread_with_globals R + Send, R: Send>( // the parallel compiler, in particular to ensure there is no accidental // sharing of data between the main thread and the compilation thread // (which might cause problems for the parallel compiler). - let builder = thread::Builder::new().name("rustc".to_string()).stack_size(init_stack_size()); + let builder = thread::Builder::new().name("rustc".to_string()).stack_size(thread_stack_size); // We build the session globals and run `f` on the spawned thread, because // `SessionGlobals` does not impl `Send` in the non-parallel compiler. @@ -100,16 +112,19 @@ fn run_in_thread_with_globals R + Send, R: Send>( #[cfg(not(parallel_compiler))] pub(crate) fn run_in_thread_pool_with_globals R + Send, R: Send>( + thread_builder_diag: &EarlyDiagCtxt, edition: Edition, _threads: usize, sm_inputs: SourceMapInputs, f: F, ) -> R { - run_in_thread_with_globals(edition, sm_inputs, f) + let thread_stack_size = init_stack_size(thread_builder_diag); + run_in_thread_with_globals(thread_stack_size, edition, sm_inputs, f) } #[cfg(parallel_compiler)] pub(crate) fn run_in_thread_pool_with_globals R + Send, R: Send>( + thread_builder_diag: &EarlyDiagCtxt, edition: Edition, threads: usize, sm_inputs: SourceMapInputs, @@ -121,10 +136,12 @@ pub(crate) fn run_in_thread_pool_with_globals R + Send, use rustc_query_system::query::{break_query_cycles, QueryContext}; use std::process; + let thread_stack_size = init_stack_size(thread_builder_diag); + let registry = sync::Registry::new(std::num::NonZero::new(threads).unwrap()); if !sync::is_dyn_thread_safe() { - return run_in_thread_with_globals(edition, sm_inputs, |current_gcx| { + return run_in_thread_with_globals(thread_stack_size, edition, sm_inputs, |current_gcx| { // Register the thread for use with the `WorkerLocal` type. registry.register(); @@ -167,7 +184,7 @@ pub(crate) fn run_in_thread_pool_with_globals R + Send, }) .unwrap(); }) - .stack_size(init_stack_size()); + .stack_size(thread_stack_size); // We create the session globals on the main thread, then create the thread // pool. Upon creation, each worker thread created gets a copy of the diff --git a/tests/ui/rustc-env/README.md b/tests/ui/rustc-env/README.md new file mode 100644 index 00000000000..ff674f3e6cc --- /dev/null +++ b/tests/ui/rustc-env/README.md @@ -0,0 +1,6 @@ +Some environment variables affect rustc's behavior not because they are major compiler interfaces +but rather because rustc is, ultimately, a Rust program, with debug logging, stack control, etc. + +Prefer to group tests that use environment variables to control something about rustc's core UX, +like "can we parse this number of parens if we raise RUST_MIN_STACK?" with related code for that +compiler feature. diff --git a/tests/ui/rustc-env/min-stack-banana.rs b/tests/ui/rustc-env/min-stack-banana.rs new file mode 100644 index 00000000000..abbb6843710 --- /dev/null +++ b/tests/ui/rustc-env/min-stack-banana.rs @@ -0,0 +1,2 @@ +//@ rustc-env:RUST_MIN_STACK=banana +fn main() {} diff --git a/tests/ui/rustc-env/min-stack-banana.stderr b/tests/ui/rustc-env/min-stack-banana.stderr new file mode 100644 index 00000000000..a8b46d8ae44 --- /dev/null +++ b/tests/ui/rustc-env/min-stack-banana.stderr @@ -0,0 +1,2 @@ +error: `RUST_MIN_STACK` should be unset or a number of bytes + From b6d0d6da553d3836767b484530b23ad89807f470 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sun, 19 May 2024 19:59:06 -0700 Subject: [PATCH 3/3] note value of RUST_MIN_STACK and explain unsetting --- compiler/rustc_interface/src/util.rs | 12 +++++++++--- tests/ui/rustc-env/min-stack-banana.stderr | 4 +++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index f549ae49012..ce2382b9501 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -65,9 +65,15 @@ fn init_stack_size(early_dcx: &EarlyDiagCtxt) -> usize { // so no one thinks we parsed them setting `RUST_MIN_STACK="64 megabytes"` // FIXME: we could accept `RUST_MIN_STACK=64MB`, perhaps? .map(|s| { - s.trim().parse::().unwrap_or_else(|_| { - #[allow(rustc::untranslatable_diagnostic)] - early_dcx.early_fatal("`RUST_MIN_STACK` should be unset or a number of bytes") + let s = s.trim(); + // FIXME(workingjubilee): add proper diagnostics when we factor out "pre-run" setup + #[allow(rustc::untranslatable_diagnostic, rustc::diagnostic_outside_of_impl)] + s.parse::().unwrap_or_else(|_| { + let mut err = early_dcx.early_struct_fatal(format!( + r#"`RUST_MIN_STACK` should be a number of bytes, but was "{s}""#, + )); + err.note("you can also unset `RUST_MIN_STACK` to use the default stack size"); + err.emit() }) }) // otherwise pick a consistent default diff --git a/tests/ui/rustc-env/min-stack-banana.stderr b/tests/ui/rustc-env/min-stack-banana.stderr index a8b46d8ae44..d379367ab4f 100644 --- a/tests/ui/rustc-env/min-stack-banana.stderr +++ b/tests/ui/rustc-env/min-stack-banana.stderr @@ -1,2 +1,4 @@ -error: `RUST_MIN_STACK` should be unset or a number of bytes +error: `RUST_MIN_STACK` should be a number of bytes, but was "banana" + | + = note: you can also unset `RUST_MIN_STACK` to use the default stack size