Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Probable bug with the output of option_env!("NON_UNICODE_ENV_VAR") #122669

Closed
beetrees opened this issue Mar 18, 2024 · 1 comment · Fixed by #122670
Closed

Probable bug with the output of option_env!("NON_UNICODE_ENV_VAR") #122669

beetrees opened this issue Mar 18, 2024 · 1 comment · Fixed by #122670
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@beetrees
Copy link
Contributor

beetrees commented Mar 18, 2024

I tried this code:

fn main() {
    dbg!(option_env!("NON_UNICODE_ENV_VAR"));
}

compiler with NON_UNICODE_ENV_VAR=$'\xFF' rustc code.rs (using Bash shell syntax, any non-Unicode environment variable value for NON_UNICODE_ENV_VAR will work).

I expected to see this happen: a Some value, as the documentation currently states "If the named environment variable is present at compile time, this will expand into an expression of type Option<&'static str> whose value is Some of the value of the environment variable".

Instead, this happened: The program outputted:

[code.rs:2:2] option_env!("NON_UNICODE_ENV_VAR") = None

Currently, the documentation doesn't consider the possibility of a non-Unicode environment variable value (as only Unicode values can be expressed as a &'static str), and the implementation emits None, the same as for a non-existent environment variable.

There are three possibilities that I can think of:

  1. The current behaviour is a bug, and a compilation error should be emitted as it isn't possibly to represent a non-Unicode value as a &'static str, and the documentation should also be updated to reflect this. Note that this would be a breaking change to the compiler itself, but would be allowed as it would be a bugfix.
  2. The current behaviour is not a bug, and the documentation should be updated to reflect the current behaviour.
  3. Same as 2, but the behaviour is unexpected enough to warrant a warn/deny-by-default lint.

Personally, I believe 1 to be the case, as the current behaviour is unexpected and leads to code patterns like let was_defined_at_compile_time = option_env!("VAR").is_some() silently returning incorrect results. There doesn't seem to be a "correct" value to return, as None is already taken by "does not exist" and Some(&str) cannot represent a non-Unicode value. It seems unlikely anyone is relying on the current behaviour given Rust's very UTF-8 everywhere design, the fact the same result can be achieved much more easily by just not setting the environment variable, and the platform-specific nature of what non-Unicode values can exist (Windows UTF-16 unpaired surrogates vs. Unix invalid UTF-8 bytes). It therefore seems to be more likely to occur when someone has mistyped something in their shell or due to data corruption, which the current output would make rather confusing to track down (as it would seem to say the variable was missing, rather than set incorrectly).

Meta

rustc --version --verbose:

commit-hash: 07dca489ac2d933c78d3c5158e3f43beefeb02ce
commit-date: 2024-02-04
host: x86_64-unknown-linux-gnu
release: 1.76.0
LLVM version: 17.0.6

I believe the decision of what to do is down to libs-api, so

@rustbot label +T-libs-api

@beetrees beetrees added the C-bug Category: This is a bug. label Mar 18, 2024
@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 18, 2024
@beetrees
Copy link
Contributor Author

I've created a draft implementation of option 1 in #122670.

@beetrees beetrees changed the title Probable bug with output of option_env!("NON_UNICODE_ENV_VAR") Probable bug with the output of option_env!("NON_UNICODE_ENV_VAR") Mar 18, 2024
@saethlin saethlin added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 25, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 14, 2024
…ror, r=compiler-errors

Fix bug where `option_env!` would return `None` when env var is present but not valid Unicode

Fixes rust-lang#122669 by making `option_env!` emit an error when the value of the environment variable is not valid Unicode.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 14, 2024
…ror, r=compiler-errors

Fix bug where `option_env!` would return `None` when env var is present but not valid Unicode

Fixes rust-lang#122669 by making `option_env!` emit an error when the value of the environment variable is not valid Unicode.
compiler-errors added a commit to compiler-errors/rust that referenced this issue Oct 15, 2024
…ror, r=compiler-errors

Fix bug where `option_env!` would return `None` when env var is present but not valid Unicode

Fixes rust-lang#122669 by making `option_env!` emit an error when the value of the environment variable is not valid Unicode.
@bors bors closed this as completed in 6d99996 Oct 15, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 15, 2024
Rollup merge of rust-lang#122670 - beetrees:non-unicode-option-env-error, r=compiler-errors

Fix bug where `option_env!` would return `None` when env var is present but not valid Unicode

Fixes rust-lang#122669 by making `option_env!` emit an error when the value of the environment variable is not valid Unicode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
3 participants