Repository navigation
Conversation
This comment has been minimized.
This comment has been minimized.
|
How does this differ from the (also your tracking issue link is wrong) |
80ee038 to
54cd284
Compare
This was something @oli-obk talked about during his talk at Eurorust 2026! I modified the passing ui test to showcase what you can do with it. It basically allows you to chose dynamically at runtime which variable you are reading. And fixed the tracking issue. Thanks! |
| sym::type_id_env_var => { | ||
| let arg_place = ecx.deref_pointer(&args[0])?; | ||
| let key = ecx.read_str(&arg_place)?; | ||
| let value = std::env::var_os(&key); |
There was a problem hiding this comment.
Reading from environment will result in inconsistent evaluation, which is unsound.
There was a problem hiding this comment.
The env! and option_env! macros already allow doing the same. Main difference is that this allows you to dynamically choose which variable you would be reading from.
Is there different properties with that change that makes this unsound? Or is it something different you are talking about?
There was a problem hiding this comment.
Those are quite different. In the one case the environment is consulted at macro expansion time, in the other during constant evaluation.
Constant evaluation must be deterministic, including across different compiler sessions with different environment.
There was a problem hiding this comment.
jup, my hope was that we were already doing sth sane wrt env vars during macro expansion, but we probably are just relying on macro expansion happening regardless of query caching.
So, what we also need to do here is to add a query that is eval_always, which does the env var reading, and then call that query from here. This way we re-read the env every time, and if it changes, invalidate every part of the compiler that depended on the change.
There was a problem hiding this comment.
The evaluation must be consistent across different crates. It would have to capture a complete environment, and then query would have to identify which crate the environment is for.
The capture of complete environment sounds like a bad idea to me, and as far as I see, this is what this functionality implies.
This comment has been minimized.
This comment has been minimized.
54cd284 to
2c5bdaa
Compare
This comment has been minimized.
This comment has been minimized.
To avoid having change maybe ui tests, rename to const_env. The compiler tries to find matching `env` modules when you use one without important it. It then starts listing the one in type_info too.
|
The job Click to see the possible cause of the failure (guessed by this bot) |
| #[rustc_intrinsic] | ||
| #[unstable(feature = "core_intrinsics", issue = "none")] | ||
| #[rustc_comptime] | ||
| pub fn type_id_env_var(_env_name: &str) -> Option<&'static [u8]>; |
There was a problem hiding this comment.
since this is not a TypeId based query, we should give it a different prefix. Or just none, just having it be the env_var intrinsic seems fine
| /// Temporary feature, part of comptime. Not really type info related | ||
| pub mod const_env { | ||
| /// Returns the raw bytes of an environment variable that is visible to the compiler | ||
| /// while compile-time constant evaluation. |
There was a problem hiding this comment.
| /// while compile-time constant evaluation. | |
| /// during constant evaluation. |
|
Reminder, once the PR becomes ready for a review, use |
Tracking issue: #146922
This is a first draft for allowing people to read environment variables dynamically in const evaluation context.
For now, this allows reading a variable as a &[u8] from any const context. Since
core::str::from_utf8is const, it should be easy to wrap that to have an interface that looks a tad likestd::env::var*but for comptime.Haven't really thought about how to organize this yet. But we might want a
core::(reflection/comptime)::envor something equivalent to store those functions. Maybe they could also just sit in acore::env::comptime_varbut that feels weirdr? @oli-obk