From 1bc733187eaff4a3d5e1d1beade163ffea2a6a0f Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Fri, 23 May 2025 17:48:08 +0800 Subject: [PATCH] remove `visit_clobber` and move `DummyAstNode` to `rustc_expand` `visit_clobber` is not really useful except for one niche purpose involving generic code. We should just use the replace logic where we can. --- compiler/rustc_ast/src/ast.rs | 15 ++- compiler/rustc_ast/src/ast_traits.rs | 6 + compiler/rustc_ast/src/mut_visit.rs | 105 ------------------ compiler/rustc_expand/src/expand.rs | 99 ++++++++++++----- tests/ui-fulldeps/pprust-expr-roundtrip.rs | 14 +-- .../pprust-parenthesis-insertion.rs | 4 +- 6 files changed, 95 insertions(+), 148 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index a16219361c051..39805649ab229 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -32,7 +32,7 @@ use rustc_data_structures::tagged_ptr::Tag; use rustc_macros::{Decodable, Encodable, HashStable_Generic}; pub use rustc_span::AttrId; use rustc_span::source_map::{Spanned, respan}; -use rustc_span::{ErrorGuaranteed, Ident, Span, Symbol, kw, sym}; +use rustc_span::{DUMMY_SP, ErrorGuaranteed, Ident, Span, Symbol, kw, sym}; use thin_vec::{ThinVec, thin_vec}; pub use crate::format::*; @@ -1526,6 +1526,19 @@ impl Expr { | ExprKind::Struct(_) ) } + + /// Creates a dummy `P`. + /// + /// Should only be used when it will be replaced afterwards or as a return value when an error was encountered. + pub fn dummy() -> P { + P(Expr { + id: DUMMY_NODE_ID, + kind: ExprKind::Dummy, + span: DUMMY_SP, + attrs: ThinVec::new(), + tokens: None, + }) + } } #[derive(Clone, Encodable, Decodable, Debug)] diff --git a/compiler/rustc_ast/src/ast_traits.rs b/compiler/rustc_ast/src/ast_traits.rs index 21de7ff7719b6..950808bd67a45 100644 --- a/compiler/rustc_ast/src/ast_traits.rs +++ b/compiler/rustc_ast/src/ast_traits.rs @@ -304,6 +304,7 @@ impl HasAttrs for Stmt { } /// A newtype around an AST node that implements the traits above if the node implements them. +#[repr(transparent)] pub struct AstNodeWrapper { pub wrapped: Wrapped, pub tag: PhantomData, @@ -313,6 +314,11 @@ impl AstNodeWrapper { pub fn new(wrapped: Wrapped, _tag: Tag) -> AstNodeWrapper { AstNodeWrapper { wrapped, tag: Default::default() } } + + pub fn from_mut(wrapped: &mut Wrapped, _tag: Tag) -> &mut AstNodeWrapper { + // SAFETY: `AstNodeWrapper` is `repr(transparent)` w.r.t `Wrapped` + unsafe { <*mut Wrapped>::from(wrapped).cast::().as_mut().unwrap() } + } } impl HasNodeId for AstNodeWrapper { diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index 6770fd5a4aaeb..b34e500ed0ade 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -378,16 +378,6 @@ pub trait MutVisitor: Sized { super::common_visitor_and_walkers!((mut) MutVisitor); -/// Use a map-style function (`FnOnce(T) -> T`) to overwrite a `&mut T`. Useful -/// when using a `flat_map_*` or `filter_map_*` method within a `visit_` -/// method. -// -// No `noop_` prefix because there isn't a corresponding method in `MutVisitor`. -pub fn visit_clobber(t: &mut T, f: impl FnOnce(T) -> T) { - let old_t = std::mem::replace(t, T::dummy()); - *t = f(old_t); -} - // No `noop_` prefix because there isn't a corresponding method in `MutVisitor`. #[inline] fn visit_vec(elems: &mut Vec, mut visit_elem: F) @@ -1570,101 +1560,6 @@ fn walk_capture_by(vis: &mut T, capture_by: &mut CaptureBy) { } } -/// Some value for the AST node that is valid but possibly meaningless. Similar -/// to `Default` but not intended for wide use. The value will never be used -/// meaningfully, it exists just to support unwinding in `visit_clobber` in the -/// case where its closure panics. -pub trait DummyAstNode { - fn dummy() -> Self; -} - -impl DummyAstNode for Option { - fn dummy() -> Self { - Default::default() - } -} - -impl DummyAstNode for P { - fn dummy() -> Self { - P(DummyAstNode::dummy()) - } -} - -impl DummyAstNode for Item { - fn dummy() -> Self { - Item { - attrs: Default::default(), - id: DUMMY_NODE_ID, - span: Default::default(), - vis: Visibility { - kind: VisibilityKind::Public, - span: Default::default(), - tokens: Default::default(), - }, - kind: ItemKind::ExternCrate(None, Ident::dummy()), - tokens: Default::default(), - } - } -} - -impl DummyAstNode for Expr { - fn dummy() -> Self { - Expr { - id: DUMMY_NODE_ID, - kind: ExprKind::Dummy, - span: Default::default(), - attrs: Default::default(), - tokens: Default::default(), - } - } -} - -impl DummyAstNode for Ty { - fn dummy() -> Self { - Ty { - id: DUMMY_NODE_ID, - kind: TyKind::Dummy, - span: Default::default(), - tokens: Default::default(), - } - } -} - -impl DummyAstNode for Pat { - fn dummy() -> Self { - Pat { - id: DUMMY_NODE_ID, - kind: PatKind::Wild, - span: Default::default(), - tokens: Default::default(), - } - } -} - -impl DummyAstNode for Stmt { - fn dummy() -> Self { - Stmt { id: DUMMY_NODE_ID, kind: StmtKind::Empty, span: Default::default() } - } -} - -impl DummyAstNode for Crate { - fn dummy() -> Self { - Crate { - attrs: Default::default(), - items: Default::default(), - spans: Default::default(), - id: DUMMY_NODE_ID, - is_placeholder: Default::default(), - } - } -} - -impl DummyAstNode for crate::ast_traits::AstNodeWrapper { - fn dummy() -> Self { - crate::ast_traits::AstNodeWrapper::new(N::dummy(), T::dummy()) - } -} - #[derive(Debug)] pub enum FnKind<'a> { /// E.g., `fn foo()`, `fn foo(&self)`, or `extern "Abi" fn foo()`. diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 81d4d59ee0451..97b0724f8a6cd 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -3,15 +3,14 @@ use std::rc::Rc; use std::sync::Arc; use std::{iter, mem}; -use rustc_ast as ast; use rustc_ast::mut_visit::*; use rustc_ast::ptr::P; use rustc_ast::tokenstream::TokenStream; use rustc_ast::visit::{self, AssocCtxt, Visitor, VisitorResult, try_visit, walk_list}; use rustc_ast::{ - AssocItemKind, AstNodeWrapper, AttrArgs, AttrStyle, AttrVec, ExprKind, ForeignItemKind, - HasAttrs, HasNodeId, Inline, ItemKind, MacStmtStyle, MetaItemInner, MetaItemKind, ModKind, - NodeId, PatKind, StmtKind, TyKind, token, + self as ast, AssocItemKind, AstNodeWrapper, AttrArgs, AttrStyle, AttrVec, DUMMY_NODE_ID, + ExprKind, ForeignItemKind, HasAttrs, HasNodeId, Inline, ItemKind, MacStmtStyle, MetaItemInner, + MetaItemKind, ModKind, NodeId, PatKind, StmtKind, TyKind, token, }; use rustc_ast_pretty::pprust; use rustc_data_structures::flat_map_in_place::FlatMapInPlace; @@ -131,13 +130,9 @@ macro_rules! ast_fragments { pub(crate) fn mut_visit_with(&mut self, vis: &mut F) { match self { AstFragment::OptExpr(opt_expr) => { - visit_clobber(opt_expr, |opt_expr| { - if let Some(expr) = opt_expr { - vis.filter_map_expr(expr) - } else { - None - } - }); + if let Some(expr) = opt_expr.take() { + *opt_expr = vis.filter_map_expr(expr) + } } AstFragment::MethodReceiverExpr(expr) => vis.visit_method_receiver_expr(expr), $($(AstFragment::$Kind(ast) => vis.$mut_visit_ast(ast),)?)* @@ -1782,11 +1777,7 @@ impl InvocationCollectorNode for AstNodeWrapper, OptExprTag> { /// This struct is a hack to workaround unstable of `stmt_expr_attributes`. /// It can be removed once that feature is stabilized. struct MethodReceiverTag; -impl DummyAstNode for MethodReceiverTag { - fn dummy() -> MethodReceiverTag { - MethodReceiverTag - } -} + impl InvocationCollectorNode for AstNodeWrapper, MethodReceiverTag> { type OutputTy = Self; const KIND: AstFragmentKind = AstFragmentKind::MethodReceiverExpr; @@ -1852,6 +1843,57 @@ fn build_single_delegations<'a, Node: InvocationCollectorNode>( }) } +/// Required for `visit_node` obtained an owned `Node` from `&mut Node`. +trait DummyAstNode { + fn dummy() -> Self; +} + +impl DummyAstNode for ast::Crate { + fn dummy() -> Self { + ast::Crate { + attrs: Default::default(), + items: Default::default(), + spans: Default::default(), + id: DUMMY_NODE_ID, + is_placeholder: Default::default(), + } + } +} + +impl DummyAstNode for P { + fn dummy() -> Self { + P(ast::Ty { + id: DUMMY_NODE_ID, + kind: TyKind::Dummy, + span: Default::default(), + tokens: Default::default(), + }) + } +} + +impl DummyAstNode for P { + fn dummy() -> Self { + P(ast::Pat { + id: DUMMY_NODE_ID, + kind: PatKind::Wild, + span: Default::default(), + tokens: Default::default(), + }) + } +} + +impl DummyAstNode for P { + fn dummy() -> Self { + ast::Expr::dummy() + } +} + +impl DummyAstNode for AstNodeWrapper, MethodReceiverTag> { + fn dummy() -> Self { + AstNodeWrapper::new(ast::Expr::dummy(), MethodReceiverTag) + } +} + struct InvocationCollector<'a, 'b> { cx: &'a mut ExtCtxt<'b>, invocations: Vec<(Invocation, Option>)>, @@ -2155,18 +2197,19 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { self.expand_cfg_attr(node, &attr, pos); continue; } - _ => visit_clobber(node, |node| { - self.collect_attr((attr, pos, derives), node.to_annotatable(), Node::KIND) + _ => { + let n = mem::replace(node, Node::dummy()); + *node = self + .collect_attr((attr, pos, derives), n.to_annotatable(), Node::KIND) .make_ast::() - }), + } }, None if node.is_mac_call() => { - visit_clobber(node, |node| { - // Do not clobber unless it's actually a macro (uncommon case). - let (mac, attrs, _) = node.take_mac_call(); - self.check_attributes(&attrs, &mac); - self.collect_bang(mac, Node::KIND).make_ast::() - }) + let n = mem::replace(node, Node::dummy()); + let (mac, attrs, _) = n.take_mac_call(); + self.check_attributes(&attrs, &mac); + + *node = self.collect_bang(mac, Node::KIND).make_ast::() } None if node.delegation().is_some() => unreachable!(), None => { @@ -2293,11 +2336,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { } fn visit_method_receiver_expr(&mut self, node: &mut P) { - visit_clobber(node, |node| { - let mut wrapper = AstNodeWrapper::new(node, MethodReceiverTag); - self.visit_node(&mut wrapper); - wrapper.wrapped - }) + self.visit_node(AstNodeWrapper::from_mut(node, MethodReceiverTag)) } fn filter_map_expr(&mut self, node: P) -> Option> { diff --git a/tests/ui-fulldeps/pprust-expr-roundtrip.rs b/tests/ui-fulldeps/pprust-expr-roundtrip.rs index 4a866560e798a..f5cfa9e0bccff 100644 --- a/tests/ui-fulldeps/pprust-expr-roundtrip.rs +++ b/tests/ui-fulldeps/pprust-expr-roundtrip.rs @@ -34,7 +34,7 @@ extern crate thin_vec; extern crate rustc_driver; use parser::parse_expr; -use rustc_ast::mut_visit::{visit_clobber, MutVisitor}; +use rustc_ast::mut_visit::MutVisitor; use rustc_ast::ptr::P; use rustc_ast::*; use rustc_ast_pretty::pprust; @@ -202,15 +202,9 @@ struct AddParens; impl MutVisitor for AddParens { fn visit_expr(&mut self, e: &mut P) { mut_visit::walk_expr(self, e); - visit_clobber(e, |e| { - P(Expr { - id: DUMMY_NODE_ID, - kind: ExprKind::Paren(e), - span: DUMMY_SP, - attrs: AttrVec::new(), - tokens: None, - }) - }); + let expr = std::mem::replace(e, Expr::dummy()); + + e.kind = ExprKind::Paren(expr); } } diff --git a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs index 2b41020d3071c..c566ac459e0d2 100644 --- a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs +++ b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs @@ -42,7 +42,7 @@ use std::process::ExitCode; use parser::parse_expr; use rustc_ast::ast::{Expr, ExprKind}; -use rustc_ast::mut_visit::{self, DummyAstNode as _, MutVisitor}; +use rustc_ast::mut_visit::{self, MutVisitor}; use rustc_ast::ptr::P; use rustc_ast_pretty::pprust; use rustc_session::parse::ParseSess; @@ -154,7 +154,7 @@ struct Unparenthesize; impl MutVisitor for Unparenthesize { fn visit_expr(&mut self, e: &mut P) { while let ExprKind::Paren(paren) = &mut e.kind { - **e = mem::replace(&mut *paren, Expr::dummy()); + *e = mem::replace(paren, Expr::dummy()); } mut_visit::walk_expr(self, e); }