From ddd544856ecd181ee02490d12f723be549d3ecb3 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 14 Jul 2021 18:24:12 -0500 Subject: [PATCH 1/5] Compute a better `lint_node_id` during expansion When we need to emit a lint at a macro invocation, we currently use the `NodeId` of its parent definition (e.g. the enclosing function). This means that any `#[allow]` / `#[deny]` attributes placed 'closer' to the macro (e.g. on an enclosing block or statement) will have no effect. This commit computes a better `lint_node_id` in `InvocationCollector`. When we visit/flat_map an AST node, we assign it a `NodeId` (earlier than we normally would), and store than `NodeId` in current `ExpansionData`. When we collect a macro invocation, the current `lint_node_id` gets cloned along with our `ExpansionData`, allowing it to be used if we need to emit a lint later on. This improves the handling of `#[allow]` / `#[deny]` for `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` and some `asm!`-related lints. The 'legacy derive helpers' lint retains its current behavior (I've inlined the now-removed `lint_node_id` function), since there isn't an `ExpansionData` readily available. --- compiler/rustc_builtin_macros/src/asm.rs | 4 +- .../rustc_builtin_macros/src/source_util.rs | 2 +- compiler/rustc_expand/src/base.rs | 6 +- compiler/rustc_expand/src/expand.rs | 100 ++++++++++++++---- compiler/rustc_expand/src/mbe/macro_rules.rs | 3 +- compiler/rustc_lint_defs/src/lib.rs | 4 +- compiler/rustc_resolve/src/macros.rs | 16 ++- src/test/ui/asm/inline-syntax.x86_64.stderr | 16 +-- .../semicolon-in-expressions-from-macros.rs | 24 +++-- ...emicolon-in-expressions-from-macros.stderr | 29 +++-- 10 files changed, 139 insertions(+), 65 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/asm.rs b/compiler/rustc_builtin_macros/src/asm.rs index 97e07d52cc31a..ff13f0d4e4207 100644 --- a/compiler/rustc_builtin_macros/src/asm.rs +++ b/compiler/rustc_builtin_macros/src/asm.rs @@ -455,7 +455,7 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, args: AsmArgs) -> Option, args: AsmArgs) -> Option( } } - Box::new(ExpandResult { p, node_id: cx.resolver.lint_node_id(cx.current_expansion.id) }) + Box::new(ExpandResult { p, node_id: cx.current_expansion.lint_node_id }) } // include_str! : read the given file, insert it as a literal string expr diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 497be2d931872..03ded6465d079 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -869,9 +869,6 @@ pub trait ResolverExpand { fn check_unused_macros(&mut self); - /// Some parent node that is close enough to the given macro call. - fn lint_node_id(&self, expn_id: LocalExpnId) -> NodeId; - // Resolver interfaces for specific built-in macros. /// Does `#[derive(...)]` attribute with the given `ExpnId` have built-in `Copy` inside it? fn has_derive_copy(&self, expn_id: LocalExpnId) -> bool; @@ -926,6 +923,8 @@ pub struct ExpansionData { pub module: Rc, pub dir_ownership: DirOwnership, pub prior_type_ascription: Option<(Span, bool)>, + /// Some parent node that is close to this macro call + pub lint_node_id: NodeId, } type OnExternModLoaded<'a> = @@ -971,6 +970,7 @@ impl<'a> ExtCtxt<'a> { module: Default::default(), dir_ownership: DirOwnership::Owned { relative: None }, prior_type_ascription: None, + lint_node_id: ast::CRATE_NODE_ID, }, force_mode: false, expansions: FxHashMap::default(), diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index b9d4096241142..f79e9648ab66e 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -1098,6 +1098,41 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { } } +/// Wraps a call to `noop_visit_*` / `noop_flat_map_*` +/// This method assigns a `NodeId`, and sets that `NodeId` +/// as our current 'lint node id'. If a macro call is found +/// inside this AST node, we will use this AST node's `NodeId` +/// to emit lints associated with that macro (allowing +/// `#[allow]` / `#[deny]` to be applied close to +/// the macro invocation). +/// +/// Do *not* call this for a macro AST node +/// (e.g. `ExprKind::MacCall`) - we cannot emit lints +/// at these AST nodes, since they are removed and +/// replaced with the result of macro expansion. +/// +/// All other `NodeId`s are assigned by `visit_id`. +/// * `self` is the 'self' parameter for the current method, +/// * `id` is a mutable reference to the `NodeId` field +/// of the current AST node. +/// * `closure` is a closure that executes the +/// `noop_visit_*` / `noop_flat_map_*` method +/// for the current AST node. +macro_rules! assign_id { + ($self:ident, $id:expr, $closure:expr) => {{ + let old_id = $self.cx.current_expansion.lint_node_id; + if $self.monotonic { + debug_assert_eq!(*$id, ast::DUMMY_NODE_ID); + let new_id = $self.cx.resolver.next_node_id(); + *$id = new_id; + $self.cx.current_expansion.lint_node_id = new_id; + } + let ret = ($closure)(); + $self.cx.current_expansion.lint_node_id = old_id; + ret + }}; +} + impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { fn visit_expr(&mut self, expr: &mut P) { self.cfg.configure_expr(expr); @@ -1118,7 +1153,9 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { self.check_attributes(&expr.attrs); self.collect_bang(mac, expr.span, AstFragmentKind::Expr).make_expr().into_inner() } else { - ensure_sufficient_stack(|| noop_visit_expr(&mut expr, self)); + assign_id!(self, &mut expr.id, || { + ensure_sufficient_stack(|| noop_visit_expr(&mut expr, self)); + }); expr } }); @@ -1133,7 +1170,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { .make_arms(); } - noop_flat_map_arm(arm, self) + assign_id!(self, &mut arm.id, || noop_flat_map_arm(arm, self)) } fn flat_map_expr_field(&mut self, field: ast::ExprField) -> SmallVec<[ast::ExprField; 1]> { @@ -1145,7 +1182,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { .make_expr_fields(); } - noop_flat_map_expr_field(field, self) + assign_id!(self, &mut field.id, || noop_flat_map_expr_field(field, self)) } fn flat_map_pat_field(&mut self, fp: ast::PatField) -> SmallVec<[ast::PatField; 1]> { @@ -1157,7 +1194,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { .make_pat_fields(); } - noop_flat_map_pat_field(fp, self) + assign_id!(self, &mut fp.id, || noop_flat_map_pat_field(fp, self)) } fn flat_map_param(&mut self, p: ast::Param) -> SmallVec<[ast::Param; 1]> { @@ -1169,7 +1206,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { .make_params(); } - noop_flat_map_param(p, self) + assign_id!(self, &mut p.id, || noop_flat_map_param(p, self)) } fn flat_map_field_def(&mut self, sf: ast::FieldDef) -> SmallVec<[ast::FieldDef; 1]> { @@ -1181,7 +1218,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { .make_field_defs(); } - noop_flat_map_field_def(sf, self) + assign_id!(self, &mut sf.id, || noop_flat_map_field_def(sf, self)) } fn flat_map_variant(&mut self, variant: ast::Variant) -> SmallVec<[ast::Variant; 1]> { @@ -1193,7 +1230,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { .make_variants(); } - noop_flat_map_variant(variant, self) + assign_id!(self, &mut variant.id, || noop_flat_map_variant(variant, self)) } fn filter_map_expr(&mut self, expr: P) -> Option> { @@ -1214,9 +1251,11 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { .make_opt_expr() .map(|expr| expr.into_inner()) } else { - Some({ - noop_visit_expr(&mut expr, self); - expr + assign_id!(self, &mut expr.id, || { + Some({ + noop_visit_expr(&mut expr, self); + expr + }) }) } }) @@ -1225,7 +1264,9 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { fn visit_pat(&mut self, pat: &mut P) { match pat.kind { PatKind::MacCall(_) => {} - _ => return noop_visit_pat(pat, self), + _ => { + return assign_id!(self, &mut pat.id, || noop_visit_pat(pat, self)); + } } visit_clobber(pat, |mut pat| match mem::replace(&mut pat.kind, PatKind::Wild) { @@ -1278,7 +1319,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { &mut self.cx.current_expansion.dir_ownership, DirOwnership::UnownedViaBlock, ); - noop_visit_block(block, self); + assign_id!(self, &mut block.id, || noop_visit_block(block, self)); self.cx.current_expansion.dir_ownership = orig_dir_ownership; } @@ -1377,7 +1418,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { let orig_dir_ownership = mem::replace(&mut self.cx.current_expansion.dir_ownership, dir_ownership); - let result = noop_flat_map_item(item, self); + let result = assign_id!(self, &mut item.id, || noop_flat_map_item(item, self)); // Restore the module info. self.cx.current_expansion.dir_ownership = orig_dir_ownership; @@ -1387,7 +1428,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { } _ => { item.attrs = attrs; - noop_flat_map_item(item, self) + // The crate root is special - don't assign an ID to it. + if !(matches!(item.kind, ast::ItemKind::Mod(..)) && ident == Ident::invalid()) { + assign_id!(self, &mut item.id, || noop_flat_map_item(item, self)) + } else { + noop_flat_map_item(item, self) + } } } } @@ -1411,7 +1457,9 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { _ => unreachable!(), }) } - _ => noop_flat_map_assoc_item(item, self), + _ => { + assign_id!(self, &mut item.id, || noop_flat_map_assoc_item(item, self)) + } } } @@ -1434,14 +1482,16 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { _ => unreachable!(), }) } - _ => noop_flat_map_assoc_item(item, self), + _ => { + assign_id!(self, &mut item.id, || noop_flat_map_assoc_item(item, self)) + } } } fn visit_ty(&mut self, ty: &mut P) { match ty.kind { ast::TyKind::MacCall(_) => {} - _ => return noop_visit_ty(ty, self), + _ => return assign_id!(self, &mut ty.id, || noop_visit_ty(ty, self)), }; visit_clobber(ty, |mut ty| match mem::replace(&mut ty.kind, ast::TyKind::Err) { @@ -1478,7 +1528,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { _ => unreachable!(), }) } - _ => noop_flat_map_foreign_item(foreign_item, self), + _ => { + assign_id!(self, &mut foreign_item.id, || noop_flat_map_foreign_item( + foreign_item, + self + )) + } } } @@ -1498,13 +1553,14 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { .make_generic_params(); } - noop_flat_map_generic_param(param, self) + assign_id!(self, &mut param.id, || noop_flat_map_generic_param(param, self)) } fn visit_id(&mut self, id: &mut ast::NodeId) { - if self.monotonic { - debug_assert_eq!(*id, ast::DUMMY_NODE_ID); - *id = self.cx.resolver.next_node_id() + // We may have already assigned a `NodeId` + // by calling `assign_id` + if self.monotonic && *id == ast::DUMMY_NODE_ID { + *id = self.cx.resolver.next_node_id(); } } } diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 8b68c94e61a38..7f985af364d7d 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -289,7 +289,6 @@ fn generic_extension<'cx>( let mut p = Parser::new(sess, tts, false, None); p.last_type_ascription = cx.current_expansion.prior_type_ascription; - let lint_node_id = cx.resolver.lint_node_id(cx.current_expansion.id); // Let the context choose how to interpret the result. // Weird, but useful for X-macros. @@ -301,7 +300,7 @@ fn generic_extension<'cx>( // macro leaves unparsed tokens. site_span: sp, macro_ident: name, - lint_node_id, + lint_node_id: cx.current_expansion.lint_node_id, arm_span, }); } diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index 001198226d9a3..4d85bf6b499d9 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -274,7 +274,7 @@ impl ToStableHashKey for LintId { } // Duplicated from rustc_session::config::ExternDepSpec to avoid cyclic dependency -#[derive(PartialEq)] +#[derive(PartialEq, Debug)] pub enum ExternDepSpec { Json(Json), Raw(String), @@ -282,7 +282,7 @@ pub enum ExternDepSpec { // This could be a closure, but then implementing derive trait // becomes hacky (and it gets allocated). -#[derive(PartialEq)] +#[derive(PartialEq, Debug)] pub enum BuiltinLintDiagnostics { Normal, BareTraitObject(Span, /* is_global */ bool), diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 86f271fdeceb8..b2a8aa0ceccaa 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -281,7 +281,7 @@ impl<'a> ResolverExpand for Resolver<'a> { // Derives are not included when `invocations` are collected, so we have to add them here. let parent_scope = &ParentScope { derives, ..parent_scope }; let supports_macro_expansion = invoc.fragment_kind.supports_macro_expansion(); - let node_id = self.lint_node_id(eager_expansion_root); + let node_id = invoc.expansion_data.lint_node_id; let (ext, res) = self.smart_resolve_macro_path( path, kind, @@ -348,14 +348,6 @@ impl<'a> ResolverExpand for Resolver<'a> { } } - fn lint_node_id(&self, expn_id: LocalExpnId) -> NodeId { - // FIXME - make this more precise. This currently returns the NodeId of the - // nearest closing item - we should try to return the closest parent of the ExpnId - self.invocation_parents - .get(&expn_id) - .map_or(ast::CRATE_NODE_ID, |id| self.def_id_to_node_id[id.0]) - } - fn has_derive_copy(&self, expn_id: LocalExpnId) -> bool { self.containers_deriving_copy.contains(&expn_id) } @@ -1105,9 +1097,13 @@ impl<'a> Resolver<'a> { let seg = Segment::from_ident(ident); check_consistency(self, &[seg], ident.span, kind, initial_res, res); if res == Res::NonMacroAttr(NonMacroAttrKind::DeriveHelperCompat) { + let node_id = self + .invocation_parents + .get(&parent_scope.expansion) + .map_or(ast::CRATE_NODE_ID, |id| self.def_id_to_node_id[id.0]); self.lint_buffer.buffer_lint_with_diagnostic( LEGACY_DERIVE_HELPERS, - self.lint_node_id(parent_scope.expansion), + node_id, ident.span, "derive helper attribute is used before it is introduced", BuiltinLintDiagnostics::LegacyDeriveHelpers(binding.span), diff --git a/src/test/ui/asm/inline-syntax.x86_64.stderr b/src/test/ui/asm/inline-syntax.x86_64.stderr index dcbc17bb26089..a0e2a5ea0ef97 100644 --- a/src/test/ui/asm/inline-syntax.x86_64.stderr +++ b/src/test/ui/asm/inline-syntax.x86_64.stderr @@ -1,16 +1,10 @@ -warning: avoid using `.intel_syntax`, Intel syntax is the default - --> $DIR/inline-syntax.rs:57:14 - | -LL | global_asm!(".intel_syntax noprefix", "nop"); - | ^^^^^^^^^^^^^^^^^^^^^^ - | - = note: `#[warn(bad_asm_style)]` on by default - warning: avoid using `.intel_syntax`, Intel syntax is the default --> $DIR/inline-syntax.rs:31:15 | LL | asm!(".intel_syntax noprefix", "nop"); | ^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(bad_asm_style)]` on by default warning: avoid using `.intel_syntax`, Intel syntax is the default --> $DIR/inline-syntax.rs:34:15 @@ -42,5 +36,11 @@ warning: avoid using `.intel_syntax`, Intel syntax is the default LL | .intel_syntax noprefix | ^^^^^^^^^^^^^^^^^^^^^^ +warning: avoid using `.intel_syntax`, Intel syntax is the default + --> $DIR/inline-syntax.rs:57:14 + | +LL | global_asm!(".intel_syntax noprefix", "nop"); + | ^^^^^^^^^^^^^^^^^^^^^^ + warning: 7 warnings emitted diff --git a/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.rs b/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.rs index 0bbd7dc6c8a7a..fff380934e8e9 100644 --- a/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.rs +++ b/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.rs @@ -1,14 +1,17 @@ // check-pass // edition:2018 +#![feature(stmt_expr_attributes)] #![warn(semicolon_in_expressions_from_macros)] #[allow(dead_code)] macro_rules! foo { ($val:ident) => { - true; //~ WARN trailing - //~| WARN this was previously - //~| WARN trailing - //~| WARN this was previously + true; //~ WARN trailing semicolon in macro + //~| WARN this was previously accepted + //~| WARN trailing semicolon in macro + //~| WARN this was previously accepted + //~| WARN trailing semicolon in macro + //~| WARN this was previously accepted } } @@ -18,17 +21,14 @@ async fn bar() { } fn main() { - // This `allow` doesn't work #[allow(semicolon_in_expressions_from_macros)] let _ = { foo!(first) }; - // This 'allow' doesn't work either #[allow(semicolon_in_expressions_from_macros)] let _ = foo!(second); - // But this 'allow' does #[allow(semicolon_in_expressions_from_macros)] fn inner() { let _ = foo!(third); @@ -38,4 +38,14 @@ fn main() { async { let _ = foo!(fourth); }; + + let _ = { + foo!(warn_in_block) + }; + + let _ = foo!(warn_in_expr); + + // This `#[allow]` does not work, since the attribute gets dropped + // when we expand the macro + let _ = #[allow(semicolon_in_expressions_from_macros)] foo!(allow_does_not_work); } diff --git a/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.stderr b/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.stderr index 111ebea61dd12..c00c3d77dcedc 100644 --- a/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.stderr +++ b/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.stderr @@ -1,14 +1,14 @@ warning: trailing semicolon in macro used in expression position - --> $DIR/semicolon-in-expressions-from-macros.rs:8:13 + --> $DIR/semicolon-in-expressions-from-macros.rs:9:13 | LL | true; | ^ ... -LL | foo!(first) - | ----------- in this macro invocation +LL | foo!(warn_in_block) + | ------------------- in this macro invocation | note: the lint level is defined here - --> $DIR/semicolon-in-expressions-from-macros.rs:3:9 + --> $DIR/semicolon-in-expressions-from-macros.rs:4:9 | LL | #![warn(semicolon_in_expressions_from_macros)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,17 +17,30 @@ LL | #![warn(semicolon_in_expressions_from_macros)] = note: this warning originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info) warning: trailing semicolon in macro used in expression position - --> $DIR/semicolon-in-expressions-from-macros.rs:8:13 + --> $DIR/semicolon-in-expressions-from-macros.rs:9:13 | LL | true; | ^ ... -LL | let _ = foo!(second); - | ------------ in this macro invocation +LL | let _ = foo!(warn_in_expr); + | ------------------ in this macro invocation | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #79813 = note: this warning originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info) -warning: 2 warnings emitted +warning: trailing semicolon in macro used in expression position + --> $DIR/semicolon-in-expressions-from-macros.rs:9:13 + | +LL | true; + | ^ +... +LL | let _ = #[allow(semicolon_in_expressions_from_macros)] foo!(allow_does_not_work); + | ------------------------- in this macro invocation + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #79813 + = note: this warning originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info) + +warning: 3 warnings emitted From 2bd15a25ef24949abbcfe066c04cd2a266410c47 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 14 Jul 2021 20:07:56 -0500 Subject: [PATCH 2/5] Add missing `visit_expr_field` --- compiler/rustc_lint/src/early.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_lint/src/early.rs b/compiler/rustc_lint/src/early.rs index eb2e495f73d3c..63e2f66f8102b 100644 --- a/compiler/rustc_lint/src/early.rs +++ b/compiler/rustc_lint/src/early.rs @@ -120,6 +120,12 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> }) } + fn visit_expr_field(&mut self, f: &'a ast::ExprField) { + self.with_lint_attrs(f.id, &f.attrs, |cx| { + ast_visit::walk_expr_field(cx, f); + }) + } + fn visit_stmt(&mut self, s: &'a ast::Stmt) { // Add the statement's lint attributes to our // current state when checking the statement itself. @@ -389,9 +395,15 @@ pub fn check_ast_crate( // All of the buffered lints should have been emitted at this point. // If not, that means that we somehow buffered a lint for a node id // that was not lint-checked (perhaps it doesn't exist?). This is a bug. - for (_id, lints) in buffered.map { + for (id, lints) in buffered.map { for early_lint in lints { - sess.delay_span_bug(early_lint.span, "failed to process buffered lint here"); + sess.delay_span_bug( + early_lint.span, + &format!( + "failed to process buffered lint here (dummy = {})", + id == ast::DUMMY_NODE_ID + ), + ); } } } From d6e3c111011f6a270d56fdaf5222b484c4f38d65 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 15 Jul 2021 14:26:27 -0500 Subject: [PATCH 3/5] Add additional missing lint handling logic --- compiler/rustc_expand/src/expand.rs | 9 ++++++++- compiler/rustc_lint/src/early.rs | 6 ++++-- src/test/ui/asm/inline-syntax.x86_64.stderr | 16 ++++++++-------- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index f79e9648ab66e..208894c3791bb 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -12,7 +12,7 @@ use rustc_ast::ptr::P; use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; use rustc_ast::visit::{self, AssocCtxt, Visitor}; -use rustc_ast::{AstLike, Block, Inline, ItemKind, MacArgs}; +use rustc_ast::{AstLike, Block, Inline, ItemKind, Local, MacArgs}; use rustc_ast::{MacCallStmt, MacStmtStyle, MetaItemKind, ModKind, NestedMetaItem}; use rustc_ast::{NodeId, PatKind, Path, StmtKind, Unsafe}; use rustc_ast_pretty::pprust; @@ -1161,6 +1161,11 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { }); } + // This is needed in order to set `lint_node_id` for `let` statements + fn visit_local(&mut self, local: &mut P) { + assign_id!(self, &mut local.id, || noop_visit_local(local, self)); + } + fn flat_map_arm(&mut self, arm: ast::Arm) -> SmallVec<[ast::Arm; 1]> { let mut arm = configure!(self, arm); @@ -1307,6 +1312,8 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { } // The placeholder expander gives ids to statements, so we avoid folding the id here. + // We don't use `assign_id!` - it will be called when we visit statement's contents + // (e.g. an expression, item, or local) let ast::Stmt { id, kind, span } = stmt; noop_flat_map_stmt_kind(kind, self) .into_iter() diff --git a/compiler/rustc_lint/src/early.rs b/compiler/rustc_lint/src/early.rs index 63e2f66f8102b..7a8b731da5c2e 100644 --- a/compiler/rustc_lint/src/early.rs +++ b/compiler/rustc_lint/src/early.rs @@ -210,8 +210,10 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> } fn visit_arm(&mut self, a: &'a ast::Arm) { - run_early_pass!(self, check_arm, a); - ast_visit::walk_arm(self, a); + self.with_lint_attrs(a.id, &a.attrs, |cx| { + run_early_pass!(cx, check_arm, a); + ast_visit::walk_arm(cx, a); + }) } fn visit_expr_post(&mut self, e: &'a ast::Expr) { diff --git a/src/test/ui/asm/inline-syntax.x86_64.stderr b/src/test/ui/asm/inline-syntax.x86_64.stderr index a0e2a5ea0ef97..dcbc17bb26089 100644 --- a/src/test/ui/asm/inline-syntax.x86_64.stderr +++ b/src/test/ui/asm/inline-syntax.x86_64.stderr @@ -1,10 +1,16 @@ +warning: avoid using `.intel_syntax`, Intel syntax is the default + --> $DIR/inline-syntax.rs:57:14 + | +LL | global_asm!(".intel_syntax noprefix", "nop"); + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(bad_asm_style)]` on by default + warning: avoid using `.intel_syntax`, Intel syntax is the default --> $DIR/inline-syntax.rs:31:15 | LL | asm!(".intel_syntax noprefix", "nop"); | ^^^^^^^^^^^^^^^^^^^^^^ - | - = note: `#[warn(bad_asm_style)]` on by default warning: avoid using `.intel_syntax`, Intel syntax is the default --> $DIR/inline-syntax.rs:34:15 @@ -36,11 +42,5 @@ warning: avoid using `.intel_syntax`, Intel syntax is the default LL | .intel_syntax noprefix | ^^^^^^^^^^^^^^^^^^^^^^ -warning: avoid using `.intel_syntax`, Intel syntax is the default - --> $DIR/inline-syntax.rs:57:14 - | -LL | global_asm!(".intel_syntax noprefix", "nop"); - | ^^^^^^^^^^^^^^^^^^^^^^ - warning: 7 warnings emitted From 7ca089c6d2e834ea007d7a62b8bbbe0c880de4a2 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sat, 17 Jul 2021 08:22:09 -0500 Subject: [PATCH 4/5] Only use `assign_id!` for ast nodes that support attributes --- compiler/rustc_expand/src/base.rs | 3 +++ compiler/rustc_expand/src/expand.rs | 10 +++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 03ded6465d079..8c6aef80635cf 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -29,6 +29,9 @@ use std::rc::Rc; crate use rustc_span::hygiene::MacroKind; +// When adding new variants, make sure to +// adjust the `visit_*` / `flat_map_*` calls in `InvocationCollector` +// to use `assign_id!` #[derive(Debug, Clone)] pub enum Annotatable { Item(P), diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 208894c3791bb..dcd871c9d2050 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -1099,6 +1099,8 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { } /// Wraps a call to `noop_visit_*` / `noop_flat_map_*` +/// for an AST node that supports attributes +/// (see the `Annotatable` enum) /// This method assigns a `NodeId`, and sets that `NodeId` /// as our current 'lint node id'. If a macro call is found /// inside this AST node, we will use this AST node's `NodeId` @@ -1269,9 +1271,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { fn visit_pat(&mut self, pat: &mut P) { match pat.kind { PatKind::MacCall(_) => {} - _ => { - return assign_id!(self, &mut pat.id, || noop_visit_pat(pat, self)); - } + _ => return noop_visit_pat(pat, self), } visit_clobber(pat, |mut pat| match mem::replace(&mut pat.kind, PatKind::Wild) { @@ -1326,7 +1326,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { &mut self.cx.current_expansion.dir_ownership, DirOwnership::UnownedViaBlock, ); - assign_id!(self, &mut block.id, || noop_visit_block(block, self)); + noop_visit_block(block, self); self.cx.current_expansion.dir_ownership = orig_dir_ownership; } @@ -1498,7 +1498,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { fn visit_ty(&mut self, ty: &mut P) { match ty.kind { ast::TyKind::MacCall(_) => {} - _ => return assign_id!(self, &mut ty.id, || noop_visit_ty(ty, self)), + _ => return noop_visit_ty(ty, self), }; visit_clobber(ty, |mut ty| match mem::replace(&mut ty.kind, ast::TyKind::Err) { From 1c1c7949ab6a8d4150b7e645ae4779677e86ebd1 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sat, 17 Jul 2021 08:31:13 -0500 Subject: [PATCH 5/5] Add test for `#[allow]` for warnings on attribute macro --- .../proc-macro/auxiliary/call-deprecated.rs | 19 +++++++++++ src/test/ui/proc-macro/call-deprecated.rs | 34 +++++++++++++++++++ src/test/ui/proc-macro/call-deprecated.stderr | 16 +++++++++ 3 files changed, 69 insertions(+) create mode 100644 src/test/ui/proc-macro/auxiliary/call-deprecated.rs create mode 100644 src/test/ui/proc-macro/call-deprecated.rs create mode 100644 src/test/ui/proc-macro/call-deprecated.stderr diff --git a/src/test/ui/proc-macro/auxiliary/call-deprecated.rs b/src/test/ui/proc-macro/auxiliary/call-deprecated.rs new file mode 100644 index 0000000000000..2f484809a5c99 --- /dev/null +++ b/src/test/ui/proc-macro/auxiliary/call-deprecated.rs @@ -0,0 +1,19 @@ +// force-host +// no-prefer-dynamic + +#![crate_type = "proc-macro"] + +extern crate proc_macro; +use proc_macro::*; + +#[proc_macro_attribute] +#[deprecated(since = "1.0.0", note = "test")] +pub fn attr(_: TokenStream, input: TokenStream) -> TokenStream { + input +} + +#[proc_macro_attribute] +#[deprecated(since = "1.0.0", note = "test")] +pub fn attr_remove(_: TokenStream, _: TokenStream) -> TokenStream { + TokenStream::new() +} diff --git a/src/test/ui/proc-macro/call-deprecated.rs b/src/test/ui/proc-macro/call-deprecated.rs new file mode 100644 index 0000000000000..b92cc23638ae1 --- /dev/null +++ b/src/test/ui/proc-macro/call-deprecated.rs @@ -0,0 +1,34 @@ +// check-pass +// aux-build:call-deprecated.rs + +extern crate call_deprecated; + +// These first two `#[allow(deprecated)]` attributes +// do nothing, since the AST nodes for `First` and `Second` +// haven't been been assigned a `NodeId`. +// See #63221 for a discussion about how we should +// handle the interaction of 'inert' attributes and +// proc-macro attributes. + +#[allow(deprecated)] +#[call_deprecated::attr] //~ WARN use of deprecated macro +struct First; + +#[allow(deprecated)] +#[call_deprecated::attr_remove] //~ WARN use of deprecated macro +struct Second; + +#[allow(deprecated)] +mod bar { + #[allow(deprecated)] + #[call_deprecated::attr] + struct Third; + + #[allow(deprecated)] + #[call_deprecated::attr_remove] + struct Fourth; +} + + +fn main() { +} diff --git a/src/test/ui/proc-macro/call-deprecated.stderr b/src/test/ui/proc-macro/call-deprecated.stderr new file mode 100644 index 0000000000000..3506f9a16a3c4 --- /dev/null +++ b/src/test/ui/proc-macro/call-deprecated.stderr @@ -0,0 +1,16 @@ +warning: use of deprecated macro `call_deprecated::attr`: test + --> $DIR/call-deprecated.rs:14:3 + | +LL | #[call_deprecated::attr] + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(deprecated)]` on by default + +warning: use of deprecated macro `call_deprecated::attr_remove`: test + --> $DIR/call-deprecated.rs:18:3 + | +LL | #[call_deprecated::attr_remove] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: 2 warnings emitted +