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

Liveness lints should consider statement attributes #30326

Closed
mitaa opened this issue Dec 11, 2015 · 5 comments
Closed

Liveness lints should consider statement attributes #30326

mitaa opened this issue Dec 11, 2015 · 5 comments
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut.

Comments

@mitaa
Copy link
Contributor

mitaa commented Dec 11, 2015

#![feature(stmt_expr_attributes)]

fn main() {
    #[allow(unused_variables)]
    let mut foo = 7;
    #[allow(unused_assignments)]
    foo = 42;
}
<anon>:5:9: 5:16 warning: variable `foo` is assigned to, but never used, #[warn(unused_variables)] on by default
<anon>:5     let mut foo = 7;
                 ^~~~~~~
<anon>:7:5: 7:8 warning: value assigned to `foo` is never read, #[warn(unused_assignments)] on by default
<anon>:7     foo = 42;
             ^~~

I would expect to see no warnings.

@mitaa mitaa changed the title Liveness lints don't consider statement attributes Liveness lints should consider statement attributes Dec 11, 2015
@alexcrichton
Copy link
Member

cc @Kimundi
cc #15701

@jonas-schievink
Copy link
Contributor

The issue is that built-in lints are emitted by storing lint messages in a map keyed by some NodeId. The lint pass then traverses each item using an IdVisitor, which walks an item (ignoring nested items) and invokes a callback for all NodeIds. The callback then emits the stored lint messages if the current level allows that. Since no lint attributes are processed while the IdVisitor is doing its thing, the levels aren't changed by attributes applied to things within an item.

This issue has existed before, since lint attributes on match arms are ignored as well.

Fixing this either requires some tuning of the IdVisitor (maybe refactor how it works altogether) or doing the NodeId stuff manually. Or change how built-in lints are handled. Not sure what's the best way to do this.

@fhahn
Copy link
Contributor

fhahn commented Dec 19, 2015

I also had a look at this issue and I thought it could be solved by extending Session.add_lint to also take and store the lint attribute of the node. Then when actually emitting the lints, all neccessary information should be available.

@jonas-schievink
Copy link
Contributor

@fhahn You'd have to traverse parent blocks, since lint attributes could be defined on them, but then it could work. But at that point you'd be duplicating a lot of the logic of the lint contexts.

@huonw huonw added the A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. label Jan 6, 2016
bors added a commit that referenced this issue Jan 27, 2016
…richton

`LateContext` already does this, looks like this was just forgotten in #29850.

Found while investigating #30326 (but doesn't fix it)
@Mark-Simulacrum
Copy link
Member

This appears to have been fixed as of rustc 1.18.0-nightly (bbdaad0dc 2017-04-14). Output from the code sample in the top comment is empty (no lint warnings).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut.
Projects
None yet
Development

No branches or pull requests

6 participants