Auto merge of #122663 - beetrees:non-unicode-env-error, r=TaKO8Ki

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

Currently (without this PR) the `env!` macro emits an ```environment variable `name` not defined at compile time``` error when the environment variable is defined, but not a valid Unicode string. This PR introduces a separate more accurate error message, and a test to verify this behaviour.

For reference, before this PR, the new test would have outputted:
```
error: environment variable `NON_UNICODE_VAR` not defined at compile time
 --> non_unicode_env.rs:2:13
  |
2 |     let _ = env!("NON_UNICODE_VAR");
  |             ^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: use `std::env::var("NON_UNICODE_VAR")` to read the variable at run time
  = 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
```

whereas with this PR, the test ouputs:
```
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
```

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,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))
 }

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 {