Rust: #[allow_internal_unstable] does not allow unstable library features when applied on functions

[kennytm]

The #[allow_internal_unstable] attribute can only enable compiler features. Library features cannot be enabled.

Examples:

#![feature(allow_internal_unstable)]
// #![feature(wrapping_next_power_of_two, const_transmute)]

#[allow_internal_unstable(wrapping_next_power_of_two)]
fn g(a: u64) -> u64 {
    a.wrapping_next_power_of_two()
}

#[allow_internal_unstable(const_transmute)]
const fn h(a: u64) -> i64 {
    unsafe { std::mem::transmute(a) }
}

• Expected: can be built without errors.
• Actual: still complains about missing #![feature]s.

Rustc version = 1.43.0-nightly (2020-02-22 436494b8f8008b600d64)

[eddyb]

allow_internal_unstable is a macro attribute, the only bug here is that it doesn't complain when you put it on a non-macro.

[ecstatic-morse]

@eddyb This was changed as part of #63770.

[eddyb]

Ugh, that's an unprincipled hack, and the attribute is still not checked correctly. I had this example where it doesn't do anything without errors.

I can now see how it's used in libcore, but it should be named #[rustc_allow_const_fn_unstable(...)] or something, and be properly checked.

[eddyb]

Also, do we need that anymore? Given that we're starting to add more and more const fns.

[RalfJung]

Also, do we need that anymore? Given that we're starting to add more and more const fns.

The point of this is that we don't want to stabilize const fn that use unstable features (to avoid de-facto stabilizing unstable features of the const-eval engine). I think that is still very much needed.

[kennytm]

cc @Centril @oli-obk for use of #[allow_internal_unstable] around const fn. https://github.com/rust-lang/rustc-guide/blob/master/src/stability.md#allow_internal_unstable.

[ecstatic-morse]

The fix for this is described in https://github.com/rust-lang/rust/pull/69373#issuecomment-591366565.

[liketechnik]

I would like to tackle this issue, but I'm not 100% sure how to go forward. If I understood correctly, I would just need to follow the instructions in the above fix.
This would mean I'd add the described check at the described line, right?

[oli-obk]

I believe that should indeed suffice to fix the issue, but I may have overlooked larger things

[liketechnik]

I don't think I'll be able to fix this issue on my own, I'm afraid.

I'm not familiar enough with the internal workings and the fix linked above cannot really be used like that anymore, if I understood the code correctly.

[oli-obk]

https://github.com/rust-lang/rust/blob/62bfcfd8a3b1603e2b5f5e2c011aa0f35f117af1/compiler/rustc_mir/src/transform/check_consts/validation.rs#L802-L828 is what handles this now.

So I guess we'll need to just remove https://github.com/rust-lang/rust/blob/62bfcfd8a3b1603e2b5f5e2c011aa0f35f117af1/compiler/rustc_mir/src/transform/check_consts/validation.rs#L807-L812

[liketechnik]

I applied that change locally and it didn't fix the issue.

I think the error lies elsewhere, because compiling the above code with -Z treat-err-as-bug=1 and RUST_BACKTRACE=full creates a backtrace where the error seems to be created in rustc_middle::middle::stability::check_optional_stability (here's a gist with the code and the backtrace).

Edit: https://github.com/rust-lang/rust/blob/e1d76818b2b505e88bcd0c965da945e7e2e4329e/compiler/rustc_middle/src/middle/stability.rs#L409-L428

[oli-obk]

oh.. oof. This means it's part of https://github.com/rust-lang/rust/blob/d902752866cbbdb331e3cf28ff6bba86ab0f6c62/compiler/rustc_middle/src/middle/stability.rs#L296 which... we don't really want to touch. That would allow us to allow allow arbitrary library features. I now realize that this issue was created for non-const-fn, I always thought this was about const fn and relevant features.

Yea, I don't think we should change anything here and keep requiring the feature on the entire crate.

So the only change left from this issue that I think we should address is to rename the attribute on functions to rustc_allow_const_fn_unstable and not have the same name as the attribute for macros.

[liketechnik]

My current understanding of what needs to be done to address the renaming:

• Add rustc_allow_const_fn_unstable to the internal attribute list https://github.com/rust-lang/rust/blob/9832374f6e378971e1a933362cf9781b121bb845/compiler/rustc_feature/src/builtin_attrs.rs#L174, the list of active feature https://github.com/rust-lang/rust/blob/9832374f6e378971e1a933362cf9781b121bb845/compiler/rustc_feature/src/active.rs#L95 and the symbol list https://github.com/rust-lang/rust/blob/9832374f6e378971e1a933362cf9781b121bb845/compiler/rustc_span/src/symbol.rs#L22
• Include feature names from both (either) allow_internal_unstable and rustc_allow_const_fn_unstable https://github.com/rust-lang/rust/blob/9832374f6e378971e1a933362cf9781b121bb845/compiler/rustc_attr/src/builtin.rs#L1012-L1016
• Make sure that allow_internal_unstable can only be applied to macros (which would happen in compiler/rustc_passes/src/check_attr.rs?)
• Make sure that rustc_allow_const_fn_unstable can only be applied to functions

(Note: If somebody more experienced than me should rather deal with this, please tell me. I'm really enjoying working on this so far, but I also understand if I'm too much of a burden with that many (simple) questions...)

[oli-obk]

(Note: If somebody more experienced than me should rather deal with this, please tell me. I'm really enjoying working on this so far, but I also understand if I'm too much of a burden with that many (simple) questions...)

From my end it looks like you are approaching this in a well-organized manner, and not at all a burden!

* Include feature names from both (either) `allow_internal_unstable` and `rustc_allow_const_fn_unstable`

I would make that function private and take a Symbol argument that gets checked and reported instead. Then you can add public wrappers for both allow_internal_unstable and rustc_allow_const_fn_unstable that fill in that new argument with the appropriate symbol. This way we don't mix the two functions while still deduplicating their implementation.

• Make sure that allow_internal_unstable can only be applied to macros (which would happen in compiler/rustc_passes/src/check_attr.rs?)

• Make sure that rustc_allow_const_fn_unstable can only be applied to functions

Yes, both of these can be checked similarly to
https://github.com/rust-lang/rust/blob/master/compiler/rustc_passes/src/check_attr.rs#L86-L87