Never regard macro rules with compile_error! invocations as unused #97903
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
the
T-compiler
rust-highfive
added
the
S-waiting-on-review
eggyal
reviewed
Jun 9, 2022
petrochenkov
reviewed
Jun 9, 2022
petrochenkov
added
S-waiting-on-author
The very point of compile_error! is to never be reached, and one of the use cases of the macro, currently also listed as examples in the documentation of compile_error, is to create nicer errors for wrong macro invocations. Thus, we shuuld never warn about unused macro arms that contain invocations of compile_error.
Prior to this commit, if a macro had any malformed rules, all rules would be reported as unused, regardless of whether they were used or not. So we just turn off unused rule checking completely for macros with malformed rules.
The unused_macro_rules lint had a bug where it would regard all rules of a macro as unused if one rule were malformed. This bug doesn't exist with the unused_macros lint. To ensure it doesn't appear in the future, we add a test for it.
est31
force-pushed
the
unused_macro_rules_compile_error
branch
from
dd95fe3 to
787e24c
Compare
|
As requested, I've removed the glob import. I've also added two commits. The first added commit silences |
petrochenkov
added
S-waiting-on-review
|
Thanks! |
|
📌 Commit 787e24c has been approved by |
bors
added
S-waiting-on-bors
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this pull request
Jun 10, 2022
…ror, r=petrochenkov Never regard macro rules with compile_error! invocations as unused The very point of compile_error! is to never be reached, and one of the use cases of the macro, currently also listed as examples in the documentation of compile_error, is to create nicer errors for wrong macro invocations. Thus, we should never warn about unused macro arms that contain invocations of compile_error. See also rust-lang#96150 (comment) and the discussion after that. Furthermore, the PR also contains two commits to silence `unused_macro_rules` when a macro has an invalid rule, and to add a test that `unused_macros` does not behave badly in the same situation. r? `@petrochenkov` as I've talked to them about this
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (fa68e73): comparison url. Instruction count
Max RSS (memory usage)Results
CyclesResults
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The very point of compile_error! is to never be reached, and one of
the use cases of the macro, currently also listed as examples in the
documentation of compile_error, is to create nicer errors for wrong
macro invocations. Thus, we should never warn about unused macro arms
that contain invocations of compile_error.
See also #96150 (comment) and the discussion after that.
Furthermore, the PR also contains two commits to silence
unused_macro_ruleswhen a macro has an invalid rule, and to add a test thatunused_macrosdoes not behave badly in the same situation.r? @petrochenkov as I've talked to them about this