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

redundant_semicolon and clippy::no_effect tripped without emitting line numbers #63967

Closed
brson opened this issue Aug 28, 2019 · 7 comments · Fixed by #64387
Closed

redundant_semicolon and clippy::no_effect tripped without emitting line numbers #63967

brson opened this issue Aug 28, 2019 · 7 comments · Fixed by #64387
Assignees
@rustbot
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@brson
Copy link
Contributor

brson commented Aug 28, 2019 •
edited by rustbot
Loading

Using nightly-2019-08-26.

In this TiKV PR we upgraded the compiler. Our tidb_query component tripped the redundant_semicolon lint (and seemingly in turn clippy::no_effect) but rustc/clippy did not tell us the line number.

I couldn't find the code that triggered the lint and had to just allow both for the whole crate.

This issue has been assigned to @nathanwhit via this comment.
@brson
Copy link
Contributor Author

brson commented Aug 28, 2019 •
edited
Loading

It can be reproduced by cloning that PR, removing the commit that adds the allows, and running make clippy.

This was referenced Aug 28, 2019
Closed
Merged
@estebank
Copy link
Contributor

Introduced in #62984. It has caused similar effects in other cases like #63947. CC #63679.

@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 28, 2019
@Centril
Copy link
Contributor

Centril commented Sep 5, 2019

cc @nathanwhit @varkor

@nikomatsakis nikomatsakis added the P-high High priority label Sep 5, 2019
@nikomatsakis
Copy link
Contributor

Check-in from compiler team:

Marking as P-high " as this is a regression, though admittedly one without particularly serious consequences (the lint can always be allowed).

@nathanwhit or @varkor, perhaps one of you can take a stab at this? Please claim the issue if you do start working on it (even if it's just to write mentoring instructions).

@nikomatsakis nikomatsakis added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Sep 5, 2019
@nikomatsakis
Copy link
Contributor

Also, it'd be great to get a minimized version of the problem. Therefore, marking as E-needs-mvce.

@nathanwhit
Copy link
Member

I'll work on this. Looking into it a bit, I think it has to do with the lint's interaction with custom proc macro attributes. Specifically, when the item TokenStream passed to the attribute contains a redundant semicolon, every span in the TokenStream is the dummy span. This occurs in both this issue and #63947. It's definitely due to the changes to the parser, but I'm not sure why the changes would have that effect on the spans.
@rustbot claim

@rustbot rustbot self-assigned this Sep 5, 2019
@nathanwhit nathanwhit mentioned this issue Sep 11, 2019
Merged
Centril added a commit to Centril/rust that referenced this issue Sep 20, 2019
@Centril
Rollup merge of rust-lang#64387 - nathanwhit:redundant-semi-fix, r=va…
fa5d8ab 
…rkor

Fix redundant semicolon lint interaction with proc macro attributes

Fixes rust-lang#63967 and fixes rust-lang#63947, both of which were caused by the lint's changes to the parser interacting poorly with proc macro attributes and causing span information to be lost

r? @varkor
tmandry added a commit to tmandry/rust that referenced this issue Sep 20, 2019
@tmandry
Rollup merge of rust-lang#64387 - nathanwhit:redundant-semi-fix, r=va…
3754e61 
…rkor

Fix redundant semicolon lint interaction with proc macro attributes

Fixes rust-lang#63967 and fixes rust-lang#63947, both of which were caused by the lint's changes to the parser interacting poorly with proc macro attributes and causing span information to be lost

r? @varkor
tmandry added a commit to tmandry/rust that referenced this issue Sep 20, 2019
@tmandry
Rollup merge of rust-lang#64387 - nathanwhit:redundant-semi-fix, r=va…
736b4ae 
…rkor

Fix redundant semicolon lint interaction with proc macro attributes

Fixes rust-lang#63967 and fixes rust-lang#63947, both of which were caused by the lint's changes to the parser interacting poorly with proc macro attributes and causing span information to be lost

r? @varkor
tmandry added a commit to tmandry/rust that referenced this issue Sep 20, 2019
@tmandry
Rollup merge of rust-lang#64387 - nathanwhit:redundant-semi-fix, r=va…
50df43d 
…rkor

Fix redundant semicolon lint interaction with proc macro attributes

Fixes rust-lang#63967 and fixes rust-lang#63947, both of which were caused by the lint's changes to the parser interacting poorly with proc macro attributes and causing span information to be lost

r? @varkor
tmandry added a commit to tmandry/rust that referenced this issue Sep 20, 2019
@tmandry
Rollup merge of rust-lang#64387 - nathanwhit:redundant-semi-fix, r=va…
1d4f769 
…rkor

Fix redundant semicolon lint interaction with proc macro attributes

Fixes rust-lang#63967 and fixes rust-lang#63947, both of which were caused by the lint's changes to the parser interacting poorly with proc macro attributes and causing span information to be lost

r? @varkor
@pnkfelix
Copy link
Member

@nathanwhit just in case you didn't see, the attempt to fix this with your PR #64387 failed the rollup, see #64387 (comment)

@bors bors closed this as completed in 55a3ead Sep 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rustbot rustbot

Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix redundant semicolon lint interaction with proc macro attributes nathanwhit/rust
7 participants
@brson @nikomatsakis @pnkfelix @Centril @estebank @nathanwhit @rustbot