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

Fix inconsistencies in handling of inert attributes on statements #78306

Closed
wants to merge 2 commits into from
Closed
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
4 changes: 2 additions & 2 deletions compiler/rustc_ast/src/util/comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub struct Comment {
/// Makes a doc string more presentable to users.
/// Used by rustdoc and perhaps other tools, but not by rustc.
pub fn beautify_doc_string(data: Symbol) -> String {
/// remove whitespace-only lines from the start/end of lines
// remove whitespace-only lines from the start/end of lines
fn vertical_trim(lines: Vec<String>) -> Vec<String> {
let mut i = 0;
let mut j = lines.len();
Expand All @@ -50,7 +50,7 @@ pub fn beautify_doc_string(data: Symbol) -> String {
lines[i..j].to_vec()
}

/// remove a "[ \t]*\*" block from each line, if possible
// remove a "[ \t]*\*" block from each line, if possible
fn horizontal_trim(lines: Vec<String>) -> Vec<String> {
let mut i = usize::MAX;
let mut can_trim = true;
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,11 +419,11 @@ impl Visitor<'_> for ImplTraitTypeIdVisitor<'_> {

impl<'a, 'hir> LoweringContext<'a, 'hir> {
fn lower_crate(mut self, c: &Crate) -> hir::Crate<'hir> {
/// Full-crate AST visitor that inserts into a fresh
/// `LoweringContext` any information that may be
/// needed from arbitrary locations in the crate,
/// e.g., the number of lifetime generic parameters
/// declared for every type and trait definition.
// Full-crate AST visitor that inserts into a fresh
// `LoweringContext` any information that may be
// needed from arbitrary locations in the crate,
// e.g., the number of lifetime generic parameters
// declared for every type and trait definition.
struct MiscCollector<'tcx, 'lowering, 'hir> {
lctx: &'tcx mut LoweringContext<'lowering, 'hir>,
hir_id_owner: Option<NodeId>,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1160,7 +1160,7 @@ impl EmitterWriter {
// `max_line_num_len`
let padding = " ".repeat(padding + label.len() + 5);

/// Returns `override` if it is present and `style` is `NoStyle` or `style` otherwise
// Returns `override` if it is present and `style` is `NoStyle` or `style` otherwise
fn style_or_override(style: Style, override_: Option<Style>) -> Style {
match (style, override_) {
(Style::NoStyle, Some(override_)) => override_,
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
2 changes: 1 addition & 1 deletion compiler/rustc_index/src/bit_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -900,7 +900,7 @@ impl<R: Idx, C: Idx> BitMatrix<R, C> {

impl<R: Idx, C: Idx> fmt::Debug for BitMatrix<R, C> {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
/// Forces its contents to print in regular mode instead of alternate mode.
// Forces its contents to print in regular mode instead of alternate mode.
struct OneLinePrinter<T>(T);
impl<T: fmt::Debug> fmt::Debug for OneLinePrinter<T> {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand Down
20 changes: 10 additions & 10 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1658,8 +1658,8 @@ impl EarlyLintPass for EllipsisInclusiveRangePatterns {

use self::ast::{PatKind, RangeSyntax::DotDotDot};

/// If `pat` is a `...` pattern, return the start and end of the range, as well as the span
/// corresponding to the ellipsis.
// If `pat` is a `...` pattern, return the start and end of the range, as well as the span
// corresponding to the ellipsis.
fn matches_ellipsis_pat(pat: &ast::Pat) -> Option<(Option<&Expr>, &Expr, Span)> {
match &pat.kind {
PatKind::Range(
Expand Down Expand Up @@ -2348,11 +2348,11 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
Uninit,
};

/// Information about why a type cannot be initialized this way.
/// Contains an error message and optionally a span to point at.
// Information about why a type cannot be initialized this way.
// Contains an error message and optionally a span to point at.
type InitError = (String, Option<Span>);

/// Test if this constant is all-0.
// Test if this constant is all-0.
fn is_zero(expr: &hir::Expr<'_>) -> bool {
use hir::ExprKind::*;
use rustc_ast::LitKind::*;
Expand All @@ -2369,7 +2369,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
}
}

/// Determine if this expression is a "dangerous initialization".
// Determine if this expression is a "dangerous initialization".
fn is_dangerous_init(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<InitKind> {
if let hir::ExprKind::Call(ref path_expr, ref args) = expr.kind {
// Find calls to `mem::{uninitialized,zeroed}` methods.
Expand Down Expand Up @@ -2409,16 +2409,16 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
None
}

/// Test if this enum has several actually "existing" variants.
/// Zero-sized uninhabited variants do not always have a tag assigned and thus do not "exist".
// Test if this enum has several actually "existing" variants.
// Zero-sized uninhabited variants do not always have a tag assigned and thus do not "exist".
fn is_multi_variant(adt: &ty::AdtDef) -> bool {
// As an approximation, we only count dataless variants. Those are definitely inhabited.
let existing_variants = adt.variants.iter().filter(|v| v.fields.is_empty()).count();
existing_variants > 1
}

/// Return `Some` only if we are sure this type does *not*
/// allow zero initialization.
// Return `Some` only if we are sure this type does *not*
// allow zero initialization.
fn ty_find_init_error<'tcx>(
tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>,
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
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/mir/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ pub struct GeneratorLayout<'tcx> {

impl Debug for GeneratorLayout<'_> {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
/// Prints an iterator of (key, value) tuples as a map.
// Prints an iterator of (key, value) tuples as a map.
struct MapPrinter<'a, K, V>(Cell<Option<Box<dyn Iterator<Item = (K, V)> + 'a>>>);
impl<'a, K, V> MapPrinter<'a, K, V> {
fn new(iter: impl Iterator<Item = (K, V)> + 'a) -> Self {
Expand All @@ -180,7 +180,7 @@ impl Debug for GeneratorLayout<'_> {
}
}

/// Prints the generator variant name.
// Prints the generator variant name.
struct GenVariantPrinter(VariantIdx);
impl From<VariantIdx> for GenVariantPrinter {
fn from(idx: VariantIdx) -> Self {
Expand All @@ -198,7 +198,7 @@ impl Debug for GeneratorLayout<'_> {
}
}

/// Forces its contents to print in regular mode instead of alternate mode.
// Forces its contents to print in regular mode instead of alternate mode.
struct OneLinePrinter<T>(T);
impl<T: Debug> Debug for OneLinePrinter<T> {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ pub struct ImmTy<'tcx, Tag = ()> {

impl<Tag: Copy> std::fmt::Display for ImmTy<'tcx, Tag> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
/// Helper function for printing a scalar to a FmtPrinter
// Helper function for printing a scalar to a FmtPrinter
fn p<'a, 'tcx, F: std::fmt::Write, Tag>(
cx: FmtPrinter<'a, 'tcx, F>,
s: ScalarMaybeUninit<Tag>,
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_mir/src/transform/simplify_try.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ fn get_arm_identity_info<'a, 'tcx>(
matches!(stmt.kind, StatementKind::StorageLive(_) | StatementKind::StorageDead(_))
}

/// Eats consecutive Statements which match `test`, performing the specified `action` for each.
/// The iterator `stmt_iter` is not advanced if none were matched.
// Eats consecutive Statements which match `test`, performing the specified `action` for each.
// The iterator `stmt_iter` is not advanced if none were matched.
fn try_eat<'a, 'tcx>(
stmt_iter: &mut StmtIter<'a, 'tcx>,
test: impl Fn(&'a Statement<'tcx>) -> bool,
Expand All @@ -120,8 +120,8 @@ fn get_arm_identity_info<'a, 'tcx>(
}
}

/// Eats consecutive `StorageLive` and `StorageDead` Statements.
/// The iterator `stmt_iter` is not advanced if none were found.
// Eats consecutive `StorageLive` and `StorageDead` Statements.
// The iterator `stmt_iter` is not advanced if none were found.
fn try_eat_storage_stmts<'a, 'tcx>(
stmt_iter: &mut StmtIter<'a, 'tcx>,
storage_live_stmts: &mut Vec<(usize, Local)>,
Expand All @@ -145,7 +145,7 @@ fn get_arm_identity_info<'a, 'tcx>(
}
}

/// Eats consecutive `Assign` Statements.
// Eats consecutive `Assign` Statements.
// The iterator `stmt_iter` is not advanced if none were found.
fn try_eat_assign_tmp_stmts<'a, 'tcx>(
stmt_iter: &mut StmtIter<'a, 'tcx>,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir_build/src/thir/pattern/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2431,9 +2431,9 @@ fn split_grouped_constructors<'p, 'tcx>(
continue;
}

/// Represents a border between 2 integers. Because the intervals spanning borders
/// must be able to cover every integer, we need to be able to represent
/// 2^128 + 1 such borders.
// Represents a border between 2 integers. Because the intervals spanning borders
// must be able to cover every integer, we need to be able to represent
// 2^128 + 1 such borders.
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug)]
enum Border {
JustBefore(u128),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/nonterminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ impl<'a> Parser<'a> {
/// Returning `false` is a *stability guarantee* that such a matcher will *never* begin with that
/// token. Be conservative (return true) if not sure.
pub fn nonterminal_may_begin_with(kind: NonterminalKind, token: &Token) -> bool {
/// Checks whether the non-terminal may contain a single (non-keyword) identifier.
// Checks whether the non-terminal may contain a single (non-keyword) identifier.
fn may_be_ident(nt: &token::Nonterminal) -> bool {
match *nt {
token::NtItem(_) | token::NtBlock(_) | token::NtVis(_) | token::NtLifetime(_) => {
Expand Down
Loading