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

Don't allow -Zunstable-options to take a value #133159

Merged
merged 2 commits into from
Nov 23, 2024

Conversation

Zalathar
Copy link
Contributor

Passing an explicit boolean value (-Zunstable-options=on, off etc.) sometimes appears to work, but actually puts the compiler into an unintended state where unstable options are still forbidden, but unstable values of some stable options are allowed.

This is a result of -Zunstable-options being checked in multiple different places, in slightly different ways. Fixing the checks in config::nightly_options to understand boolean values would be non-trivial, so for now it's easier to make things consistent by forbidding values in the -Z parser.


There were a few uses of this in tests, which happened to work because they were tests of unstable values.

The old name and comment suggest that this parser is only used for options
beginning with `no-`, which is mostly true but not entirely true.
Passing an explicit boolean value (`on`, `off` etc.) appears to work, but
actually puts the compiler into an unintended state where unstable _options_
are still forbidden, but unstable values of _some_ stable options are allowed.
@rustbot
Copy link
Collaborator

rustbot commented Nov 18, 2024

r? @compiler-errors

rustbot has assigned @compiler-errors.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 18, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Nov 18, 2024

Passing an explicit boolean value (-Zunstable-options=on, off etc.) sometimes appears to work, but actually puts the compiler into an unintended state where unstable options are still forbidden, but unstable values of some stable options are allowed.

I feel like that sounds intentional but is an abuse of the same flag, maybe worth introducing -Zunstable-option-values I wonder... Because wanting to allow an unstable cli flag (incl. all its possible values) isn't quite the same as allowing a stable cli flag to accept an unstable value I think.

@Kobzol
Copy link
Contributor

Kobzol commented Nov 18, 2024

We already have -Zallow-features:

.env("RUSTFLAGS", "-Zallow-features=")
for enumerating specific features, isn't that enough?

@jieyouxu
Copy link
Member

jieyouxu commented Nov 18, 2024

We already have -Zallow-features:

.env("RUSTFLAGS", "-Zallow-features=")

for enumerating specific features, isn't that enough?

See #123404 (comment), -Zallow-features affect all the #[feature(..)]s, but it doesn't affect rustc cli flags AFAIK. (To make it more confusing 😆). IOW, this is orthogonal to if compiler stable cli flags can or cannot take unstable values AFAIK.

EDIT: oh wait I think I see why this is confusing, because cargo's -Zallow-features= also gates cargo's unstable cli flags, right?

@Zalathar
Copy link
Contributor Author

but unstable values of some stable options are allowed

Also note that when I say “some” here, I really do mean some. All unstable values are checked by ad-hoc code, so whether a particular option's unstable values are enabled by yes depends on which of the two APIs that ad-hoc code calls. And we already have a mixture of both styles in the compiler.

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is strictly better than having an inconsistent weird remix of some unstable values being gated while others are not. So, cool.

@jieyouxu
Copy link
Member

Thanks!
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 21, 2024

📌 Commit 660246b has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 21, 2024
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Nov 22, 2024
…, r=jieyouxu

Don't allow `-Zunstable-options` to take a value

Passing an explicit boolean value (`-Zunstable-options=on`, `off` etc.) sometimes appears to work, but actually puts the compiler into an unintended state where unstable _options_ are still forbidden, but unstable values of _some_ stable options are allowed.

This is a result of `-Zunstable-options` being checked in multiple different places, in slightly different ways. Fixing the checks in `config::nightly_options` to understand boolean values would be non-trivial, so for now it's easier to make things consistent by forbidding values in the `-Z` parser.

---

There were a few uses of this in tests, which happened to work because they were tests of unstable values.
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 22, 2024
…mpiler-errors

Rollup of 9 pull requests

Successful merges:

 - rust-lang#129238 (Stabilize `Ipv6Addr::is_unique_local` and `Ipv6Addr::is_unicast_link_local`)
 - rust-lang#130867 (distinguish overflow and unimplemented in Step::steps_between)
 - rust-lang#131505 (use `confstr(_CS_DARWIN_USER_TEMP_DIR, ...)` as a `TMPDIR` fallback on Darwin)
 - rust-lang#132090 (Stop being so bail-y in candidate assembly)
 - rust-lang#133159 (Don't allow `-Zunstable-options` to take a value )
 - rust-lang#133215 (Fix missing submodule in `./x vendor`)
 - rust-lang#133286 (Re-delay a resolve `bug` related to `Self`-ctor in patterns)
 - rust-lang#133301 (Add code example for `wrapping_neg` method for signed integers)
 - rust-lang#133313 (Use arc4random of libc for RTEMS target)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 22, 2024
…mpiler-errors

Rollup of 9 pull requests

Successful merges:

 - rust-lang#129238 (Stabilize `Ipv6Addr::is_unique_local` and `Ipv6Addr::is_unicast_link_local`)
 - rust-lang#130867 (distinguish overflow and unimplemented in Step::steps_between)
 - rust-lang#131505 (use `confstr(_CS_DARWIN_USER_TEMP_DIR, ...)` as a `TMPDIR` fallback on Darwin)
 - rust-lang#132090 (Stop being so bail-y in candidate assembly)
 - rust-lang#133159 (Don't allow `-Zunstable-options` to take a value )
 - rust-lang#133215 (Fix missing submodule in `./x vendor`)
 - rust-lang#133286 (Re-delay a resolve `bug` related to `Self`-ctor in patterns)
 - rust-lang#133301 (Add code example for `wrapping_neg` method for signed integers)
 - rust-lang#133313 (Use arc4random of libc for RTEMS target)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request Nov 22, 2024
…, r=jieyouxu

