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

libsyntax visitor does not visit the paths in macros #54110

Closed
nikomatsakis opened this issue Sep 10, 2018 · 3 comments
Closed

libsyntax visitor does not visit the paths in macros #54110

nikomatsakis opened this issue Sep 10, 2018 · 3 comments
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

It's a minor point, but the macro struct includes a path:

rust/src/libsyntax/ast.rs

Lines 1223 to 1228 in 551244f

#[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
pub struct Mac_ {
pub path: Path,
pub delim: MacDelimiter,
pub tts: ThinTokenStream,
}

However, the walk_mac helper just ignores this completely:

rust/src/libsyntax/visit.rs

Lines 650 to 652 in 551244f

pub fn walk_mac<'a, V: Visitor<'a>>(_: &mut V, _: &Mac) {
// Empty!
}

As a result — well, a partial result — the early lint checker just ignores this path altogether (in particular, visit_path is never invoked, and hence check_path and friends are not invoked):

fn visit_mac(&mut self, mac: &'a ast::Mac) {
run_lints!(self, check_mac, mac);
}

This was a contributing factor to #53686.

I say that this is a "partial result" because the early lint checker never calls walk_mac -- but, really, it ought to. And, if it did, and walk_mac did what it was supposed to do and walked all the subparts, then everything would be fine.

Adding a hacky fix for now.

@nikomatsakis nikomatsakis added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 10, 2018
@nikomatsakis
Copy link
Contributor Author

cc @Manishearth @oli-obk — Do you agree that the lint visitor ought to change here?

cc @petrochenkov — do the paths in macros have a NodeId like other paths? That was one thing I wans't sure of when I tried to push this through.

@petrochenkov
Copy link
Contributor

do the paths in macros have a NodeId like other paths?

No, node ids are assigned during expansion to "final" nodes that will stay in AST after expansion.

@Manishearth
Copy link
Member

Yeah, the lint visitor should change.

Centril added a commit to Centril/rust that referenced this issue Mar 29, 2019
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. C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants