Skip to content

Commit

Permalink
Rollup merge of #78710 - petrochenkov:macvisit, r=davidtwco
Browse files Browse the repository at this point in the history
rustc_ast: Do not panic by default when visiting macro calls

Panicking by default made sense when we didn't have HIR or MIR and everything worked on AST, but now all AST visitors run early and majority of them have to deal with macro calls, often by ignoring them.

The second commit renames `visit_mac` to `visit_mac_call`, the corresponding structures were renamed earlier in #69589.
  • Loading branch information
Dylan-DPC authored Nov 9, 2020
2 parents ee1fedf + 90fafc8 commit 8ebca24
Show file tree
Hide file tree
Showing 18 changed files with 29 additions and 93 deletions.
21 changes: 9 additions & 12 deletions compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,8 @@ pub trait MutVisitor: Sized {
noop_visit_local(l, self);
}

fn visit_mac(&mut self, _mac: &mut MacCall) {
panic!("visit_mac disabled by default");
// N.B., see note about macros above. If you really want a visitor that
// works on macros, use this definition in your trait impl:
// mut_visit::noop_visit_mac(_mac, self);
fn visit_mac_call(&mut self, mac: &mut MacCall) {
noop_visit_mac(mac, self);
}

fn visit_macro_def(&mut self, def: &mut MacroDef) {
Expand Down Expand Up @@ -494,7 +491,7 @@ pub fn noop_visit_ty<T: MutVisitor>(ty: &mut P<Ty>, vis: &mut T) {
vis.visit_id(id);
visit_vec(bounds, |bound| vis.visit_param_bound(bound));
}
TyKind::MacCall(mac) => vis.visit_mac(mac),
TyKind::MacCall(mac) => vis.visit_mac_call(mac),
}
vis.visit_span(span);
visit_lazy_tts(tokens, vis);
Expand Down Expand Up @@ -962,7 +959,7 @@ pub fn noop_visit_item_kind<T: MutVisitor>(kind: &mut ItemKind, vis: &mut T) {
vis.visit_generics(generics);
visit_bounds(bounds, vis);
}
ItemKind::MacCall(m) => vis.visit_mac(m),
ItemKind::MacCall(m) => vis.visit_mac_call(m),
ItemKind::MacroDef(def) => vis.visit_macro_def(def),
}
}
Expand Down Expand Up @@ -991,7 +988,7 @@ pub fn noop_flat_map_assoc_item<T: MutVisitor>(
visit_bounds(bounds, visitor);
visit_opt(ty, |ty| visitor.visit_ty(ty));
}
AssocItemKind::MacCall(mac) => visitor.visit_mac(mac),
AssocItemKind::MacCall(mac) => visitor.visit_mac_call(mac),
}
visitor.visit_span(span);
visit_lazy_tts(tokens, visitor);
Expand Down Expand Up @@ -1081,7 +1078,7 @@ pub fn noop_flat_map_foreign_item<T: MutVisitor>(
visit_bounds(bounds, visitor);
visit_opt(ty, |ty| visitor.visit_ty(ty));
}
ForeignItemKind::MacCall(mac) => visitor.visit_mac(mac),
ForeignItemKind::MacCall(mac) => visitor.visit_mac_call(mac),
}
visitor.visit_span(span);
visit_lazy_tts(tokens, visitor);
Expand Down Expand Up @@ -1121,7 +1118,7 @@ pub fn noop_visit_pat<T: MutVisitor>(pat: &mut P<Pat>, vis: &mut T) {
visit_vec(elems, |elem| vis.visit_pat(elem))
}
PatKind::Paren(inner) => vis.visit_pat(inner),
PatKind::MacCall(mac) => vis.visit_mac(mac),
PatKind::MacCall(mac) => vis.visit_mac_call(mac),
}
vis.visit_span(span);
visit_lazy_tts(tokens, vis);
Expand Down Expand Up @@ -1287,7 +1284,7 @@ pub fn noop_visit_expr<T: MutVisitor>(
}
visit_vec(inputs, |(_c, expr)| vis.visit_expr(expr));
}
ExprKind::MacCall(mac) => vis.visit_mac(mac),
ExprKind::MacCall(mac) => vis.visit_mac_call(mac),
ExprKind::Struct(path, fields, expr) => {
vis.visit_path(path);
fields.flat_map_in_place(|field| vis.flat_map_field(field));
Expand Down Expand Up @@ -1350,7 +1347,7 @@ pub fn noop_flat_map_stmt_kind<T: MutVisitor>(
StmtKind::Empty => smallvec![StmtKind::Empty],
StmtKind::MacCall(mut mac) => {
let MacCallStmt { mac: mac_, style: _, attrs } = mac.deref_mut();
vis.visit_mac(mac_);
vis.visit_mac_call(mac_);
visit_thin_attrs(attrs, vis);
smallvec![StmtKind::MacCall(mac)]
}
Expand Down
23 changes: 9 additions & 14 deletions compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,8 @@ pub trait Visitor<'ast>: Sized {
fn visit_lifetime(&mut self, lifetime: &'ast Lifetime) {
walk_lifetime(self, lifetime)
}
fn visit_mac(&mut self, _mac: &'ast MacCall) {
panic!("visit_mac disabled by default");
// N.B., see note about macros above.
// if you really want a visitor that
// works on macros, use this
// definition in your trait impl:
// visit::walk_mac(self, _mac)
fn visit_mac_call(&mut self, mac: &'ast MacCall) {
walk_mac(self, mac)
}
fn visit_mac_def(&mut self, _mac: &'ast MacroDef, _id: NodeId) {
// Nothing to do
Expand Down Expand Up @@ -346,7 +341,7 @@ pub fn walk_item<'a, V: Visitor<'a>>(visitor: &mut V, item: &'a Item) {
visitor.visit_generics(generics);
walk_list!(visitor, visit_param_bound, bounds);
}
ItemKind::MacCall(ref mac) => visitor.visit_mac(mac),
ItemKind::MacCall(ref mac) => visitor.visit_mac_call(mac),
ItemKind::MacroDef(ref ts) => visitor.visit_mac_def(ts, item.id),
}
walk_list!(visitor, visit_attribute, &item.attrs);
Expand Down Expand Up @@ -414,7 +409,7 @@ pub fn walk_ty<'a, V: Visitor<'a>>(visitor: &mut V, typ: &'a Ty) {
}
TyKind::Typeof(ref expression) => visitor.visit_anon_const(expression),
TyKind::Infer | TyKind::ImplicitSelf | TyKind::Err => {}
TyKind::MacCall(ref mac) => visitor.visit_mac(mac),
TyKind::MacCall(ref mac) => visitor.visit_mac_call(mac),
TyKind::Never | TyKind::CVarArgs => {}
}
}
Expand Down Expand Up @@ -532,7 +527,7 @@ pub fn walk_pat<'a, V: Visitor<'a>>(visitor: &mut V, pattern: &'a Pat) {
PatKind::Tuple(ref elems) | PatKind::Slice(ref elems) | PatKind::Or(ref elems) => {
walk_list!(visitor, visit_pat, elems);
}
PatKind::MacCall(ref mac) => visitor.visit_mac(mac),
PatKind::MacCall(ref mac) => visitor.visit_mac_call(mac),
}
}

