-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Split out statement attributes changes from #78306 #78326
Merged
Merged
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
When the 'early' and 'late' visitors visit an attribute target, they activate any lint attributes (e.g. `#[allow]`) that apply to it. This can affect warnings emitted on sibiling attributes. For example, the following code does not produce an `unused_attributes` for `#[inline]`, since the sibiling `#[allow(unused_attributes)]` suppressed the warning. ```rust trait Foo { #[allow(unused_attributes)] #[inline] fn first(); #[inline] #[allow(unused_attributes)] fn second(); } ``` However, we do not do this for statements - instead, the lint attributes only become active when we visit the struct nested inside `StmtKind` (e.g. `Item`). Currently, this is difficult to observe due to another issue - the `HasAttrs` impl for `StmtKind` ignores attributes for `StmtKind::Item`. As a result, the `unused_doc_comments` lint will never see attributes on item statements. This commit makes two interrelated fixes to the handling of inert (non-proc-macro) attributes on statements: * The `HasAttr` impl for `StmtKind` now returns attributes for `StmtKind::Item`, treating it just like every other `StmtKind` variant. The only place relying on the old behavior was macro which has been updated to explicitly ignore attributes on item statements. This allows the `unused_doc_comments` lint to fire for item statements. * The `early` and `late` lint visitors now activate lint attributes when invoking the callback for `Stmt`. This ensures that a lint attribute (e.g. `#[allow(unused_doc_comments)]`) can be applied to sibiling attributes on an item statement. For now, the `unused_doc_comments` lint is explicitly disabled on item statements, which preserves the current behavior. The exact locatiosn where this lint should fire are being discussed in PR rust-lang#78306
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
label
Oct 24, 2020
@bors r+ |
📌 Commit ac384ac has been approved by |
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
Oct 24, 2020
jyn514
added
A-attributes
Area: Attributes (`#[…]`, `#![…]`)
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
Oct 24, 2020
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 25, 2020
Rollup of 8 pull requests Successful merges: - rust-lang#77984 (Compute proper module parent during resolution) - rust-lang#78085 (MIR validation should check `SwitchInt` values are valid for the type) - rust-lang#78208 (replace `#[allow_internal_unstable]` with `#[rustc_allow_const_fn_unstable]` for `const fn`s) - rust-lang#78209 (Update `compiler_builtins` to 0.1.36) - rust-lang#78276 (Bump backtrace-rs to enable Mach-O support on iOS.) - rust-lang#78320 (Link to cargo's `build-std` feature instead of `xargo` in custom target docs) - rust-lang#78322 (BTreeMap: stop mistaking node::MIN_LEN for a node level constraint) - rust-lang#78326 (Split out statement attributes changes from rust-lang#78306) Failed merges: r? `@ghost`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-attributes
Area: Attributes (`#[…]`, `#![…]`)
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.
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.
This is the same as PR #78306, but
unused_doc_comments
is modified to explicitly ignore statement items (which preserves the current behavior).This shouldn't have any user-visible effects, so it can be landed without lang team discussion.
When the 'early' and 'late' visitors visit an attribute target, they
activate any lint attributes (e.g.
#[allow]
) that apply to it.This can affect warnings emitted on sibiling attributes. For example,
the following code does not produce an
unused_attributes
for#[inline]
, since the sibiling#[allow(unused_attributes)]
suppressedthe warning.
However, we do not do this for statements - instead, the lint attributes
only become active when we visit the struct nested inside
StmtKind
(e.g.
Item
).Currently, this is difficult to observe due to another issue - the
HasAttrs
impl forStmtKind
ignores attributes forStmtKind::Item
.As a result, the
unused_doc_comments
lint will never see attributes onitem statements.
This commit makes two interrelated fixes to the handling of inert
(non-proc-macro) attributes on statements:
HasAttr
impl forStmtKind
now returns attributes forStmtKind::Item
, treating it just like every otherStmtKind
variant. The only place relying on the old behavior was macro
which has been updated to explicitly ignore attributes on item
statements. This allows the
unused_doc_comments
lint to fire foritem statements.
early
andlate
lint visitors now activate lint attributes wheninvoking the callback for
Stmt
. This ensures that a lintattribute (e.g.
#[allow(unused_doc_comments)]
) can be applied tosibiling attributes on an item statement.
For now, the
unused_doc_comments
lint is explicitly disabled on itemstatements, which preserves the current behavior. The exact locatiosn
where this lint should fire are being discussed in PR #78306