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.

Sign up for GitHub

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

Jump to bottom

Re-enable detection of unused library #![feature] directives #44232

Open
alexcrichton opened this issue Aug 31, 2017 · 7 comments
Open

Re-enable detection of unused library #![feature] directives #44232

alexcrichton opened this issue Aug 31, 2017 · 7 comments
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

Currently the compiler will warn users about unused #![feature] directives if they're (a) a library feature and (b) not actually ever used. The compiler doesn't currently warn you about unused #![feature] directives tied to language features.

In the process of making the stability annotations more incremental-friendly (#44137) this check in the compiler has proven to be very difficult to make incremental. Namely what's happening here is:

  • There's a function, check_stability, which is used to verify that an API can actually be used.
  • In this function if the API in question is unstable and the feature is activated, then this feature name is stored off in a side table.
  • As we run through the compiler this side table gets bigger and bigger. Right now this table is mostly built up during typeck. (I don't know the precise reason as to why it's in typeck, personally)
  • Finally near the end of compilation we go through this table and use it to warn about unused #![feature] directives.

This "put things in a table on the side" behavior isn't very incremental friently and isn't too easy to transition over to the query system. For now this specific warning seems like it probably isn't that important at Rust 1.20.0 (while it was probably very important at Rust 1.0.0). I'm going to comment this out for now so we're not going to get lints/warnings about unused library features.

This is a FIXME to track re-enabling this warning!
@alexcrichton alexcrichton added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. labels Aug 31, 2017
@alexcrichton
Copy link
Member Author

Note that this also applies to the lint of "this #[feature] isn't needed because it's stable", that gets emitted at most once but it's not clear how to do that any more.

@durka
Copy link
Contributor

durka commented Sep 1, 2017

Oh, so the "unused feature" lint is going to be disabled entirely? That could potentially unblock me over at #43017 (comment) as I currently can't figure out how to correctly populate the side table you're talking about.

@durka durka mentioned this issue Sep 2, 2017
Merged
alexcrichton added a commit to alexcrichton/rust that referenced this issue Sep 5, 2017
@alexcrichton
rustc: Move stability functionality into queries
9a23196 
This commit primarily removes the `stability` field from `TyCtxt` as well as its
internal mutable state, instead using a query to build the stability index as
well as primarily using queries for other related lookups.

Like previous commits the calculation of the stability index is wrapped in a
`with_ignore` node to avoid regressing the current tests, and otherwise this
commit also introduces rust-lang#44232 but somewhat intentionally so.
@steveklabnik steveklabnik added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 14, 2017
@zackmdavis zackmdavis mentioned this issue Oct 30, 2017
Closed
@SimonSapin
Copy link
Contributor

Maybe side tables are fundamentally incompatible with incremental and the set of used features (perhaps optimized as a bit map) should be part of the “result” of relevant compiler queries?

@varkor varkor mentioned this issue Jul 4, 2018
Closed
@varkor varkor mentioned this issue Jul 23, 2018
Merged
3 tasks
bors added a commit that referenced this issue Aug 6, 2018
@bors
Auto merge of #52644 - varkor:lib-feature-gate-2, r=withoutboats
b239743 
Add errors for unknown, stable and duplicate feature attributes

- Adds an error for unknown (lang and lib) features.
- Extends the lint for unnecessary feature attributes for stable features to libs features (this already exists for lang features).
- Adds an error for duplicate (lang and lib) features.

```rust
#![feature(fake_feature)] //~ ERROR unknown feature `fake_feature`

#![feature(i128_type)] //~ WARNING the feature `i128_type` has been stable since 1.26.0

#![feature(non_exhaustive)]
#![feature(non_exhaustive)] //~ ERROR duplicate `non_exhaustive` feature attribute
```

Fixes #52053, fixes #53032 and address some of the problems noted in #44232 (though not unused features).

There are a few outstanding problems, that I haven't narrowed down yet:
- [x] Stability attributes on macros do not seem to be taken into account.
- [x] Stability attributes behind `cfg` attributes are not taken into account.
- [x] There are failing incremental tests.
@varkor varkor mentioned this issue Aug 28, 2019
Closed
@est31
Copy link
Member

est31 commented Aug 28, 2019

How is this different from an unused item, say struct Foo; that's never used inside the crate? This unused lint works well with incremental compilation. The tasks seem similar to me.

@Centril Centril added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Aug 29, 2019
@Centril
Copy link
Contributor

Centril commented Aug 29, 2019

Nominating at the request of @est31 in #63850.

@nikomatsakis nikomatsakis added P-medium Medium priority and removed I-nominated labels Aug 29, 2019
@nikomatsakis
Copy link
Contributor

Visiting as part of compiler triage.

I agree this would be nice, but it is certainly not P-high. It seems connected to the end-to-end query discussion. Based on Alex's comment, seems like there is some design work. In an effort to get somebody to maybe leave some thoughts, I'll cc @rust-lang/compiler, but not sure what else to do here. =)

@bjorn3 bjorn3 mentioned this issue Sep 17, 2020
Merged
@ehuss ehuss mentioned this issue Dec 5, 2020
Closed
@eggyal eggyal mentioned this issue Apr 8, 2021
Closed
@newca12 newca12 mentioned this issue Jul 10, 2021
Closed
@hudson-ayers hudson-ayers mentioned this issue Jul 13, 2022
Merged
2 tasks
@MrCroxx
Copy link

MrCroxx commented Mar 8, 2024 •
edited
Loading

Hi, sorry to bother you with the old issue. I'm looking for a command or a tool to detect the unnecessary features and I found this issue. Would you please tell that if the issue is still in the plan? Thanks a lot.

@fmease fmease added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. and removed A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. labels Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@steveklabnik @durka @alexcrichton @nikomatsakis @SimonSapin @Centril @est31 @fmease @MrCroxx