Expand All @@ -557,7 +552,7 @@ pub fn walk_foreign_item<'a, V: Visitor<'a>>(visitor: &mut V, item: &'a ForeignI
walk_list!(visitor, visit_ty, ty);
}
ForeignItemKind::MacCall(mac) => {
visitor.visit_mac(mac);
visitor.visit_mac_call(mac);
}
}
}
Expand Down Expand Up @@ -662,7 +657,7 @@ pub fn walk_assoc_item<'a, V: Visitor<'a>>(visitor: &mut V, item: &'a AssocItem,
walk_list!(visitor, visit_ty, ty);
}
AssocItemKind::MacCall(mac) => {
visitor.visit_mac(mac);
visitor.visit_mac_call(mac);
}
}
}
Expand Down Expand Up @@ -692,7 +687,7 @@ pub fn walk_stmt<'a, V: Visitor<'a>>(visitor: &mut V, statement: &'a Stmt) {
StmtKind::Empty => {}
StmtKind::MacCall(ref mac) => {
let MacCallStmt { ref mac, style: _, ref attrs } = **mac;
visitor.visit_mac(mac);
visitor.visit_mac_call(mac);
for attr in attrs.iter() {
visitor.visit_attribute(attr);
}
Expand Down Expand Up @@ -823,7 +818,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
ExprKind::Ret(ref optional_expression) => {
walk_list!(visitor, visit_expr, optional_expression);
}
ExprKind::MacCall(ref mac) => visitor.visit_mac(mac),
ExprKind::MacCall(ref mac) => visitor.visit_mac_call(mac),
ExprKind::Paren(ref subexpression) => visitor.visit_expr(subexpression),
ExprKind::InlineAsm(ref ia) => {
for (op, _) in &ia.operands {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ast_passes/src/node_count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ impl<'ast> Visitor<'ast> for NodeCounter {
self.count += 1;
walk_lifetime(self, lifetime)
}
fn visit_mac(&mut self, _mac: &MacCall) {
fn visit_mac_call(&mut self, mac: &MacCall) {
self.count += 1;
walk_mac(self, _mac)
walk_mac(self, mac)
}
fn visit_path(&mut self, path: &Path, _id: NodeId) {
self.count += 1;
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_ast_passes/src/show_span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@ impl<'a> Visitor<'a> for ShowSpanVisitor<'a> {
}
visit::walk_ty(self, t);
}

fn visit_mac(&mut self, mac: &'a ast::MacCall) {
visit::walk_mac(self, mac);
}
}

pub fn run(span_diagnostic: &rustc_errors::Handler, mode: &str, krate: &ast::Crate) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ fn find_type_parameters(
visit::walk_ty(self, ty)
}

fn visit_mac(&mut self, mac: &ast::MacCall) {
fn visit_mac_call(&mut self, mac: &ast::MacCall) {
self.cx.span_err(mac.span(), "`derive` cannot be used on items with type macros");
}
}
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_builtin_macros/src/proc_macro_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,10 +344,6 @@ impl<'a> Visitor<'a> for CollectProcMacros<'a> {
visit::walk_item(self, item);
self.in_root = prev_in_root;
}

fn visit_mac(&mut self, mac: &'a ast::MacCall) {
visit::walk_mac(self, mac)
}
}

// Creates a new module which looks like:
Expand Down
8 changes: 0 additions & 8 deletions compiler/rustc_builtin_macros/src/test_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,6 @@ impl<'a> MutVisitor for TestHarnessGenerator<'a> {
}
smallvec![P(item)]
}