Don't allow `-Zunstable-options` to take a value

Passing an explicit boolean value (`-Zunstable-options=on`, `off` etc.) sometimes appears to work, but actually puts the compiler into an unintended state where unstable _options_ are still forbidden, but unstable values of _some_ stable options are allowed.

This is a result of `-Zunstable-options` being checked in multiple different places, in slightly different ways. Fixing the checks in `config::nightly_options` to understand boolean values would be non-trivial, so for now it's easier to make things consistent by forbidding values in the `-Z` parser.

---

There were a few uses of this in tests, which happened to work because they were tests of unstable values.
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 22, 2024
Rollup of 13 pull requests

Successful merges:

 - rust-lang#130867 (distinguish overflow and unimplemented in Step::steps_between)
 - rust-lang#131859 (Update TRPL to add new Chapter 17: Async and Await)
 - rust-lang#132090 (Stop being so bail-y in candidate assembly)
 - rust-lang#132658 (Detect const in pattern with typo)
 - rust-lang#133041 (Print name of env var in `--print=deployment-target`)
 - rust-lang#133102 (aarch64 softfloat target: always pass floats in int registers)
 - rust-lang#133159 (Don't allow `-Zunstable-options` to take a value )
 - rust-lang#133217 ([AIX] Add option -X32_64 to the "strip" command)
 - rust-lang#133237 (Minimally constify `Add`)
 - rust-lang#133238 (re-export `is_loongarch_feature_detected`)
 - rust-lang#133286 (Re-delay a resolve `bug` related to `Self`-ctor in patterns)
 - rust-lang#133301 (Add code example for `wrapping_neg` method for signed integers)
 - rust-lang#133313 (Use arc4random of libc for RTEMS target)

Failed merges:

 - rust-lang#133215 (Fix missing submodule in `./x vendor`)

r? `@ghost`
`@rustbot` modify labels: rollup
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Nov 22, 2024
…, r=jieyouxu

Don't allow `-Zunstable-options` to take a value

Passing an explicit boolean value (`-Zunstable-options=on`, `off` etc.) sometimes appears to work, but actually puts the compiler into an unintended state where unstable _options_ are still forbidden, but unstable values of _some_ stable options are allowed.

This is a result of `-Zunstable-options` being checked in multiple different places, in slightly different ways. Fixing the checks in `config::nightly_options` to understand boolean values would be non-trivial, so for now it's easier to make things consistent by forbidding values in the `-Z` parser.

---

There were a few uses of this in tests, which happened to work because they were tests of unstable values.
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Nov 22, 2024
…, r=jieyouxu

Don't allow `-Zunstable-options` to take a value

Passing an explicit boolean value (`-Zunstable-options=on`, `off` etc.) sometimes appears to work, but actually puts the compiler into an unintended state where unstable _options_ are still forbidden, but unstable values of _some_ stable options are allowed.

This is a result of `-Zunstable-options` being checked in multiple different places, in slightly different ways. Fixing the checks in `config::nightly_options` to understand boolean values would be non-trivial, so for now it's easier to make things consistent by forbidding values in the `-Z` parser.

---

There were a few uses of this in tests, which happened to work because they were tests of unstable values.
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 23, 2024
…mpiler-errors

Rollup of 8 pull requests

Successful merges:

 - rust-lang#131505 (use `confstr(_CS_DARWIN_USER_TEMP_DIR, ...)` as a `TMPDIR` fallback on Darwin)
 - rust-lang#131859 (Update TRPL to add new Chapter 17: Async and Await)
 - rust-lang#132090 (Stop being so bail-y in candidate assembly)
 - rust-lang#132597 (btree: don't leak value if destructor of key panics)
 - rust-lang#132911 (Pretty print async fn sugar in opaques and trait bounds)
 - rust-lang#133159 (Don't allow `-Zunstable-options` to take a value )
 - rust-lang#133247 (Reduce integer `Display` implementation size)
 - rust-lang#133286 (Re-delay a resolve `bug` related to `Self`-ctor in patterns)

Failed merges:

 - rust-lang#133215 (Fix missing submodule in `./x vendor`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 23, 2024
…mpiler-errors

Rollup of 8 pull requests

Successful merges:

 - rust-lang#132090 (Stop being so bail-y in candidate assembly)
 - rust-lang#132658 (Detect const in pattern with typo)
 - rust-lang#132911 (Pretty print async fn sugar in opaques and trait bounds)
 - rust-lang#133102 (aarch64 softfloat target: always pass floats in int registers)
 - rust-lang#133159 (Don't allow `-Zunstable-options` to take a value )
 - rust-lang#133208 (generate-copyright: Now generates a library file too.)
 - rust-lang#133215 (Fix missing submodule in `./x vendor`)
 - rust-lang#133264 (implement OsString::truncate)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bb92131 into rust-lang:master Nov 23, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Nov 23, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 23, 2024
Rollup merge of rust-lang#133159 - Zalathar:unstable-options-no-value, r=jieyouxu

Don't allow `-Zunstable-options` to take a value

Passing an explicit boolean value (`-Zunstable-options=on`, `off` etc.) sometimes appears to work, but actually puts the compiler into an unintended state where unstable _options_ are still forbidden, but unstable values of _some_ stable options are allowed.

This is a result of `-Zunstable-options` being checked in multiple different places, in slightly different ways. Fixing the checks in `config::nightly_options` to understand boolean values would be non-trivial, so for now it's easier to make things consistent by forbidding values in the `-Z` parser.

---

There were a few uses of this in tests, which happened to work because they were tests of unstable values.
@Zalathar Zalathar deleted the unstable-options-no-value branch November 23, 2024 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants