From 0bbaa2505be6d7e6483a0dcbe585c96684e73da5 Mon Sep 17 00:00:00 2001 From: beetrees Date: Sun, 17 Mar 2024 21:12:17 +0000 Subject: [PATCH] Fix error message for `env!` when env var is not valid Unicode --- compiler/rustc_builtin_macros/messages.ftl | 2 + compiler/rustc_builtin_macros/src/env.rs | 51 +++++++++++-------- compiler/rustc_builtin_macros/src/errors.rs | 8 +++ library/core/src/macros/mod.rs | 3 +- src/tools/run-make-support/src/lib.rs | 6 ++- src/tools/run-make-support/src/rustc.rs | 18 +++++++ .../non-unicode-env/non_unicode_env.rs | 3 ++ .../non-unicode-env/non_unicode_env.stderr | 10 ++++ tests/run-make/non-unicode-env/rmake.rs | 14 +++++ 9 files changed, 92 insertions(+), 23 deletions(-) create mode 100644 tests/run-make/non-unicode-env/non_unicode_env.rs create mode 100644 tests/run-make/non-unicode-env/non_unicode_env.stderr create mode 100644 tests/run-make/non-unicode-env/rmake.rs diff --git a/compiler/rustc_builtin_macros/messages.ftl b/compiler/rustc_builtin_macros/messages.ftl index ba8401393d7..2a5bc58af3b 100644 --- a/compiler/rustc_builtin_macros/messages.ftl +++ b/compiler/rustc_builtin_macros/messages.ftl @@ -114,6 +114,8 @@ builtin_macros_env_not_defined = environment variable `{$var}` not defined at co .cargo = Cargo sets build script variables at run time. Use `std::env::var({$var_expr})` instead .custom = use `std::env::var({$var_expr})` to read the variable at run time +builtin_macros_env_not_unicode = environment variable `{$var}` is not a valid Unicode string + builtin_macros_env_takes_args = `env!()` takes 1 or 2 arguments builtin_macros_expected_one_cfg_pattern = expected 1 cfg-pattern diff --git a/compiler/rustc_builtin_macros/src/env.rs b/compiler/rustc_builtin_macros/src/env.rs index bce710e5cab..93873045943 100644 --- a/compiler/rustc_builtin_macros/src/env.rs +++ b/compiler/rustc_builtin_macros/src/env.rs @@ -11,18 +11,19 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::Span; use std::env; +use std::env::VarError; use thin_vec::thin_vec; use crate::errors; -fn lookup_env<'cx>(cx: &'cx ExtCtxt<'_>, var: Symbol) -> Option { +fn lookup_env<'cx>(cx: &'cx ExtCtxt<'_>, var: Symbol) -> Result { let var = var.as_str(); if let Some(value) = cx.sess.opts.logical_env.get(var) { - return Some(Symbol::intern(value)); + return Ok(Symbol::intern(value)); } // If the environment variable was not defined with the `--env-set` option, we try to retrieve it // from rustc's environment. - env::var(var).ok().as_deref().map(Symbol::intern) + Ok(Symbol::intern(&env::var(var)?)) } pub fn expand_option_env<'cx>( @@ -39,7 +40,7 @@ pub fn expand_option_env<'cx>( }; let sp = cx.with_def_site_ctxt(sp); - let value = lookup_env(cx, var); + let value = lookup_env(cx, var).ok(); cx.sess.psess.env_depinfo.borrow_mut().insert((var, value)); let e = match value { None => { @@ -108,9 +109,9 @@ pub fn expand_env<'cx>( let span = cx.with_def_site_ctxt(sp); let value = lookup_env(cx, var); - cx.sess.psess.env_depinfo.borrow_mut().insert((var, value)); + cx.sess.psess.env_depinfo.borrow_mut().insert((var, value.as_ref().ok().copied())); let e = match value { - None => { + Err(err) => { let ExprKind::Lit(token::Lit { kind: LitKind::Str | LitKind::StrRaw(..), symbol, .. }) = &var_expr.kind @@ -118,25 +119,33 @@ pub fn expand_env<'cx>( unreachable!("`expr_to_string` ensures this is a string lit") }; - let guar = if let Some(msg_from_user) = custom_msg { - cx.dcx().emit_err(errors::EnvNotDefinedWithUserMessage { span, msg_from_user }) - } else if is_cargo_env_var(var.as_str()) { - cx.dcx().emit_err(errors::EnvNotDefined::CargoEnvVar { - span, - var: *symbol, - var_expr: var_expr.ast_deref(), - }) - } else { - cx.dcx().emit_err(errors::EnvNotDefined::CustomEnvVar { - span, - var: *symbol, - var_expr: var_expr.ast_deref(), - }) + let guar = match err { + VarError::NotPresent => { + if let Some(msg_from_user) = custom_msg { + cx.dcx() + .emit_err(errors::EnvNotDefinedWithUserMessage { span, msg_from_user }) + } else if is_cargo_env_var(var.as_str()) { + cx.dcx().emit_err(errors::EnvNotDefined::CargoEnvVar { + span, + var: *symbol, + var_expr: var_expr.ast_deref(), + }) + } else { + cx.dcx().emit_err(errors::EnvNotDefined::CustomEnvVar { + span, + var: *symbol, + var_expr: var_expr.ast_deref(), + }) + } + } + VarError::NotUnicode(_) => { + cx.dcx().emit_err(errors::EnvNotUnicode { span, var: *symbol }) + } }; return ExpandResult::Ready(DummyResult::any(sp, guar)); } - Some(value) => cx.expr_str(span, value), + Ok(value) => cx.expr_str(span, value), }; ExpandResult::Ready(MacEager::expr(e)) } diff --git a/compiler/rustc_builtin_macros/src/errors.rs b/compiler/rustc_builtin_macros/src/errors.rs index 377aff8fb6c..6b6647ef085 100644 --- a/compiler/rustc_builtin_macros/src/errors.rs +++ b/compiler/rustc_builtin_macros/src/errors.rs @@ -458,6 +458,14 @@ pub(crate) enum EnvNotDefined<'a> { }, } +#[derive(Diagnostic)] +#[diag(builtin_macros_env_not_unicode)] +pub(crate) struct EnvNotUnicode { + #[primary_span] + pub(crate) span: Span, + pub(crate) var: Symbol, +} + #[derive(Diagnostic)] #[diag(builtin_macros_format_requires_string)] pub(crate) struct FormatRequiresString { diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index 574a357b44a..6da05a1ca86 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -1053,7 +1053,8 @@ macro_rules! format_args_nl { /// /// If the environment variable is not defined, then a compilation error /// will be emitted. To not emit a compile error, use the [`option_env!`] - /// macro instead. + /// macro instead. A compilation error will also be emitted if the + /// environment variable is not a vaild Unicode string. /// /// # Examples /// diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index 7975677286d..48fa2bbf1ac 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -19,7 +19,11 @@ pub fn tmp_dir() -> PathBuf { } fn handle_failed_output(cmd: &str, output: Output, caller_line_number: u32) -> ! { - eprintln!("command failed at line {caller_line_number}"); + if output.status.success() { + eprintln!("command incorrectly succeeded at line {caller_line_number}"); + } else { + eprintln!("command failed at line {caller_line_number}"); + } eprintln!("{cmd}"); eprintln!("output status: `{}`", output.status); eprintln!("=== STDOUT ===\n{}\n\n", String::from_utf8(output.stdout).unwrap()); diff --git a/src/tools/run-make-support/src/rustc.rs b/src/tools/run-make-support/src/rustc.rs index 1b358817a79..50ff0d26bbb 100644 --- a/src/tools/run-make-support/src/rustc.rs +++ b/src/tools/run-make-support/src/rustc.rs @@ -1,4 +1,5 @@ use std::env; +use std::ffi::OsStr; use std::path::Path; use std::process::{Command, Output}; @@ -133,6 +134,11 @@ pub fn args(&mut self, args: &[&str]) -> &mut Self { self } + pub fn env(&mut self, name: impl AsRef, value: impl AsRef) -> &mut Self { + self.cmd.env(name, value); + self + } + // Command inspection, output and running helper methods /// Get the [`Output`][std::process::Output] of the finished `rustc` process. @@ -153,6 +159,18 @@ pub fn run(&mut self) -> Output { output } + #[track_caller] + pub fn run_fail(&mut self) -> Output { + let caller_location = std::panic::Location::caller(); + let caller_line_number = caller_location.line(); + + let output = self.cmd.output().unwrap(); + if output.status.success() { + handle_failed_output(&format!("{:#?}", self.cmd), output, caller_line_number); + } + output + } + /// Inspect what the underlying [`Command`] is up to the current construction. pub fn inspect(&mut self, f: impl FnOnce(&Command)) -> &mut Self { f(&self.cmd); diff --git a/tests/run-make/non-unicode-env/non_unicode_env.rs b/tests/run-make/non-unicode-env/non_unicode_env.rs new file mode 100644 index 00000000000..865fc937365 --- /dev/null +++ b/tests/run-make/non-unicode-env/non_unicode_env.rs @@ -0,0 +1,3 @@ +fn main() { + let _ = env!("NON_UNICODE_VAR"); +} diff --git a/tests/run-make/non-unicode-env/non_unicode_env.stderr b/tests/run-make/non-unicode-env/non_unicode_env.stderr new file mode 100644 index 00000000000..c4dcd7b2eb7 --- /dev/null +++ b/tests/run-make/non-unicode-env/non_unicode_env.stderr @@ -0,0 +1,10 @@ +error: environment variable `NON_UNICODE_VAR` is not a valid Unicode string + --> non_unicode_env.rs:2:13 + | +2 | let _ = env!("NON_UNICODE_VAR"); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `env` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 1 previous error + diff --git a/tests/run-make/non-unicode-env/rmake.rs b/tests/run-make/non-unicode-env/rmake.rs new file mode 100644 index 00000000000..ba4aa1609b5 --- /dev/null +++ b/tests/run-make/non-unicode-env/rmake.rs @@ -0,0 +1,14 @@ +extern crate run_make_support; + +use run_make_support::rustc; + +fn main() { + #[cfg(unix)] + let non_unicode: &std::ffi::OsStr = std::os::unix::ffi::OsStrExt::from_bytes(&[0xFF]); + #[cfg(windows)] + let non_unicode: std::ffi::OsString = std::os::windows::ffi::OsStringExt::from_wide(&[0xD800]); + let output = rustc().input("non_unicode_env.rs").env("NON_UNICODE_VAR", non_unicode).run_fail(); + let actual = std::str::from_utf8(&output.stderr).unwrap(); + let expected = std::fs::read_to_string("non_unicode_env.stderr").unwrap(); + assert_eq!(actual, expected); +}