fn visit_mac(&mut self, _mac: &mut ast::MacCall) {
// Do nothing.
}
}

// Beware, this is duplicated in librustc_passes/entry.rs (with
Expand Down Expand Up @@ -201,10 +197,6 @@ impl<'a> MutVisitor for EntryPointCleaner<'a> {

smallvec![item]
}

fn visit_mac(&mut self, _mac: &mut ast::MacCall) {
// Do nothing.
}
}

/// Crawl over the crate, inserting test reexports and the test main function
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_expand/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,11 +547,6 @@ impl<'a> MutVisitor for StripUnconfigured<'a> {
noop_flat_map_assoc_item(configure!(self, item), self)
}

fn visit_mac(&mut self, _mac: &mut ast::MacCall) {
// Don't configure interpolated AST (cf. issue #34171).
// Interpolated AST will get configured once the surrounding tokens are parsed.
}

fn visit_pat(&mut self, pat: &mut P<ast::Pat>) {
self.configure_pat(pat);
noop_visit_pat(pat, self)
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -850,8 +850,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> {

visit::walk_item(self, item);
}

fn visit_mac(&mut self, _: &'ast ast::MacCall) {}
}

if !self.cx.ecfg.proc_macro_hygiene() {
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_expand/src/mbe/transcribe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::mbe::macro_parser::{MatchedNonterminal, MatchedSeq, NamedMatch};
use rustc_ast::mut_visit::{self, MutVisitor};
use rustc_ast::token::{self, NtTT, Token};
use rustc_ast::tokenstream::{DelimSpan, TokenStream, TokenTree, TreeAndSpacing};
use rustc_ast::MacCall;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::sync::Lrc;
use rustc_errors::{pluralize, PResult};
Expand All @@ -27,10 +26,6 @@ impl MutVisitor for Marker {
fn visit_span(&mut self, span: &mut Span) {
*span = span.apply_mark(self.0, self.1)
}

fn visit_mac(&mut self, mac: &mut MacCall) {
mut_visit::noop_visit_mac(mac, self)
}
}

/// An iterator over the token trees in a delimited token tree (`{ ... }`) or a sequence (`$(...)`).
Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_expand/src/mut_visit/tests.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::tests::{matches_codepattern, string_to_crate};

use rustc_ast as ast;
use rustc_ast::mut_visit::{self, MutVisitor};
use rustc_ast::mut_visit::MutVisitor;
use rustc_ast_pretty::pprust;
use rustc_span::symbol::Ident;
use rustc_span::with_default_session_globals;
Expand All @@ -21,9 +21,6 @@ impl MutVisitor for ToZzIdentMutVisitor {
fn visit_ident(&mut self, ident: &mut Ident) {
*ident = Ident::from_str("zz");
}
fn visit_mac(&mut self, mac: &mut ast::MacCall) {
mut_visit::noop_visit_mac(mac, self)
}
}

// Maybe add to `expand.rs`.
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_expand/src/placeholders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,4 @@ impl<'a, 'b> MutVisitor for PlaceholderExpander<'a, 'b> {
|item| !matches!(item.kind, ast::ItemKind::MacCall(_) if !self.cx.ecfg.keep_macs),
);
}

fn visit_mac(&mut self, _mac: &mut ast::MacCall) {
// Do nothing.
}
}
6 changes: 0 additions & 6 deletions compiler/rustc_interface/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -880,12 +880,6 @@ impl<'a> MutVisitor for ReplaceBodyWithLoop<'a, '_> {
})
}
}

