Missing help when trying to enable stabilized library feature on stable

[jruderman]

Given the following code:

#![feature(discriminant_value)]
fn main() {
    let y = Some(1);
    println!("{:?}", std::mem::discriminant(&y));
}

The current output is:

error[E0554]: `#![feature]` may not be used on the stable release channel
 --> src/main.rs:1:1
  |
1 | #![feature(discriminant_value)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Ideally the output should look like:

error: ...
help: the feature `discriminant_value` has been stable since 1.21.0 and no longer requires an attribute to enable

(or it could be downgraded from error to warning)

#83722 fixed this for #![feature(min_const_generics)] but not for #![feature(discriminant_value)], somehow

[jyn514]

Hmm, this might be because it's a library feature and not a language feature?

[jyn514]

Mentoring instructions: include library features in this check:

rust/compiler/rustc_ast_passes/src/feature_gate.rs

Line 723 in 497ee32

let stable_since = lang_features

[chansuke]

@rustbot claim

[vishadGoyal]

Hi! I started working on this but didn't know I had to claim it first. It's my first time contributing to open source.
Anyway, so far I realized we need to combine these two Vectors instead of just using the declared_lang_features vector
the vectors

use of features

The remaining challenge is to combine two vectors with different data types.
Hope this helps @chansuke Best of luck!

[chansuke]

@vishadGoyal I appreciate your great work. You should work on this :)

[vishadGoyal]

@rustbot claim

[vishadGoyal]

Thanks a lot! I hope I didn't guilt you into doing this 😅 @chansuke

[vishadGoyal]

Hi I'm having trouble with debugging my changes.

I have made changes in https://github.com/rust-lang/rust/blob/497ee321af3b8496eaccd7af7b437f18bab81abf/compiler/rustc_ast_passes/src/feature_gate.rs

Then I'm running
./x.py build
build/x86_64-unknown-linux-gnu/stage1/bin/rustc test.rs

I'm not seeing the effect of my changes when I do this.

Also is there a way to debug this by attaching breakpoints? I don't think my approach of using the binary is the correct way to do this.

[jyn514]

@vishadGoyal this bit of code only runs on the beta and stable channels. Try setting channel = "beta" in config.toml.

Also is there a way to debug this by attaching breakpoints? I don't think my approach of using the binary is the correct way to do this.

You can set debug = true in config.toml and then run it under gdb if you like.

[vishadGoyal]

I tried this but it says unknown field channel

My config.toml

# Includes one of the default files in src/bootstrap/defaults
profile = "compiler"
changelog-seen = 2
channel = "beta"

The output on running ./x.py build

Updating only changed submodules
Submodules updated in 0.01 seconds
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
failed to parse TOML configuration 'config.toml': unknown field `channel`, expected one of `changelog-seen`, `build`, `install`, `llvm`, `rust`, `target`, `dist`, `profile` at line 1 column 1
Build completed unsuccessfully in 0:00:00

[jyn514]

@vishadGoyal it needs to be under one of the tables (I think maybe rust?). Look at config.toml.example, it will give an example.

[vishadGoyal]

Thanks
It should be under [rust]. I think the config.toml.example can be improved by adding indentation.

[jyn514]

Feel free to make a PR adding it :)

[vishadGoyal]

I'm facing a problem. declared_lang_features obtained from the Session object have a since property but not the declared_lib_features.

rust/compiler/rustc_ast_passes/src/feature_gate.rs

Line 709 in 547d937

let lang_features = &sess.features_untracked().declared_lang_features;

rust/compiler/rustc_feature/src/active.rs

Line 47 in 547d937

pub declared_lang_features: Vec<(Symbol, Span, Option<Symbol>)>,

rust/compiler/rustc_feature/src/active.rs

Line 49 in 547d937

pub declared_lib_features: Vec<(Symbol, Span)>,

I searched the code and found that a list of all lib features can be queried from the TyCtxt object like in

rust/compiler/rustc_passes/src/stability.rs

Line 964 in 547d937

let local_defined_features = tcx.lib_features().to_vec();

and

rust/compiler/rustc_passes/src/stability.rs

Line 972 in 547d937

check_features(&mut remaining_lib_features, tcx.defined_lib_features(cnum));

Is there a way to get the same data from the Session object? I couldn't find it.

[jyn514]

Is there a way to get the same data from the Session object? I couldn't find it.

@vishadGoyal I think it would be reasonable to add it to declared_lib_features :) the features are created in

rust/compiler/rustc_expand/src/config.rs

Line 193 in c7dbe7a

features.declared_lib_features.push((name, mi.span()));

[vishadGoyal]

The implementation there is to access the declared features in https://github.com/rust-lang/rust/tree/c7dbe7a830100c70d59994fd940bf75bb6e39b39/compiler/rustc_feature/src

and if the feature name doesn't match with any of these, it is assumed to be a lib feature, the stable since data is still not there.

However, I found the data in

rust/library/core/src/intrinsics.rs

Line 1864 in c7dbe7a

#[rustc_const_unstable(feature = "const_discriminant", issue = "69821")]

This is accessed via the HIR here,

