Auto merge of #141430 - fee1-dead-contrib:push-nmzoprvtsvww, r=<try>

bors · bors · commit 25ef124a4c5f · 2025-05-24T07:24:15.000Z
remove `visit_clobber` and `DummyAstNode`

used for one specific niche purpose, turns out we can just remove them and inline dummy values into callsites. this cleans them up.
diff --git 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<Expr>`.
+    ///
+    /// Should only be used when it will be replaced afterwards or as a return value when an error was encountered.
+    pub fn dummy() -> P<Expr> {
+        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
@@ -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<Wrapped, Tag> {
     pub wrapped: Wrapped,
     pub tag: PhantomData<Tag>,
@@ -313,6 +314,11 @@ impl<Wrapped, Tag> AstNodeWrapper<Wrapped, Tag> {
     pub fn new(wrapped: Wrapped, _tag: Tag) -> AstNodeWrapper<Wrapped, Tag> {
         AstNodeWrapper { wrapped, tag: Default::default() }
     }
+
+    pub fn from_mut(wrapped: &mut Wrapped, _tag: Tag) -> &mut AstNodeWrapper<Wrapped, Tag> {
+        // SAFETY: `AstNodeWrapper` is `repr(transparent)` w.r.t `Wrapped`
+        unsafe { <*mut Wrapped>::from(wrapped).cast::<Self>().as_mut().unwrap() }
+    }
 }
 
 impl<Wrapped: HasNodeId, Tag> HasNodeId for AstNodeWrapper<Wrapped, Tag> {
diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs
@@ -390,16 +390,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: DummyAstNode>(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<T, F>(elems: &mut Vec<T>, mut visit_elem: F)
@@ -1798,101 +1788,6 @@ fn walk_define_opaques<T: MutVisitor>(
     }
 }
 
-/// 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<T> DummyAstNode for Option<T> {
-    fn dummy() -> Self {
-        Default::default()
-    }
-}
-
-impl<T: DummyAstNode + 'static> DummyAstNode for P<T> {
-    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<N: DummyAstNode, T: DummyAstNode> DummyAstNode for crate::ast_traits::AstNodeWrapper<N, T> {
-    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
@@ -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<F: MutVisitor>(&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<P<ast::Expr>, 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<P<ast::Expr>, 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 DummyValue {
+    fn dummy() -> Self;
+}
+
+impl DummyValue 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 DummyValue for P<ast::Ty> {
+    fn dummy() -> Self {
+        P(ast::Ty {
+            id: DUMMY_NODE_ID,
+            kind: TyKind::Dummy,
+            span: Default::default(),
+            tokens: Default::default(),
+        })
+    }
+}
+
+impl DummyValue for P<ast::Pat> {
+    fn dummy() -> Self {
+        P(ast::Pat {
+            id: DUMMY_NODE_ID,
+            kind: PatKind::Wild,
+            span: Default::default(),
+            tokens: Default::default(),
+        })
+    }
+}
+
+impl DummyValue for P<ast::Expr> {
+    fn dummy() -> Self {
+        ast::Expr::dummy()
+    }
+}
+
+impl DummyValue for AstNodeWrapper<P<ast::Expr>, MethodReceiverTag> {
+    fn dummy() -> Self {
+        AstNodeWrapper::new(ast::Expr::dummy(), MethodReceiverTag)
+    }
+}
+
 struct InvocationCollector<'a, 'b> {
     cx: &'a mut ExtCtxt<'b>,
     invocations: Vec<(Invocation, Option<Arc<SyntaxExtension>>)>,
@@ -2135,7 +2177,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
         }
     }
 
-    fn visit_node<Node: InvocationCollectorNode<OutputTy = Node> + DummyAstNode>(
+    fn visit_node<Node: InvocationCollectorNode<OutputTy = Node> + DummyValue>(
         &mut self,
         node: &mut Node,
     ) {
@@ -2155,22 +2197,23 @@ 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::<Node>()
-                    }),
+                    }
                 },
                 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::<Node>()
-                    })
+                    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::<Node>()
                 }
                 None if node.delegation().is_some() => unreachable!(),
                 None => {
-                    assign_id!(self, node.node_id_mut(), || node.walk(self))
+                    assign_id!(self, node.node_id_mut(), || node.walk(self));
                 }
             };
         }
@@ -2273,11 +2316,11 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
     }
 
     fn visit_crate(&mut self, node: &mut ast::Crate) {
-        self.visit_node(node)
+        self.visit_node(node);
     }
 
     fn visit_ty(&mut self, node: &mut P<ast::Ty>) {
-        self.visit_node(node)
+        self.visit_node(node);
     }
 
     fn visit_pat(&mut self, node: &mut P<ast::Pat>) {
@@ -2289,15 +2332,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
         if let Some(attr) = node.attrs.first() {
             self.cfg().maybe_emit_expr_attr_err(attr);
         }
-        self.visit_node(node)
+        self.visit_node(node);
     }
 
     fn visit_method_receiver_expr(&mut self, node: &mut P<ast::Expr>) {
-        visit_clobber(node, |node| {
-            let mut wrapper = AstNodeWrapper::new(node, MethodReceiverTag);
-            self.visit_node(&mut wrapper);
-            wrapper.wrapped
-        })
+        let wrapper = AstNodeWrapper::from_mut(node, MethodReceiverTag);
+        self.visit_node(wrapper);
     }
 
     fn filter_map_expr(&mut self, node: P<ast::Expr>) -> Option<P<ast::Expr>> {
diff --git 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<Expr>) {
         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
@@ -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<Expr>) {
         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);
     }

Original file line number	Diff line number	Diff line change
`@@ -304,6 +304,7 @@ impl HasAttrs for Stmt {`
`304`	`304`	`}`
`305`	`305`
`306`	`306`	`/// A newtype around an AST node that implements the traits above if the node implements them.`
	`307`	`+#[repr(transparent)]`
`307`	`308`	`pub struct AstNodeWrapper<Wrapped, Tag> {`
`308`	`309`	`pub wrapped: Wrapped,`
`309`	`310`	`pub tag: PhantomData<Tag>,`
`@@ -313,6 +314,11 @@ impl<Wrapped, Tag> AstNodeWrapper<Wrapped, Tag> {`
`313`	`314`	`pub fn new(wrapped: Wrapped, _tag: Tag) -> AstNodeWrapper<Wrapped, Tag> {`
`314`	`315`	`AstNodeWrapper { wrapped, tag: Default::default() }`
`315`	`316`	`}`
	`317`	`+`
	`318`	`+ pub fn from_mut(wrapped: &mut Wrapped, _tag: Tag) -> &mut AstNodeWrapper<Wrapped, Tag> {`
	`319`	+ // SAFETY: `AstNodeWrapper` is `repr(transparent)` w.r.t `Wrapped`
	`320`	`+ unsafe { <*mut Wrapped>::from(wrapped).cast::<Self>().as_mut().unwrap() }`
	`321`	`+ }`
`316`	`322`	`}`
`317`	`323`
`318`	`324`	`impl<Wrapped: HasNodeId, Tag> HasNodeId for AstNodeWrapper<Wrapped, Tag> {`