// in general the pretty printer processes unexpanded code, so
// we override the default `visit_mac` method which panics.
fn visit_mac(&mut self, mac: &mut ast::MacCall) {
noop_visit_mac(mac, self)
}
}

/// Returns a version string such as "rustc 1.46.0 (04488afe3 2020-08-24)"
Expand Down
10 changes: 2 additions & 8 deletions compiler/rustc_lint/src/early.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,15 +270,9 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
self.check_id(id);
}

fn visit_mac(&mut self, mac: &'a ast::MacCall) {
// FIXME(#54110): So, this setup isn't really right. I think
// that (a) the librustc_ast visitor ought to be doing this as
// part of `walk_mac`, and (b) we should be calling
// `visit_path`, *but* that would require a `NodeId`, and I
// want to get #53686 fixed quickly. -nmatsakis
ast_visit::walk_path(self, &mac.path);

fn visit_mac_call(&mut self, mac: &'a ast::MacCall) {
run_early_pass!(self, check_mac, mac);
ast_visit::walk_mac(self, mac);
}
}

Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_parse/src/parser/pat.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::{Parser, PathStyle};
use crate::{maybe_recover_from_interpolated_ty_qpath, maybe_whole};
use rustc_ast::mut_visit::{noop_visit_mac, noop_visit_pat, MutVisitor};
use rustc_ast::mut_visit::{noop_visit_pat, MutVisitor};
use rustc_ast::ptr::P;
use rustc_ast::token;
use rustc_ast::{self as ast, AttrVec, Attribute, FieldPat, MacCall, Pat, PatKind, RangeEnd};
Expand Down Expand Up @@ -570,10 +570,6 @@ impl<'a> Parser<'a> {
fn make_all_value_bindings_mutable(pat: &mut P<Pat>) -> bool {
struct AddMut(bool);
impl MutVisitor for AddMut {
fn visit_mac(&mut self, mac: &mut MacCall) {
noop_visit_mac(mac, self);
}

fn visit_pat(&mut self, pat: &mut P<Pat>) {
if let PatKind::Ident(BindingMode::ByValue(m @ Mutability::Not), ..) = &mut pat.kind
{
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_passes/src/hir_stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,9 @@ impl<'v> ast_visit::Visitor<'v> for StatCollector<'v> {
ast_visit::walk_lifetime(self, lifetime)
}

fn visit_mac(&mut self, mac: &'v ast::MacCall) {
fn visit_mac_call(&mut self, mac: &'v ast::MacCall) {
self.record("MacCall", Id::None, mac);
ast_visit::walk_mac(self, mac)
}

fn visit_path_segment(&mut self, path_span: Span, path_segment: &'v ast::PathSegment) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Regression test; used to ICE with 'visit_mac disabled by default' due to a
// Regression test; used to ICE with 'visit_mac_call disabled by default' due to a
// `MutVisitor` in `fn make_all_value_bindings_mutable` (`parse/parser/pat.rs`).

macro_rules! mac1 {
Expand Down
8 changes: 1 addition & 7 deletions src/tools/clippy/clippy_lints/src/non_expressive_names.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::utils::{span_lint, span_lint_and_then};
use rustc_ast::ast::{
Arm, AssocItem, AssocItemKind, Attribute, Block, FnDecl, Item, ItemKind, Local, MacCall, Pat, PatKind,
Arm, AssocItem, AssocItemKind, Attribute, Block, FnDecl, Item, ItemKind, Local, Pat, PatKind,
};
use rustc_ast::visit::{walk_block, walk_expr, walk_pat, Visitor};
use rustc_lint::{EarlyContext, EarlyLintPass};
Expand Down Expand Up @@ -150,9 +150,6 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for SimilarNamesNameVisitor<'a, 'tcx, 'b> {
_ => walk_pat(self, pat),
}
}
fn visit_mac(&mut self, _mac: &MacCall) {
// do not check macs
}
}

#[must_use]
Expand Down Expand Up @@ -357,9 +354,6 @@ impl<'a, 'tcx> Visitor<'tcx> for SimilarNamesLocalVisitor<'a, 'tcx> {
fn visit_item(&mut self, _: &Item) {
// do not recurse into inner items
}
fn visit_mac(&mut self, _mac: &MacCall) {
// do not check macs
}
}

impl EarlyLintPass for NonExpressiveNames {
Expand Down

0 comments on commit 8ebca24

Please sign in to comment.