From dc910b23f929c283893de43164a54377f84dd5e8 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 16 Nov 2024 18:50:44 +0100 Subject: [PATCH 1/5] Test that env! works with incremental compilation This currently works because it's part of expansion, and that isn't yet tracked by the query system. But we want to ensure it continues working, even if that is changed. --- tests/incremental/env/env_macro.rs | 18 ++++++++++++++++++ tests/incremental/env/option_env_macro.rs | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 tests/incremental/env/env_macro.rs create mode 100644 tests/incremental/env/option_env_macro.rs diff --git a/tests/incremental/env/env_macro.rs b/tests/incremental/env/env_macro.rs new file mode 100644 index 0000000000000..0c026328874dd --- /dev/null +++ b/tests/incremental/env/env_macro.rs @@ -0,0 +1,18 @@ +// Check that changes to environment variables are propagated to `env!`. +// +// This test is intentionally written to not use any `#[cfg(rpass*)]`, to +// _really_ test that we re-compile if the environment variable changes. + +//@ revisions: cfail1 rpass2 rpass3 cfail4 +//@ [cfail1]unset-rustc-env:EXAMPLE_ENV +//@ [rpass2]rustc-env:EXAMPLE_ENV=one +//@ [rpass2]exec-env:EXAMPLE_ENV=one +//@ [rpass3]rustc-env:EXAMPLE_ENV=two +//@ [rpass3]exec-env:EXAMPLE_ENV=two +//@ [cfail4]unset-rustc-env:EXAMPLE_ENV + +fn main() { + assert_eq!(env!("EXAMPLE_ENV"), std::env::var("EXAMPLE_ENV").unwrap()); + //[cfail1]~^ ERROR environment variable `EXAMPLE_ENV` not defined at compile time + //[cfail4]~^^ ERROR environment variable `EXAMPLE_ENV` not defined at compile time +} diff --git a/tests/incremental/env/option_env_macro.rs b/tests/incremental/env/option_env_macro.rs new file mode 100644 index 0000000000000..44c3bfd69e050 --- /dev/null +++ b/tests/incremental/env/option_env_macro.rs @@ -0,0 +1,18 @@ +// Check that changes to environment variables are propagated to `option_env!`. +// +// This test is intentionally written to not use any `#[cfg(rpass*)]`, to +// _really_ test that we re-compile if the environment variable changes. + +//@ revisions: rpass1 rpass2 rpass3 rpass4 +//@ [rpass1]unset-rustc-env:EXAMPLE_ENV +//@ [rpass1]unset-exec-env:EXAMPLE_ENV +//@ [rpass2]rustc-env:EXAMPLE_ENV=one +//@ [rpass2]exec-env:EXAMPLE_ENV=one +//@ [rpass3]rustc-env:EXAMPLE_ENV=two +//@ [rpass3]exec-env:EXAMPLE_ENV=two +//@ [rpass4]unset-rustc-env:EXAMPLE_ENV +//@ [rpass4]unset-exec-env:EXAMPLE_ENV + +fn main() { + assert_eq!(option_env!("EXAMPLE_ENV"), std::env::var("EXAMPLE_ENV").ok().as_deref()); +} From ffa2135a8f1ef0314ae772b64e762d9a0d310026 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 1 Dec 2024 11:42:50 +0100 Subject: [PATCH 2/5] Add `TyCtx::env_var` --- compiler/rustc_errors/src/diagnostic_impls.rs | 6 +++++ compiler/rustc_interface/messages.ftl | 3 +++ compiler/rustc_interface/src/errors.rs | 8 +++++++ compiler/rustc_interface/src/passes.rs | 23 +++++++++++++++++++ compiler/rustc_middle/src/query/erase.rs | 1 + compiler/rustc_middle/src/query/mod.rs | 17 ++++++++++++++ 6 files changed, 58 insertions(+) diff --git a/compiler/rustc_errors/src/diagnostic_impls.rs b/compiler/rustc_errors/src/diagnostic_impls.rs index 798668b8bc1f0..f5ef1408fac36 100644 --- a/compiler/rustc_errors/src/diagnostic_impls.rs +++ b/compiler/rustc_errors/src/diagnostic_impls.rs @@ -242,6 +242,12 @@ impl IntoDiagArg for std::ffi::CString { } } +impl IntoDiagArg for std::ffi::OsString { + fn into_diag_arg(self) -> DiagArgValue { + DiagArgValue::Str(Cow::Owned(self.to_string_lossy().into_owned())) + } +} + impl IntoDiagArg for rustc_data_structures::small_c_str::SmallCStr { fn into_diag_arg(self) -> DiagArgValue { DiagArgValue::Str(Cow::Owned(self.to_string_lossy().into_owned())) diff --git a/compiler/rustc_interface/messages.ftl b/compiler/rustc_interface/messages.ftl index 47dfbc1d7fbf1..7c98f07586350 100644 --- a/compiler/rustc_interface/messages.ftl +++ b/compiler/rustc_interface/messages.ftl @@ -4,6 +4,9 @@ interface_cant_emit_mir = interface_emoji_identifier = identifiers cannot contain emoji: `{$ident}` +interface_env_var_not_unicode = + cannot read environment variable "{$key}" with value "{$var}", since it contains non-unicode data + interface_error_writing_dependencies = error writing dependencies to `{$path}`: {$error} diff --git a/compiler/rustc_interface/src/errors.rs b/compiler/rustc_interface/src/errors.rs index 939980a932fdb..e7ef7d2f375fa 100644 --- a/compiler/rustc_interface/src/errors.rs +++ b/compiler/rustc_interface/src/errors.rs @@ -1,3 +1,4 @@ +use std::ffi::OsString; use std::io; use std::path::Path; @@ -21,6 +22,13 @@ pub struct EmojiIdentifier { pub ident: Symbol, } +#[derive(Diagnostic)] +#[diag(interface_env_var_not_unicode)] +pub struct EnvVarNotUnicode { + pub key: Symbol, + pub var: OsString, +} + #[derive(Diagnostic)] #[diag(interface_mixed_bin_crate)] pub struct MixedBinCrate; diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 2905fe688b501..e6ca0ed937b88 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -1,4 +1,5 @@ use std::any::Any; +use std::env::VarError; use std::ffi::OsString; use std::io::{self, BufWriter, Write}; use std::path::{Path, PathBuf}; @@ -321,6 +322,24 @@ fn early_lint_checks(tcx: TyCtxt<'_>, (): ()) { ) } +fn env_var(tcx: TyCtxt<'_>, key: Symbol) -> Option { + let var = match env::var(key.as_str()) { + Ok(var) => Some(Symbol::intern(&var)), + Err(VarError::NotPresent) => None, + Err(VarError::NotUnicode(var)) => { + tcx.dcx().emit_err(errors::EnvVarNotUnicode { key, var }); + None + } + }; + // Also add the variable to Cargo's dependency tracking + // + // NOTE: This only works for passes run before `write_dep_info`. See that + // for extension points for configuring environment variables to be + // properly change-tracked. + tcx.sess.psess.env_depinfo.borrow_mut().insert((key, var)); + var +} + // Returns all the paths that correspond to generated files. fn generated_output_paths( tcx: TyCtxt<'_>, @@ -632,6 +651,9 @@ pub fn write_dep_info(tcx: TyCtxt<'_>) { // the side-effect of providing a complete set of all // accessed files and env vars. let _ = tcx.resolver_for_lowering(); + // Similarly, analysis, codegen and linking should state which environment + // variables they depend on here, as those passes are run after this pass, + // but we'll need the information when emitting dependency info to Cargo. let sess = tcx.sess; let _timer = sess.timer("write_dep_info"); @@ -685,6 +707,7 @@ pub static DEFAULT_QUERY_PROVIDERS: LazyLock = LazyLock::new(|| { |tcx, _| tcx.arena.alloc_from_iter(tcx.resolutions(()).stripped_cfg_items.steal()); providers.resolutions = |tcx, ()| tcx.resolver_for_lowering_raw(()).1; providers.early_lint_checks = early_lint_checks; + providers.env_var = env_var; proc_macro_decls::provide(providers); rustc_const_eval::provide(providers); rustc_middle::hir::provide(providers); diff --git a/compiler/rustc_middle/src/query/erase.rs b/compiler/rustc_middle/src/query/erase.rs index 013847f0b2d53..07eb4e495cb81 100644 --- a/compiler/rustc_middle/src/query/erase.rs +++ b/compiler/rustc_middle/src/query/erase.rs @@ -255,6 +255,7 @@ trivial! { Option, Option, Option, + Option, Option, Option, Option, diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index a17aa9ecc0446..0f9c741afa0b2 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -121,6 +121,23 @@ rustc_queries! { desc { "perform lints prior to macro expansion" } } + /// Tracked access to environment variables. + /// + /// Useful for the implementation of `std::env!`, `proc-macro`s change + /// detection and other changes in the compiler's behaviour that is easier + /// to control with an environment variable than a flag. + /// + /// NOTE: This currently does not work with dependency info in the + /// analysis, codegen and linking passes, place extra code at the top of + /// `rustc_interface::passes::write_dep_info` to make that work. + /// + /// Will emit an error and return `None` if the variable is not UTF-8. + query env_var(key: Symbol) -> Option { + // Environment variables are global state + eval_always + desc { "get the value of an environment variable" } + } + query resolutions(_: ()) -> &'tcx ty::ResolverGlobalCtxt { no_hash desc { "getting the resolver outputs" } From a01330c965776c2347403d0017831231bc32975d Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 1 Dec 2024 11:42:02 +0100 Subject: [PATCH 3/5] Make OsStr usable with queries --- .../rustc_data_structures/src/stable_hasher.rs | 2 ++ compiler/rustc_middle/src/query/erase.rs | 9 +++++++++ compiler/rustc_middle/src/query/keys.rs | 18 ++++++++++++++++++ 3 files changed, 29 insertions(+) diff --git a/compiler/rustc_data_structures/src/stable_hasher.rs b/compiler/rustc_data_structures/src/stable_hasher.rs index 0872bd2c9acc9..ca48b1f301cb9 100644 --- a/compiler/rustc_data_structures/src/stable_hasher.rs +++ b/compiler/rustc_data_structures/src/stable_hasher.rs @@ -565,6 +565,8 @@ where } } +impl_stable_traits_for_trivial_type!(::std::ffi::OsStr); + impl_stable_traits_for_trivial_type!(::std::path::Path); impl_stable_traits_for_trivial_type!(::std::path::PathBuf); diff --git a/compiler/rustc_middle/src/query/erase.rs b/compiler/rustc_middle/src/query/erase.rs index 07eb4e495cb81..246994890760b 100644 --- a/compiler/rustc_middle/src/query/erase.rs +++ b/compiler/rustc_middle/src/query/erase.rs @@ -1,3 +1,4 @@ +use std::ffi::OsStr; use std::intrinsics::transmute_unchecked; use std::mem::MaybeUninit; @@ -65,6 +66,10 @@ impl EraseType for &'_ [T] { type Result = [u8; size_of::<&'static [()]>()]; } +impl EraseType for &'_ OsStr { + type Result = [u8; size_of::<&'static OsStr>()]; +} + impl EraseType for &'_ ty::List { type Result = [u8; size_of::<&'static ty::List<()>>()]; } @@ -180,6 +185,10 @@ impl EraseType for Option<&'_ [T]> { type Result = [u8; size_of::>()]; } +impl EraseType for Option<&'_ OsStr> { + type Result = [u8; size_of::>()]; +} + impl EraseType for Option> { type Result = [u8; size_of::>>()]; } diff --git a/compiler/rustc_middle/src/query/keys.rs b/compiler/rustc_middle/src/query/keys.rs index 970dc72e1ff61..b8d36082c32b0 100644 --- a/compiler/rustc_middle/src/query/keys.rs +++ b/compiler/rustc_middle/src/query/keys.rs @@ -1,5 +1,7 @@ //! Defines the set of legal keys that can be used in queries. +use std::ffi::OsStr; + use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE, LocalDefId, LocalModDefId, ModDefId}; use rustc_hir::hir_id::{HirId, OwnerId}; use rustc_query_system::dep_graph::DepNodeIndex; @@ -492,6 +494,22 @@ impl Key for Option { } } +impl<'tcx> Key for &'tcx OsStr { + type Cache = DefaultCache; + + fn default_span(&self, _tcx: TyCtxt<'_>) -> Span { + DUMMY_SP + } +} + +impl<'tcx> Key for Option<&'tcx OsStr> { + type Cache = DefaultCache; + + fn default_span(&self, _tcx: TyCtxt<'_>) -> Span { + DUMMY_SP + } +} + /// Canonical query goals correspond to abstract trait operations that /// are not tied to any crate in particular. impl<'tcx, T: Clone> Key for CanonicalQueryInput<'tcx, T> { From da9b1bc89494e95b6c133624f41c0f3675b8d74f Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 1 Dec 2024 16:52:40 +0100 Subject: [PATCH 4/5] Use OsStr in tracked environment variables --- compiler/rustc_interface/messages.ftl | 3 --- compiler/rustc_interface/src/errors.rs | 8 ------ compiler/rustc_interface/src/passes.rs | 34 +++++++++++++++----------- compiler/rustc_middle/src/query/mod.rs | 5 ++-- compiler/rustc_middle/src/util/mod.rs | 15 ++++++++++++ 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_interface/messages.ftl b/compiler/rustc_interface/messages.ftl index 7c98f07586350..47dfbc1d7fbf1 100644 --- a/compiler/rustc_interface/messages.ftl +++ b/compiler/rustc_interface/messages.ftl @@ -4,9 +4,6 @@ interface_cant_emit_mir = interface_emoji_identifier = identifiers cannot contain emoji: `{$ident}` -interface_env_var_not_unicode = - cannot read environment variable "{$key}" with value "{$var}", since it contains non-unicode data - interface_error_writing_dependencies = error writing dependencies to `{$path}`: {$error} diff --git a/compiler/rustc_interface/src/errors.rs b/compiler/rustc_interface/src/errors.rs index e7ef7d2f375fa..939980a932fdb 100644 --- a/compiler/rustc_interface/src/errors.rs +++ b/compiler/rustc_interface/src/errors.rs @@ -1,4 +1,3 @@ -use std::ffi::OsString; use std::io; use std::path::Path; @@ -22,13 +21,6 @@ pub struct EmojiIdentifier { pub ident: Symbol, } -#[derive(Diagnostic)] -#[diag(interface_env_var_not_unicode)] -pub struct EnvVarNotUnicode { - pub key: Symbol, - pub var: OsString, -} - #[derive(Diagnostic)] #[diag(interface_mixed_bin_crate)] pub struct MixedBinCrate; diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index e6ca0ed937b88..31a1fa20d4f74 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -1,6 +1,5 @@ use std::any::Any; -use std::env::VarError; -use std::ffi::OsString; +use std::ffi::{OsStr, OsString}; use std::io::{self, BufWriter, Write}; use std::path::{Path, PathBuf}; use std::sync::{Arc, LazyLock}; @@ -322,22 +321,29 @@ fn early_lint_checks(tcx: TyCtxt<'_>, (): ()) { ) } -fn env_var(tcx: TyCtxt<'_>, key: Symbol) -> Option { - let var = match env::var(key.as_str()) { - Ok(var) => Some(Symbol::intern(&var)), - Err(VarError::NotPresent) => None, - Err(VarError::NotUnicode(var)) => { - tcx.dcx().emit_err(errors::EnvVarNotUnicode { key, var }); - None - } - }; +fn env_var_os<'tcx>(tcx: TyCtxt<'tcx>, key: &'tcx OsStr) -> Option<&'tcx OsStr> { + let value = env::var_os(key); + + let value_tcx = value.as_deref().map(|value| { + let encoded_bytes = tcx.arena.alloc_slice(value.as_encoded_bytes()); + debug_assert_eq!(value.as_encoded_bytes(), encoded_bytes); + // SAFETY: The bytes came from `as_encoded_bytes`, and we assume that + // `alloc_slice` is implemented correctly, and passes the same bytes + // back (debug asserted above). + unsafe { OsStr::from_encoded_bytes_unchecked(encoded_bytes) } + }); + // Also add the variable to Cargo's dependency tracking // // NOTE: This only works for passes run before `write_dep_info`. See that // for extension points for configuring environment variables to be // properly change-tracked. - tcx.sess.psess.env_depinfo.borrow_mut().insert((key, var)); - var + tcx.sess.psess.env_depinfo.borrow_mut().insert(( + Symbol::intern(&key.to_string_lossy()), + value.and_then(|value| value.to_str()).map(|value| Symbol::intern(&value)), + )); + + value_tcx } // Returns all the paths that correspond to generated files. @@ -707,7 +713,7 @@ pub static DEFAULT_QUERY_PROVIDERS: LazyLock = LazyLock::new(|| { |tcx, _| tcx.arena.alloc_from_iter(tcx.resolutions(()).stripped_cfg_items.steal()); providers.resolutions = |tcx, ()| tcx.resolver_for_lowering_raw(()).1; providers.early_lint_checks = early_lint_checks; - providers.env_var = env_var; + providers.env_var_os = env_var_os; proc_macro_decls::provide(providers); rustc_const_eval::provide(providers); rustc_middle::hir::provide(providers); diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 0f9c741afa0b2..c1529c6a425cd 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -6,6 +6,7 @@ #![allow(unused_parens)] +use std::ffi::OsStr; use std::mem; use std::ops::Deref; use std::path::PathBuf; @@ -130,9 +131,7 @@ rustc_queries! { /// NOTE: This currently does not work with dependency info in the /// analysis, codegen and linking passes, place extra code at the top of /// `rustc_interface::passes::write_dep_info` to make that work. - /// - /// Will emit an error and return `None` if the variable is not UTF-8. - query env_var(key: Symbol) -> Option { + query env_var_os(key: &'tcx OsStr) -> Option<&'tcx OsStr> { // Environment variables are global state eval_always desc { "get the value of an environment variable" } diff --git a/compiler/rustc_middle/src/util/mod.rs b/compiler/rustc_middle/src/util/mod.rs index 8dafc42264485..fd7ee89e84ac4 100644 --- a/compiler/rustc_middle/src/util/mod.rs +++ b/compiler/rustc_middle/src/util/mod.rs @@ -3,9 +3,14 @@ pub mod call_kind; pub mod common; pub mod find_self_call; +use std::env::VarError; +use std::ffi::OsStr; + pub use call_kind::{CallDesugaringKind, CallKind, call_kind}; pub use find_self_call::find_self_call; +use crate::ty::TyCtxt; + #[derive(Default, Copy, Clone)] pub struct Providers { pub queries: rustc_middle::query::Providers, @@ -29,3 +34,13 @@ impl std::ops::Deref for Providers { &self.queries } } + +impl<'tcx> TyCtxt<'tcx> { + pub fn env_var(self, key: &'tcx OsStr) -> Result<&'tcx str, VarError> { + if let Some(value) = self.env_var_os(key.as_ref()) { + value.to_str().ok_or_else(|| VarError::NotUnicode(value.to_os_string())) + } else { + Err(VarError::NotPresent) + } + } +} From 60f18baf80563176ca2d836f8397844db6a77fbc Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 1 Dec 2024 11:55:46 +0100 Subject: [PATCH 5/5] Add environment variable tracking in places where it was convenient This won't work with Cargo's change tracking, but it should work with incremental. --- compiler/rustc_borrowck/src/nll.rs | 10 ++++++---- compiler/rustc_lint/src/non_local_def.rs | 6 ++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index be02e2f48dfd8..ab4212a2b2ebb 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -1,9 +1,9 @@ //! The entry point of the NLL borrow checker. +use std::io; use std::path::PathBuf; use std::rc::Rc; use std::str::FromStr; -use std::{env, io}; use polonius_engine::{Algorithm, Output}; use rustc_data_structures::fx::FxIndexMap; @@ -145,9 +145,11 @@ pub(crate) fn compute_regions<'a, 'tcx>( } if polonius_output { - let algorithm = - env::var("POLONIUS_ALGORITHM").unwrap_or_else(|_| String::from("Hybrid")); - let algorithm = Algorithm::from_str(&algorithm).unwrap(); + let algorithm = infcx + .tcx + .env_var("POLONIUS_ALGORITHM".as_ref()) + .unwrap_or_else(|_| "Hybrid".as_ref()); + let algorithm = Algorithm::from_str(algorithm).unwrap(); debug!("compute_regions: using polonius algorithm {:?}", algorithm); let _prof_timer = infcx.tcx.prof.generic_activity("polonius_analysis"); Some(Box::new(Output::compute(all_facts, algorithm, false))) diff --git a/compiler/rustc_lint/src/non_local_def.rs b/compiler/rustc_lint/src/non_local_def.rs index 3c33b2dd4789c..9863cf1cefbd3 100644 --- a/compiler/rustc_lint/src/non_local_def.rs +++ b/compiler/rustc_lint/src/non_local_def.rs @@ -105,8 +105,10 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { // determining if we are in a doctest context can't currently be determined // by the code itself (there are no specific attributes), but fortunately rustdoc // sets a perma-unstable env var for libtest so we just reuse that for now - let is_at_toplevel_doctest = - || self.body_depth == 2 && std::env::var("UNSTABLE_RUSTDOC_TEST_PATH").is_ok(); + let is_at_toplevel_doctest = || { + self.body_depth == 2 + && cx.tcx.env_var_os("UNSTABLE_RUSTDOC_TEST_PATH".as_ref()).is_some() + }; match item.kind { ItemKind::Impl(impl_) => {