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

Shorten Span of unused macro lints #90761

Merged
merged 1 commit into from
Nov 12, 2021
Merged

Conversation

hellow554
Copy link
Contributor

@hellow554 hellow554 commented Nov 10, 2021

The span has been reduced to the actual ident of the macro, instead of linting the
whole macro.

Closes #90745

r? @estebank

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 10, 2021
#![allow(dead_code)]
#![allow(dead_code, unused_macros)]
Copy link
Contributor Author

@hellow554 hellow554 Nov 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is very insteresting.

pub fn f(_input: TokenStream) -> TokenStream {
let rules = r#"
macro_rules! id {
($($tt:tt)*) => { $($tt)* };
}
"#;
rules.parse().unwrap()
}

Any idea why the unused macro wasn't linted before?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, no idea. My best guess is that now we have a span for id, but we didn't for the whole macro. I think changing the test is reasonable. The issue is about the macro expansion machinery, so it's reasonable to allow it.

@est31
Copy link
Member

est31 commented Nov 10, 2021

Interestingly, back when unused macro warnings were added, both lints matched. Then, the unused function lint's span was reduced further and further:

Screenshot_20211110_153156

I think the reduced span makes sense. I wonder if other unused lints have similar behaviour.

#![allow(dead_code)]
#![allow(dead_code, unused_macros)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, no idea. My best guess is that now we have a span for id, but we didn't for the whole macro. I think changing the test is reasonable. The issue is about the macro expansion machinery, so it's reasonable to allow it.

compiler/rustc_resolve/src/macros.rs Outdated Show resolved Hide resolved
@estebank
Copy link
Contributor

I have one stylistic nitpick, I'm r+ing, but if you want to fix it gets picked up, go ahead.

@bors r+

@bors
Copy link
Contributor

bors commented Nov 10, 2021

📌 Commit 3f9b7d2113e50e062a7464ad861d933290e5fda0 has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 10, 2021
@rust-log-analyzer

This comment has been minimized.

@hellow554 hellow554 force-pushed the macro_span branch 2 times, most recently from 8ea10b7 to 9f6ca74 Compare November 11, 2021 07:03
The span has been recuded to the actual ident, instead of linting the
*whole* macro.
@sanxiyn
Copy link
Member

sanxiyn commented Nov 11, 2021

@bors r=estebank

@bors
Copy link
Contributor

bors commented Nov 11, 2021

📌 Commit 9f6ca74 has been approved by estebank

@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 11, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 11, 2021
Shorten Span of unused macro lints

The span has been reduced to the actual ident of the macro, instead of linting the
*whole* macro.

Closes rust-lang#90745

r? `@estebank`
@bors
Copy link
Contributor

bors commented Nov 12, 2021

⌛ Testing commit 9f6ca74 with merge 5926b64a90e7d28dacce5b89ec935eb73656db23...

@bors
Copy link
Contributor

bors commented Nov 12, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 12, 2021
@rust-log-analyzer
Copy link
Collaborator

The job dist-arm-linux failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
   Compiling cmake v0.1.44
error: could not compile `crossbeam-utils`

Caused by:
  process didn't exit successfully: `/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/rustc --crate-name build_script_build --edition=2018 /cargo/registry/src/git.luolix.top-1ecc6299db9ec823/crossbeam-utils-0.8.3/build.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type bin --emit=dep-info,link -C embed-bitcode=no -C debuginfo=0 --cfg 'feature="default"' --cfg 'feature="lazy_static"' --cfg 'feature="std"' -C metadata=7d86092267134ed5 -C extra-filename=-7d86092267134ed5 --out-dir /checkout/obj/build/bootstrap/debug/build/crossbeam-utils-7d86092267134ed5 -L dependency=/checkout/obj/build/bootstrap/debug/deps --extern autocfg=/checkout/obj/build/bootstrap/debug/deps/libautocfg-fa5fb2f81f71684d.rlib --cap-lints allow -Wrust_2018_idioms -Wunused_lifetimes -Wsemicolon_in_expressions_from_macros -Dwarnings` (signal: 6, SIGABRT: process abort signal)
error: build failed
failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
Build completed unsuccessfully in 0:00:56
make: *** [prepare] Error 1
---
[RUSTC-TIMING] build_script_build test:false 0.381
[RUSTC-TIMING] build_script_build test:false 0.672
   Compiling rustc-std-workspace-core v1.99.0 (/checkout/library/rustc-std-workspace-core)
[RUSTC-TIMING] rustc_std_workspace_core test:false 0.185
rustc exited with signal: 11 (core dumped)

