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 require allow_internal_unstable unless staged_api is enabled. #68114

Merged
merged 4 commits into from
Jan 11, 2020

Conversation

ecstatic-morse
Copy link
Contributor

@ecstatic-morse ecstatic-morse commented Jan 11, 2020

#63770 changed qualify_min_const_fn to require allow_internal_unstable for all crates that used an unstable feature, regardless of whether staged_api was enabled or the fn that used that feature was stably const. In practice, this meant that every crate in the ecosystem that wanted to use nightly features added #![feature(const_fn)], which skips qualify_min_const_fn entirely.

After this PR, crates that do not have #![feature(staged_api)] will only need to enable the feature they are interested in. For example, #![feature(const_if_match)] will be enough to enable if and match in constants. Crates with staged_api (e.g., libstd) require #[allow_internal_unstable] to be added to a function if it uses nightly features unless that function is also marked #[rustc_const_unstable]. This prevents proliferation of #[allow_internal_unstable] into functions that are not callable in a const context on stable.

r? @oli-obk (author of #63770)
cc @Centril

This flag opts out of the min-const-fn checks entirely, which is usually
not what we want. The few cases where the flag is still necessary have
been annotated.
@Centril
Copy link
Contributor

Centril commented Jan 11, 2020

r=me with comment ^-- added.

@Centril
Copy link
Contributor

Centril commented Jan 11, 2020

If you can follow up with changes to the rustc guide that'd be great.

Co-Authored-By: Mazdak Farrokhzad <twingoow@gmail.com>
@ecstatic-morse
Copy link
Contributor Author

I'll r+ on your behalf once this passes CI. There might be some bootstrap problems now that the feature is required to be at the crate root as well as in allow_internal_unstable.

@ecstatic-morse
Copy link
Contributor Author

@bors r=Centril

@bors
Copy link
Contributor

bors commented Jan 11, 2020

📌 Commit fc30825 has been approved by Centril

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 11, 2020
Centril added a commit to Centril/rust that referenced this pull request Jan 11, 2020
…r=Centril

Don't require `allow_internal_unstable` unless `staged_api` is enabled.

rust-lang#63770 changed `qualify_min_const_fn` to require `allow_internal_unstable` for *all* crates that used an unstable feature, regardless of whether `staged_api` was enabled or the `fn` that used that feature was stably const. In practice, this meant that every crate in the ecosystem that wanted to use nightly features added `#![feature(const_fn)]`, which skips `qualify_min_const_fn` entirely.

After this PR, crates that do not have `#![feature(staged_api)]` will only need to enable the feature they are interested in. For example, `#![feature(const_if_match)]` will be enough to enable `if` and `match` in constants. Crates with `staged_api` (e.g., `libstd`) require `#[allow_internal_unstable]` to be added to a function if it uses nightly features unless that function is also marked `#[rustc_const_unstable]`. This prevents proliferation of `#[allow_internal_unstable]` into functions that are not callable in a `const` context on stable.

r? @oli-obk (author of rust-lang#63770)
cc @Centril
Centril added a commit to Centril/rust that referenced this pull request Jan 11, 2020
…r=Centril

Don't require `allow_internal_unstable` unless `staged_api` is enabled.

rust-lang#63770 changed `qualify_min_const_fn` to require `allow_internal_unstable` for *all* crates that used an unstable feature, regardless of whether `staged_api` was enabled or the `fn` that used that feature was stably const. In practice, this meant that every crate in the ecosystem that wanted to use nightly features added `#![feature(const_fn)]`, which skips `qualify_min_const_fn` entirely.

After this PR, crates that do not have `#![feature(staged_api)]` will only need to enable the feature they are interested in. For example, `#![feature(const_if_match)]` will be enough to enable `if` and `match` in constants. Crates with `staged_api` (e.g., `libstd`) require `#[allow_internal_unstable]` to be added to a function if it uses nightly features unless that function is also marked `#[rustc_const_unstable]`. This prevents proliferation of `#[allow_internal_unstable]` into functions that are not callable in a `const` context on stable.

r? @oli-obk (author of rust-lang#63770)
cc @Centril
bors added a commit that referenced this pull request Jan 11, 2020
Rollup of 9 pull requests

Successful merges:

 - #67000 (Promote references to constants instead of statics)
 - #67756 (Collector tweaks)
 - #67889 (Compile some CGUs in parallel at the start of codegen)
 - #67930 (Rename Result::as_deref_ok to as_deref)
 - #68018 (feature_gate: Remove `GateStrength`)
 - #68070 (clean up E0185 explanation)
 - #68072 (Fix ICE #68058)
 - #68114 (Don't require `allow_internal_unstable` unless `staged_api` is enabled.)
 - #68120 (Ban `...X` pats, harden tests, and improve diagnostics)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Jan 11, 2020
…r=Centril

Don't require `allow_internal_unstable` unless `staged_api` is enabled.

rust-lang#63770 changed `qualify_min_const_fn` to require `allow_internal_unstable` for *all* crates that used an unstable feature, regardless of whether `staged_api` was enabled or the `fn` that used that feature was stably const. In practice, this meant that every crate in the ecosystem that wanted to use nightly features added `#![feature(const_fn)]`, which skips `qualify_min_const_fn` entirely.

After this PR, crates that do not have `#![feature(staged_api)]` will only need to enable the feature they are interested in. For example, `#![feature(const_if_match)]` will be enough to enable `if` and `match` in constants. Crates with `staged_api` (e.g., `libstd`) require `#[allow_internal_unstable]` to be added to a function if it uses nightly features unless that function is also marked `#[rustc_const_unstable]`. This prevents proliferation of `#[allow_internal_unstable]` into functions that are not callable in a `const` context on stable.

r? @oli-obk (author of rust-lang#63770)
cc @Centril
bors added a commit that referenced this pull request Jan 11, 2020
Rollup of 8 pull requests

Successful merges:

 - #67756 (Collector tweaks)
 - #67889 (Compile some CGUs in parallel at the start of codegen)
 - #67930 (Rename Result::as_deref_ok to as_deref)
 - #68018 (feature_gate: Remove `GateStrength`)
 - #68070 (clean up E0185 explanation)
 - #68072 (Fix ICE #68058)
 - #68114 (Don't require `allow_internal_unstable` unless `staged_api` is enabled.)
 - #68120 (Ban `...X` pats, harden tests, and improve diagnostics)

Failed merges:

r? @ghost
@bors bors merged commit fc30825 into rust-lang:master Jan 11, 2020
@nbdd0121
Copy link
Contributor

Note that currently there is a workaround using macros:

#![feature(allow_internal_unstable)]

#[allow_internal_unstable(const_transmute)]
macro_rules! transmute {
    ($e: expr) => {
        std::mem::transmute($e)
    };
}

#[allow_internal_unstable(wrapping_next_power_of_two)]
macro_rules! wrapping_next_power_of_two {
    ($e: expr) => {
        $e.wrapping_next_power_of_two()
    };
}


fn g(a: u64) -> u64 {
    wrapping_next_power_of_two!(a)
}

const fn h(a: u64) -> i64 {
    unsafe { transmute!(a) }
}

@ecstatic-morse ecstatic-morse deleted the fix-feature-gating branch October 6, 2020 01:42
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants