-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Extend the unused macro lint to macros 2.0 #42334
Conversation
Right now, this is not recognized as unused yet: mod foo {
pub macro bar {
() => {}
}
} Is it a problem if the PR gets merged with that flaw? I saw code that does a similar trade off: https://github.com/rust-lang/rust/blob/master/src/librustc_resolve/check_unused.rs#L81-L85 |
@est31 Thanks!
I don't think so. Ideally we'd do reachability analysis, but it makes sense to land this first since it's also an issue with other items. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me modulo optional comment
src/librustc_resolve/lib.rs
Outdated
@@ -1200,7 +1200,7 @@ pub struct Resolver<'a> { | |||
pub found_unresolved_macro: bool, | |||
|
|||
// List of crate local macros that we need to warn about as being unused. | |||
// Right now this only includes macro_rules! macros. | |||
// Right now this only includes macro_rules! macros, and 2.0 macros. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "declarative macros" or "macros 2.0"
Also looks like Travis is failing because an existing test with an unused declarative macro needs updating. |
r? @jseyfried |
@bors: r=jseyfried #42334 (review) |
📌 Commit 03876ec has been approved by |
⌛ Testing commit 03876ec with merge a8dc246... |
Extend the unused macro lint to macros 2.0 Extends the unused macro lint (added in PR #41907) to macros 2.0 (added in PR #40847). r? @jseyfried
☀️ Test successful - status-appveyor, status-travis |
Extends the unused macro lint (added in PR #41907) to macros 2.0 (added in PR #40847).
r? @jseyfried