Fix error message for env! when env var is not valid Unicode

Author: beetrees

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

compiler/rustc_builtin_macros/src/env.rs

@@ -11,18 +11,19 @@ use rustc_expand::base::{DummyResult, ExpandResult, ExtCtxt, MacEager, MacroExpa
 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<Symbol> {
+fn lookup_env<'cx>(cx: &'cx ExtCtxt<'_>, var: Symbol) -> Result<Symbol, VarError> {
     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,35 +109,43 @@ 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
             else {
                 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))
 }

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 {

library/core/src/macros/mod.rs

@@ -1053,7 +1053,8 @@ pub(crate) mod builtin {
     ///
     /// 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
     ///

tests/run-make/non-utf8-env/Makefile

@@ -0,0 +1,7 @@
+include ../tools.mk
+
+# only-unix
+# ignore-cross-compile
+
+all:
+	(NOT_UTF8_VAR=$$(printf "\377") $(RUSTC) non_utf8_env.rs 2>&1 && exit 1 || exit 0) | diff - non_utf8_env.stderr

tests/run-make/non-utf8-env/non_utf8_env.rs

@@ -0,0 +1,3 @@
+fn main() {
+    let _ = env!("NOT_UTF8_VAR");
+}

tests/run-make/non-utf8-env/non_utf8_env.stderr

@@ -0,0 +1,10 @@
+error: environment variable `NOT_UTF8_VAR` is not a valid Unicode string
+ --> non_utf8_env.rs:2:13
+  |
+2 |     let _ = env!("NOT_UTF8_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
+