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.

Author: workingjubilee

compiler/rustc_interface/src/interface.rs

@@ -389,6 +389,7 @@ pub fn run_compiler<R: Send>(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 },

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<usize> = 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::<usize>().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::<usize>().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<F: FnOnce(CurrentGcx) -> 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<F: FnOnce(CurrentGcx) -> 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<F: FnOnce(CurrentGcx) -> R + Send, R: Send>(
 
 #[cfg(not(parallel_compiler))]
 pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce(CurrentGcx) -> 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<F: FnOnce(CurrentGcx) -> 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<F: FnOnce(CurrentGcx) -> 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<F: FnOnce(CurrentGcx) -> 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

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.

tests/ui/rustc-env/min-stack-banana.rs

@@ -0,0 +1,2 @@
+//@ rustc-env:RUST_MIN_STACK=banana
+fn main() {}

tests/ui/rustc-env/min-stack-banana.stderr

@@ -0,0 +1,2 @@
+error: `RUST_MIN_STACK` should be unset or a number of bytes
+