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

kennytm · 2020-02-23T14:31:54Z

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)

The text was updated successfully, but these errors were encountered:

eddyb · 2020-02-24T01:56:42Z

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 · 2020-02-24T22:32:36Z

@eddyb This was changed as part of #63770.

eddyb · 2020-02-25T08:42:41Z

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 · 2020-02-25T08:46:36Z

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

RalfJung · 2020-02-25T10:15:30Z

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 · 2020-02-25T10:48:58Z

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 · 2020-02-26T20:36:15Z

The fix for this is described in #69373 (comment).

liketechnik · 2020-10-19T13:44:39Z

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 · 2020-10-20T08:58:38Z

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

liketechnik · 2020-10-20T14:13:23Z

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 · 2020-10-20T14:22:51Z

rust/compiler/rustc_mir/src/transform/check_consts/validation.rs

Lines 802 to 828 in 62bfcfd

 if let Some(gate) = is_unstable_const_fn(tcx, callee) { 

 if self.span.allows_unstable(gate) { 

 return; 

 } 

 // Calling an unstable function *always* requires that the corresponding gate 

 // be enabled, even if the function has `#[allow_internal_unstable(the_gate)]`. 

 if !tcx.features().declared_lib_features.iter().any(|&(sym, _)| sym == gate) { 

 self.check_op(ops::FnCallUnstable(callee, Some(gate))); 

 return; 

 } 

 // If this crate is not using stability attributes, or the caller is not claiming to be a 

 // stable `const fn`, that is all that is required. 

 if !self.ccx.is_const_stable_const_fn() { 

 return; 

 } 

 // Otherwise, we are something const-stable calling a const-unstable fn. 

 if super::allow_internal_unstable(tcx, caller, gate) { 

 return; 

 } 

 self.check_op(ops::FnCallUnstable(callee, Some(gate))); 

 return; 

 }

is what handles this now.

So I guess we'll need to just remove

rust/compiler/rustc_mir/src/transform/check_consts/validation.rs

Lines 807 to 812 in 62bfcfd

 // Calling an unstable function *always* requires that the corresponding gate 

 // be enabled, even if the function has `#[allow_internal_unstable(the_gate)]`. 

 if !tcx.features().declared_lib_features.iter().any(|&(sym, _)| sym == gate) { 

 self.check_op(ops::FnCallUnstable(callee, Some(gate))); 

 return; 

 }

liketechnik · 2020-10-20T14:39:03Z

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:

rust/compiler/rustc_middle/src/middle/stability.rs

Lines 409 to 428 in e1d7681

 pub fn check_optional_stability( 

 self, 

 def_id: DefId, 

 id: Option<HirId>, 

 span: Span, 

 unmarked: impl FnOnce(Span, DefId) -> (), 

 ) { 

 let soft_handler = |lint, span, msg: &_| { 

 self.struct_span_lint_hir(lint, id.unwrap_or(hir::CRATE_HIR_ID), span, |lint| { 

 lint.build(msg).emit() 

 }) 

 }; 

 match self.eval_stability(def_id, id, span) { 

 EvalResult::Allow => {} 

 EvalResult::Deny { feature, reason, issue, is_soft } => { 

 report_unstable(self.sess, feature, reason, issue, is_soft, span, soft_handler) 

 } 

 EvalResult::Unmarked => unmarked(span, def_id), 

 } 

 }

oli-obk · 2020-10-20T14:53:22Z

oh.. oof. This means it's part of

rust/compiler/rustc_middle/src/middle/stability.rs

Line 296 in d902752

 pub fn eval_stability(self, def_id: DefId, id: Option<HirId>, span: Span) -> EvalResult { 

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 · 2020-10-20T18:57:04Z

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

Add rustc_allow_const_fn_unstable to the internal attribute list

rust/compiler/rustc_feature/src/builtin_attrs.rs

Line 174 in 9832374

pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[

, the list of active feature

rust/compiler/rustc_feature/src/active.rs

Line 95 in 9832374

declare_features! (

and the symbol list

rust/compiler/rustc_span/src/symbol.rs

Line 22 in 9832374

symbols! {

Include feature names from both (either) allow_internal_unstable and rustc_allow_const_fn_unstable

rust/compiler/rustc_attr/src/builtin.rs

Lines 1012 to 1016 in 9832374

 pub fn allow_internal_unstable<'a>( 

 sess: &'a Session, 

 attrs: &'a [Attribute], 

 ) -> Option<impl Iterator<Item = Symbol> 'a> { 

 let attrs = sess.filter_by_name(attrs, sym::allow_internal_unstable);

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 · 2020-10-21T08:17:59Z

(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

allow_internal_unstable is currently used to side-step feature gate and stability checks. While it was originally only meant to be used only on macros, its use was expanded to const functions. This commit prepares stricter checks for the usage of allow_internal_unstable (only on macros) and introduces the rustc_allow_const_fn_unstable attribute for usage on functions. See rust-lang#69399

kennytm added A-stability Area: issues related to #[stable] and #[unstable] attributes themselves. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Feb 23, 2020

kennytm mentioned this issue Feb 23, 2020

Stabilize const for integer {to,from}_{be,le,ne}_bytes methods #69373

Merged

RalfJung added the A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. label Feb 23, 2020

kennytm added the A-attributes Area: #[attributes(..)] label Feb 24, 2020

kennytm changed the title ~~#[allow_internal_unstable] does not allow unstable library features~~ #[allow_internal_unstable] does not allow unstable library features when applied on functions Feb 27, 2020

oli-obk mentioned this issue Jun 27, 2020

Make likely and unlikely const, gated by feature const_unlikely #73778

Merged

varkor added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Sep 28, 2020

varkor mentioned this issue Sep 28, 2020

allow_internal_unstable silently ignores typos in feature names #77226

Open

oli-obk removed the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Oct 20, 2020

liketechnik mentioned this issue Oct 21, 2020

replace #[allow_internal_unstable] with #[rustc_allow_const_fn_unstable] for const fns #78208

Merged

bors closed this as completed in 72e02b0 Oct 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

kennytm commented Feb 23, 2020

eddyb commented Feb 24, 2020

ecstatic-morse commented Feb 24, 2020

eddyb commented Feb 25, 2020

eddyb commented Feb 25, 2020

RalfJung commented Feb 25, 2020

kennytm commented Feb 25, 2020

ecstatic-morse commented Feb 26, 2020

liketechnik commented Oct 19, 2020 •

edited

Loading

oli-obk commented Oct 20, 2020

liketechnik commented Oct 20, 2020

oli-obk commented Oct 20, 2020

liketechnik commented Oct 20, 2020 •

edited

Loading

oli-obk commented Oct 20, 2020

liketechnik commented Oct 20, 2020

oli-obk commented Oct 21, 2020

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

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

Comments

kennytm commented Feb 23, 2020

eddyb commented Feb 24, 2020

ecstatic-morse commented Feb 24, 2020

eddyb commented Feb 25, 2020

eddyb commented Feb 25, 2020

RalfJung commented Feb 25, 2020

kennytm commented Feb 25, 2020

ecstatic-morse commented Feb 26, 2020

liketechnik commented Oct 19, 2020 • edited Loading

oli-obk commented Oct 20, 2020

liketechnik commented Oct 20, 2020

oli-obk commented Oct 20, 2020

liketechnik commented Oct 20, 2020 • edited Loading

oli-obk commented Oct 20, 2020

liketechnik commented Oct 20, 2020

oli-obk commented Oct 21, 2020

liketechnik commented Oct 19, 2020 •

edited

Loading

liketechnik commented Oct 20, 2020 •

edited

Loading