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

Split out statement attributes changes from #78306 #78326

Merged
merged 1 commit into from
Oct 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions compiler/rustc_ast/src/attr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,8 @@ impl HasAttrs for StmtKind {
match *self {
StmtKind::Local(ref local) => local.attrs(),
StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => expr.attrs(),
StmtKind::Empty | StmtKind::Item(..) => &[],
StmtKind::Item(ref item) => item.attrs(),
StmtKind::Empty => &[],
StmtKind::MacCall(ref mac) => mac.attrs.attrs(),
}
}
Expand All @@ -632,7 +633,8 @@ impl HasAttrs for StmtKind {
match self {
StmtKind::Local(local) => local.visit_attrs(f),
StmtKind::Expr(expr) | StmtKind::Semi(expr) => expr.visit_attrs(f),
StmtKind::Empty | StmtKind::Item(..) => {}
StmtKind::Item(item) => item.visit_attrs(f),
StmtKind::Empty => {}
StmtKind::MacCall(mac) => {
mac.attrs.visit_attrs(f);
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1357,7 +1357,8 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
// we'll expand attributes on expressions separately
if !stmt.is_expr() {
let (attr, derives, after_derive) = if stmt.is_item() {
self.classify_item(&mut stmt)
// FIXME: Handle custom attributes on statements (#15701)
(None, vec![], false)
} else {
// ignore derives on non-item statements so it falls through
// to the unused-attributes lint
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1099,11 +1099,11 @@ pub enum StmtKind<'hir> {
Semi(&'hir Expr<'hir>),
}

impl StmtKind<'hir> {
pub fn attrs(&self) -> &'hir [Attribute] {
impl<'hir> StmtKind<'hir> {
pub fn attrs(&self, get_item: impl FnOnce(ItemId) -> &'hir Item<'hir>) -> &'hir [Attribute] {
match *self {
StmtKind::Local(ref l) => &l.attrs,
StmtKind::Item(_) => &[],
StmtKind::Item(ref item_id) => &get_item(*item_id).attrs,
StmtKind::Expr(ref e) | StmtKind::Semi(ref e) => &e.attrs,
}
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,8 @@ impl EarlyLintPass for UnusedDocComment {
fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &ast::Stmt) {
let kind = match stmt.kind {
ast::StmtKind::Local(..) => "statements",
ast::StmtKind::Item(..) => "inner items",
// Disabled pending discussion in #78306
ast::StmtKind::Item(..) => return,
// expressions will be reported by `check_expr`.
ast::StmtKind::Empty
| ast::StmtKind::Semi(_)
Expand Down
19 changes: 17 additions & 2 deletions compiler/rustc_lint/src/early.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use crate::context::{EarlyContext, LintContext, LintStore};
use crate::passes::{EarlyLintPass, EarlyLintPassObject};
use rustc_ast as ast;
use rustc_ast::visit as ast_visit;
use rustc_attr::HasAttrs;
use rustc_session::lint::{BufferedEarlyLint, LintBuffer, LintPass};
use rustc_session::Session;
use rustc_span::symbol::Ident;
Expand Down Expand Up @@ -119,8 +120,22 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
}

fn visit_stmt(&mut self, s: &'a ast::Stmt) {
run_early_pass!(self, check_stmt, s);
self.check_id(s.id);
// Add the statement's lint attributes to our
// current state when checking the statement itself.
// This allows us to handle attributes like
// `#[allow(unused_doc_comments)]`, which apply to
// sibling attributes on the same target
//
// Note that statements get their attributes from
// the AST struct that they wrap (e.g. an item)
self.with_lint_attrs(s.id, s.attrs(), |cx| {
run_early_pass!(cx, check_stmt, s);
cx.check_id(s.id);
});
// The visitor for the AST struct wrapped
// by the statement (e.g. `Item`) will call
// `with_lint_attrs`, so do this walk
// outside of the above `with_lint_attrs` call
ast_visit::walk_stmt(self, s);
}

Expand Down
13 changes: 7 additions & 6 deletions compiler/rustc_lint/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,13 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas
}

fn visit_stmt(&mut self, s: &'tcx hir::Stmt<'tcx>) {
// statement attributes are actually just attributes on one of
// - item
// - local
// - expression
// so we keep track of lint levels there
lint_callback!(self, check_stmt, s);
let get_item = |id: hir::ItemId| self.context.tcx.hir().item(id.id);
let attrs = &s.kind.attrs(get_item);
// See `EarlyContextAndPass::visit_stmt` for an explanation
// of why we call `walk_stmt` outside of `with_lint_attrs`
self.with_lint_attrs(s.hir_id, attrs, |cx| {
lint_callback!(cx, check_stmt, s);
});
hir_visit::walk_stmt(self, s);
}

Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,13 @@ impl<'tcx> intravisit::Visitor<'tcx> for LintLevelMapBuilder<'_, 'tcx> {
})
}

fn visit_stmt(&mut self, e: &'tcx hir::Stmt<'tcx>) {
// We will call `with_lint_attrs` when we walk
// the `StmtKind`. The outer statement itself doesn't
// define the lint levels.
intravisit::walk_stmt(self, e);
}

fn visit_expr(&mut self, e: &'tcx hir::Expr<'tcx>) {
self.with_lint_attrs(e.hir_id, &e.attrs, |builder| {
intravisit::walk_expr(builder, e);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ impl<'hir> Map<'hir> {
Some(Node::Variant(ref v)) => Some(&v.attrs[..]),
Some(Node::Field(ref f)) => Some(&f.attrs[..]),
Some(Node::Expr(ref e)) => Some(&*e.attrs),
Some(Node::Stmt(ref s)) => Some(s.kind.attrs()),
Some(Node::Stmt(ref s)) => Some(s.kind.attrs(|id| self.item(id.id))),
Some(Node::Arm(ref a)) => Some(&*a.attrs),
Some(Node::GenericParam(param)) => Some(&param.attrs[..]),
// Unit/tuple structs/variants take the attributes straight from
Expand Down
12 changes: 12 additions & 0 deletions src/test/ui/lint/reasons-forbidden.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
//~^ NOTE `forbid` level set here
//~| NOTE `forbid` level set here
//~| NOTE `forbid` level set here
//~| NOTE `forbid` level set here
//~| NOTE `forbid` level set here
//~| NOTE `forbid` level set here
reason = "our errors & omissions insurance policy doesn't cover unsafe Rust"
)]

Expand All @@ -17,9 +20,18 @@ fn main() {
//~^ ERROR allow(unsafe_code) overruled by outer forbid(unsafe_code)
//~| ERROR allow(unsafe_code) overruled by outer forbid(unsafe_code)
//~| ERROR allow(unsafe_code) overruled by outer forbid(unsafe_code)
//~| ERROR allow(unsafe_code) overruled by outer forbid(unsafe_code)
//~| ERROR allow(unsafe_code) overruled by outer forbid(unsafe_code)
//~| ERROR allow(unsafe_code) overruled by outer forbid(unsafe_code)
//~| NOTE overruled by previous forbid
//~| NOTE overruled by previous forbid
//~| NOTE overruled by previous forbid
//~| NOTE overruled by previous forbid
//~| NOTE overruled by previous forbid
//~| NOTE overruled by previous forbid
//~| NOTE our errors & omissions insurance policy doesn't cover unsafe Rust
//~| NOTE our errors & omissions insurance policy doesn't cover unsafe Rust
//~| NOTE our errors & omissions insurance policy doesn't cover unsafe Rust
//~| NOTE our errors & omissions insurance policy doesn't cover unsafe Rust
//~| NOTE our errors & omissions insurance policy doesn't cover unsafe Rust
//~| NOTE our errors & omissions insurance policy doesn't cover unsafe Rust
Expand Down
41 changes: 37 additions & 4 deletions src/test/ui/lint/reasons-forbidden.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0453]: allow(unsafe_code) overruled by outer forbid(unsafe_code)
--> $DIR/reasons-forbidden.rs:16:13
--> $DIR/reasons-forbidden.rs:19:13
|
LL | unsafe_code,
| ----------- `forbid` level set here
Expand All @@ -10,7 +10,7 @@ LL | #[allow(unsafe_code)]
= note: our errors & omissions insurance policy doesn't cover unsafe Rust

error[E0453]: allow(unsafe_code) overruled by outer forbid(unsafe_code)
--> $DIR/reasons-forbidden.rs:16:13
--> $DIR/reasons-forbidden.rs:19:13
|
LL | unsafe_code,
| ----------- `forbid` level set here
Expand All @@ -21,7 +21,7 @@ LL | #[allow(unsafe_code)]
= note: our errors & omissions insurance policy doesn't cover unsafe Rust

error[E0453]: allow(unsafe_code) overruled by outer forbid(unsafe_code)
--> $DIR/reasons-forbidden.rs:16:13
--> $DIR/reasons-forbidden.rs:19:13
|
LL | unsafe_code,
| ----------- `forbid` level set here
Expand All @@ -31,6 +31,39 @@ LL | #[allow(unsafe_code)]
|
= note: our errors & omissions insurance policy doesn't cover unsafe Rust

error: aborting due to 3 previous errors
error[E0453]: allow(unsafe_code) overruled by outer forbid(unsafe_code)
--> $DIR/reasons-forbidden.rs:19:13
|
LL | unsafe_code,
| ----------- `forbid` level set here
...
LL | #[allow(unsafe_code)]
| ^^^^^^^^^^^ overruled by previous forbid
|
= note: our errors & omissions insurance policy doesn't cover unsafe Rust

error[E0453]: allow(unsafe_code) overruled by outer forbid(unsafe_code)
--> $DIR/reasons-forbidden.rs:19:13
|
LL | unsafe_code,
| ----------- `forbid` level set here
...
LL | #[allow(unsafe_code)]
| ^^^^^^^^^^^ overruled by previous forbid
|
= note: our errors & omissions insurance policy doesn't cover unsafe Rust

error[E0453]: allow(unsafe_code) overruled by outer forbid(unsafe_code)
--> $DIR/reasons-forbidden.rs:19:13
|
LL | unsafe_code,
| ----------- `forbid` level set here
...
LL | #[allow(unsafe_code)]
| ^^^^^^^^^^^ overruled by previous forbid
|
= note: our errors & omissions insurance policy doesn't cover unsafe Rust

error: aborting due to 6 previous errors

For more information about this error, try `rustc --explain E0453`.
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/utils/author.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ impl<'tcx> LateLintPass<'tcx> for Author {
}

fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx hir::Stmt<'_>) {
if !has_attr(cx.sess(), stmt.kind.attrs()) {
if !has_attr(cx.sess(), stmt.kind.attrs(|id| cx.tcx.hir().item(id.id))) {
return;
}
prelude();
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/utils/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl<'tcx> LateLintPass<'tcx> for DeepCodeInspector {
}

fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx hir::Stmt<'_>) {
if !has_attr(cx.sess(), stmt.kind.attrs()) {
if !has_attr(cx.sess(), stmt.kind.attrs(|id| cx.tcx.hir().item(id.id))) {
return;
}
match stmt.kind {
Expand Down