Caused by:
Caused by:
  process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc --crate-name rustc_std_workspace_core --edition=2018 library/rustc-std-workspace-core/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts --crate-type lib --emit=dep-info,metadata,link -C opt-level=3 -C embed-bitcode=no -C codegen-units=1 -C debuginfo=1 -C metadata=fd44d8b17c2359b1 -C extra-filename=-fd44d8b17c2359b1 --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/release/deps --extern core=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/libcore-2a8f88aa3a5cb424.rmeta --cfg=bootstrap -Zsymbol-mangling-version=legacy -Zmacro-backtrace '-Clink-args=-Wl,-rpath,$ORIGIN/../lib' -Cprefer-dynamic '-Zcrate-attr=doc(html_root_url="https://doc.rust-lang.org/nightly/")' -Z binary-dep-depinfo` (exit status: 254)
[RUSTC-TIMING] libc test:false 0.682
[RUSTC-TIMING] compiler_builtins test:false 0.709
[RUSTC-TIMING] core test:false 16.299
error: build failed

@hellow554
Copy link
Contributor Author

That looks spurious, nothing I can do?!

@est31
Copy link
Member

est31 commented Nov 12, 2021

@bors retry

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 12, 2021
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 12, 2021
@bors
Copy link
Contributor

bors commented Nov 12, 2021

⌛ Testing commit 9f6ca74 with merge 6cc0c20eda28e8c057542cb97e0043e066fcef74...

@bors
Copy link
Contributor

bors commented Nov 12, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 12, 2021
@rust-log-analyzer
Copy link
Collaborator

The job dist-mipsel-linux failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
   Compiling itoa v0.4.6
[RUSTC-TIMING] rustc_fs_util test:false 0.142
   Compiling snap v1.0.1
[RUSTC-TIMING] build_script_build test:false 0.223
rustc exited with signal: 11 (core dumped)
error: could not compile `serde_derive`
Caused by:
Caused by:
  process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc --crate-name build_script_build /cargo/registry/src/git.luolix.top-1ecc6299db9ec823/serde_derive-1.0.125/build.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type bin --emit=dep-info,link -C embed-bitcode=no -C debuginfo=0 -C debug-assertions=off --cfg 'feature="default"' -C metadata=ac27467a3c2a9e00 -C extra-filename=-ac27467a3c2a9e00 --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/release/build/serde_derive-ac27467a3c2a9e00 -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/release/deps --cap-lints allow -Z binary-dep-depinfo` (exit status: 254)
[RUSTC-TIMING] itoa test:false 0.142
[RUSTC-TIMING] tinyvec test:false 0.314
[RUSTC-TIMING] datafrog test:false 0.356
[RUSTC-TIMING] rustc_graphviz test:false 0.562

@est31
Copy link
Member

est31 commented Nov 12, 2021

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 12, 2021
hellow554 added a commit to hellow554/rust that referenced this pull request Nov 12, 2021
Shorten Span of unused macro lints

The span has been reduced to the actual ident of the macro, instead of linting the
*whole* macro.

Closes rust-lang#90745

r? `@estebank`
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 12, 2021
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#90589 (rustc_llvm: update PassWrapper for recent LLVM)
 - rust-lang#90644 (Extend the const swap feature)
 - rust-lang#90704 (Unix ExitStatus comments and a tiny docs fix)
 - rust-lang#90761 (Shorten Span of unused macro lints)
 - rust-lang#90795 (Add more comments to explain the code to generate the search index)
 - rust-lang#90798 (Document `unreachable!` custom panic message)
 - rust-lang#90826 (rustc_feature: Convert `BuiltinAttribute` from tuple to a struct)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 640f365 into rust-lang:master Nov 12, 2021
@rustbot rustbot added this to the 1.58.0 milestone Nov 12, 2021
@hellow554 hellow554 deleted the macro_span branch November 14, 2021 08:19
JohnTitor pushed a commit to JohnTitor/rust that referenced this pull request Nov 1, 2022
…span, r=cjgillot

Reduce span of let else irrefutable_let_patterns warning

Huge spans aren't good for IDE users as they underline constructs that are possibly multiline.

Similar PR to rust-lang#90761 which did the same for the `unused_macros` lint.
bors added a commit to rust-lang/rust-clippy that referenced this pull request Nov 2, 2022
Shrink `missing_{safety,errors,panics}_doc` spans

Shrink the spans of `clippy::missing_*_doc` to match `missing_docs` and don't cover the entire item anymore. This helps readability in IDEs by not coloring the entire screen yellow, similar to rust-lang/rust#103749 and rust-lang/rust#90761.

before:
![image](https://user-images.githubusercontent.com/26522220/199483288-af0cf0c2-a9e4-47a8-81e3-50ccf380d939.png)

after:
![image](https://user-images.githubusercontent.com/26522220/199483366-445e5ddd-9f71-4de1-85be-43461c9b7d5d.png)

changelog: [`missing_safety_doc`], [`missing_error_doc`], [`missing_panics_doc`]: These lints no longer span the entire item.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Span for unused_macros lint is too large, which makes writing macros in IDEs annoying
9 participants