rust/compiler/rustc_passes/src/lib_features.rs

Line 129 in c7dbe7a

tcx.hir().walk_attributes(&mut collector);

and exposed via this query

rust/compiler/rustc_middle/src/query/mod.rs

Line 1411 in c7dbe7a

query get_lib_features(_: ()) -> LibFeatures {

I have two questions:

1. Can this query only be run through a type context object (TyCtxt) or also through the Session object?
2. Is there a way to excess the attributes declared in the interface in library/core/src/intrinsics.rs and other such files without going via the HIR?

[jyn514]

@vishadGoyal yes, queries can only be run through a TyCtxt. No, I wouldn't expect so, there's the ast::Attributes but you need more than just a Session to access them.

What's wrong with recording the since field in Session like I suggested?

[vishadGoyal]

The data is not already present where the declared_lib_features is currently being built.
Its present in library/core/src/intrinsics.rs
I'm not sure how to access it from where you mentioned.

[jyn514]

@vishadGoyal it should be in the attribute you're currently looking at - see how it's done in

rust/compiler/rustc_passes/src/lib_features.rs

Line 36 in 1cd17ad

if let Some(stab_attr) = stab_attrs.iter().find(|stab_attr| attr.has_name(**stab_attr)) {

The language features are hard-coded into the compiler, but that doesn't mean the library features aren't available, they just have to be determined at runtime.

[vishadGoyal]

I logged the attribute being looked at

[Attribute { kind: Normal(AttrItem { path: Path { span: ../reproduce-88802.rs:1:4: 1:11 (#0), segments: [PathSegment { ident: feature#0, id: NodeId(4294967040), args: None }], tokens: None }, args: Delimited(DelimSpan { open: ../reproduce-88802.rs:1:11: 1:12 (#0), close: ../reproduce-88802.rs:1:30: 1:31 (#0) }, Parenthemetasis, TokenStream([(Token(Token { kind: Ident("discriminant_value", false), span: ../reproduce-88802.rs:1:12: 1:30 (#0) }), Alone)])), tokens: None }, Some(LazyTokenStream(AttrAnnotatedTokenStream([(Token(Token { kind: Pound, span: ../reproduce-88802.rs:1:1: 1:2 (#0) }), Joint), (Token(Token { kind: Not, span: ../reproduce-88802.rs:1:2: 1:3 (#0) }), Alone), (Delimited(DelimSpan { open: ../reproduce-88802.rs:1:3: 1:4 (#0), close: ../reproduce-88802.rs:1:31: 1:32 (#0) }, Bracket, AttrAnnotatedTokenStream([(Token(Token { kind: Ident("feature", false), span: ../reproduce-88802.rs:1:4: 1:11 (#0) }), Alone), (Delimited(DelimSpan { open: ../reproduce-88802.rs:1:11: 1:12 (#0), close: ../reproduce-88802.rs:1:30: 1:31 (#0) }, Paren, AttrAnnotatedTokenStream([(Token(Token { kind: Ident("discriminant_value", false), span: ../reproduce-88802.rs:1:12: 1:30 (#0) }), Alone)])), Alone)])), Alone)])))), id: AttrId(0), style: Inner, span: ../reproduce-88802.rs:1:1: 1:32 (#0) }]

I couldn't find any stability attribute in it or it's meta. The output when I printed the meta of this attr was

Some(MetaItem { path: Path { span: ../reproduce-88802.rs:1:4: 1:11 (#0), segments: [PathSegment { ident: feature#0, id: NodeId(4294967040), args: None }], tokens: None }, kind: List([MetaItem(MetaItem { path: Path { span: ../reproduce-88802.rs:1:12: 1:30 (#0), segments: [PathSegment { ident: discriminant_value#0, id: NodeId(4294967040), args: None }], tokens: None }, kind: Word, span: ../reproduce-88802.rs:1:12: 1:30 (#0) })]), span: ../reproduce-88802.rs:1:1: 1:32 (#0) })

[jyn514]

@vishadGoyal you're looking at the #[feature(...)] attribute. You want to look at the #[stable(...)] attribute.

Hmm, this may be tricky to do in parsing, since it's in another crate - I wonder if we could delay checking library features until later in the compiler, maybe to the pass where it checks if the feature is already stable? I remember @Mark-Simulacrum saying features were checked early to avoid stabilizing syntax by accident, but that shouldn't be an issue for library features: https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/Why.20are.20feature.20gates.20checked.20during.20parsing.3F

[vishadGoyal]

I'm new to this so can you explain why the treatment can be different for lang features and lib features?

[jyn514]

Library features can't affect the syntax passed to proc macros; language features can.

[vishadGoyal]

So, what should be the ideal behavior in this scenario?
If #![feature] is used with only lib features and not with a language feature, should the compiler still throw an error or just a warning for unnecessary_stable_feature_lint?
Or should we always throw an error if #![feature] is used in beta or stable channels?

[jyn514]

@vishadGoyal you should still give a hard error, but it should be later in the compiler after the TyCtxt has been constructed, not during parsing.

[vishadGoyal]

I've made the suggested changes. #89012

[jyn514]

Fixed in #89012.