Skip to content

Commit 1bc7331

Browse files
fee1-deadfee1-dead
committed
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.
1 parent d76fe15 commit 1bc7331

File tree

6 files changed

+95
-148
lines changed

6 files changed

+95
-148
lines changed

‎compiler/rustc_ast/src/ast.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use rustc_data_structures::tagged_ptr::Tag;
3232
use rustc_macros::{Decodable, Encodable, HashStable_Generic};
3333
pub use rustc_span::AttrId;
3434
use rustc_span::source_map::{Spanned, respan};
35-
use rustc_span::{ErrorGuaranteed, Ident, Span, Symbol, kw, sym};
35+
use rustc_span::{DUMMY_SP, ErrorGuaranteed, Ident, Span, Symbol, kw, sym};
3636
use thin_vec::{ThinVec, thin_vec};
3737

3838
pub use crate::format::*;
@@ -1526,6 +1526,19 @@ impl Expr {
15261526
| ExprKind::Struct(_)
15271527
)
15281528
}
1529+
1530+
/// Creates a dummy `P<Expr>`.
1531+
///
1532+
/// Should only be used when it will be replaced afterwards or as a return value when an error was encountered.
1533+
pub fn dummy() -> P<Expr> {
1534+
P(Expr {
1535+
id: DUMMY_NODE_ID,
1536+
kind: ExprKind::Dummy,
1537+
span: DUMMY_SP,
1538+
attrs: ThinVec::new(),
1539+
tokens: None,
1540+
})
1541+
}
15291542
}
15301543

15311544
#[derive(Clone, Encodable, Decodable, Debug)]

‎compiler/rustc_ast/src/ast_traits.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,7 @@ impl HasAttrs for Stmt {
304304
}
305305

306306
/// A newtype around an AST node that implements the traits above if the node implements them.
307+
#[repr(transparent)]
307308
pub struct AstNodeWrapper<Wrapped, Tag> {
308309
pub wrapped: Wrapped,
309310
pub tag: PhantomData<Tag>,
@@ -313,6 +314,11 @@ impl<Wrapped, Tag> AstNodeWrapper<Wrapped, Tag> {
313314
pub fn new(wrapped: Wrapped, _tag: Tag) -> AstNodeWrapper<Wrapped, Tag> {
314315
AstNodeWrapper { wrapped, tag: Default::default() }
315316
}
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+
}
316322
}
317323

318324
impl<Wrapped: HasNodeId, Tag> HasNodeId for AstNodeWrapper<Wrapped, Tag> {

‎compiler/rustc_ast/src/mut_visit.rs

Lines changed: 0 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -378,16 +378,6 @@ pub trait MutVisitor: Sized {
378378

379379
super::common_visitor_and_walkers!((mut) MutVisitor);
380380

381-
/// Use a map-style function (`FnOnce(T) -> T`) to overwrite a `&mut T`. Useful
382-
/// when using a `flat_map_*` or `filter_map_*` method within a `visit_`
383-
/// method.
384-
//
385-
// No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
386-
pub fn visit_clobber<T: DummyAstNode>(t: &mut T, f: impl FnOnce(T) -> T) {
387-
let old_t = std::mem::replace(t, T::dummy());
388-
*t = f(old_t);
389-
}
390-
391381
// No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
392382
#[inline]
393383
fn visit_vec<T, F>(elems: &mut Vec<T>, mut visit_elem: F)
@@ -1570,101 +1560,6 @@ fn walk_capture_by<T: MutVisitor>(vis: &mut T, capture_by: &mut CaptureBy) {
15701560
}
15711561
}
15721562

1573-
/// Some value for the AST node that is valid but possibly meaningless. Similar
1574-
/// to `Default` but not intended for wide use. The value will never be used
1575-
/// meaningfully, it exists just to support unwinding in `visit_clobber` in the
1576-
/// case where its closure panics.
1577-
pub trait DummyAstNode {
1578-
fn dummy() -> Self;
1579-
}
1580-
1581-
impl<T> DummyAstNode for Option<T> {
1582-
fn dummy() -> Self {
1583-
Default::default()
1584-
}
1585-
}
1586-
1587-
impl<T: DummyAstNode + 'static> DummyAstNode for P<T> {
1588-
fn dummy() -> Self {
1589-
P(DummyAstNode::dummy())
1590-
}
1591-
}
1592-
1593-
impl DummyAstNode for Item {
1594-
fn dummy() -> Self {
1595-
Item {
1596-
attrs: Default::default(),
1597-
id: DUMMY_NODE_ID,
1598-
span: Default::default(),
1599-
vis: Visibility {
1600-
kind: VisibilityKind::Public,
1601-
span: Default::default(),
1602-
tokens: Default::default(),
1603-
},
1604-
kind: ItemKind::ExternCrate(None, Ident::dummy()),
1605-
tokens: Default::default(),
1606-
}
1607-
}
1608-
}
1609-
1610-
impl DummyAstNode for Expr {
1611-
fn dummy() -> Self {
1612-
Expr {
1613-
id: DUMMY_NODE_ID,
1614-
kind: ExprKind::Dummy,
1615-
span: Default::default(),
1616-
attrs: Default::default(),
1617-
tokens: Default::default(),
1618-
}
1619-
}
1620-
}
1621-
1622-
impl DummyAstNode for Ty {
1623-
fn dummy() -> Self {
1624-
Ty {
1625-
id: DUMMY_NODE_ID,
1626-
kind: TyKind::Dummy,
1627-
span: Default::default(),
1628-
tokens: Default::default(),
1629-
}
1630-
}
1631-
}
1632-
1633-
impl DummyAstNode for Pat {
1634-
fn dummy() -> Self {
1635-
Pat {
1636-
id: DUMMY_NODE_ID,
1637-
kind: PatKind::Wild,
1638-
span: Default::default(),
1639-
tokens: Default::default(),
1640-
}
1641-
}
1642-
}
1643-
1644-
impl DummyAstNode for Stmt {
1645-
fn dummy() -> Self {
1646-
Stmt { id: DUMMY_NODE_ID, kind: StmtKind::Empty, span: Default::default() }
1647-
}
1648-
}
1649-
1650-
impl DummyAstNode for Crate {
1651-
fn dummy() -> Self {
1652-
Crate {
1653-
attrs: Default::default(),
1654-
items: Default::default(),
1655-
spans: Default::default(),
1656-
id: DUMMY_NODE_ID,
1657-
is_placeholder: Default::default(),
1658-
}
1659-
}
1660-
}
1661-
1662-
impl<N: DummyAstNode, T: DummyAstNode> DummyAstNode for crate::ast_traits::AstNodeWrapper<N, T> {
1663-
fn dummy() -> Self {
1664-
crate::ast_traits::AstNodeWrapper::new(N::dummy(), T::dummy())
1665-
}
1666-
}
1667-
16681563
#[derive(Debug)]
16691564
pub enum FnKind<'a> {
16701565
/// E.g., `fn foo()`, `fn foo(&self)`, or `extern "Abi" fn foo()`.

‎compiler/rustc_expand/src/expand.rs

Lines changed: 69 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,14 @@ use std::rc::Rc;
33
use std::sync::Arc;
44
use std::{iter, mem};
55

6-
use rustc_ast as ast;
76
use rustc_ast::mut_visit::*;
87
use rustc_ast::ptr::P;
98
use rustc_ast::tokenstream::TokenStream;
109
use rustc_ast::visit::{self, AssocCtxt, Visitor, VisitorResult, try_visit, walk_list};
1110
use rustc_ast::{
12-
AssocItemKind, AstNodeWrapper, AttrArgs, AttrStyle, AttrVec, ExprKind, ForeignItemKind,
13-
HasAttrs, HasNodeId, Inline, ItemKind, MacStmtStyle, MetaItemInner, MetaItemKind, ModKind,
14-
NodeId, PatKind, StmtKind, TyKind, token,
11+
self as ast, AssocItemKind, AstNodeWrapper, AttrArgs, AttrStyle, AttrVec, DUMMY_NODE_ID,
12+
ExprKind, ForeignItemKind, HasAttrs, HasNodeId, Inline, ItemKind, MacStmtStyle, MetaItemInner,
13+
MetaItemKind, ModKind, NodeId, PatKind, StmtKind, TyKind, token,
1514
};
1615
use rustc_ast_pretty::pprust;
1716
use rustc_data_structures::flat_map_in_place::FlatMapInPlace;
@@ -131,13 +130,9 @@ macro_rules! ast_fragments {
131130
pub(crate) fn mut_visit_with<F: MutVisitor>(&mut self, vis: &mut F) {
132131
match self {
133132
AstFragment::OptExpr(opt_expr) => {
134-
visit_clobber(opt_expr, |opt_expr| {
135-
if let Some(expr) = opt_expr {
136-
vis.filter_map_expr(expr)
137-
} else {
138-
None
139-
}
140-
});
133+
if let Some(expr) = opt_expr.take() {
134+
*opt_expr = vis.filter_map_expr(expr)
135+
}
141136
}
142137
AstFragment::MethodReceiverExpr(expr) => vis.visit_method_receiver_expr(expr),
143138
$($(AstFragment::$Kind(ast) => vis.$mut_visit_ast(ast),)?)*
@@ -1782,11 +1777,7 @@ impl InvocationCollectorNode for AstNodeWrapper<P<ast::Expr>, OptExprTag> {
17821777
/// This struct is a hack to workaround unstable of `stmt_expr_attributes`.
17831778
/// It can be removed once that feature is stabilized.
17841779
struct MethodReceiverTag;
1785-
impl DummyAstNode for MethodReceiverTag {
1786-
fn dummy() -> MethodReceiverTag {
1787-
MethodReceiverTag
1788-
}
1789-
}
1780+
17901781
impl InvocationCollectorNode for AstNodeWrapper<P<ast::Expr>, MethodReceiverTag> {
17911782
type OutputTy = Self;
17921783
const KIND: AstFragmentKind = AstFragmentKind::MethodReceiverExpr;
@@ -1852,6 +1843,57 @@ fn build_single_delegations<'a, Node: InvocationCollectorNode>(
18521843
})
18531844
}
18541845

1846+
/// Required for `visit_node` obtained an owned `Node` from `&mut Node`.
1847+
trait DummyAstNode {
1848+
fn dummy() -> Self;
1849+
}
1850+
1851+
impl DummyAstNode for ast::Crate {
1852+
fn dummy() -> Self {
1853+
ast::Crate {
1854+
attrs: Default::default(),
1855+
items: Default::default(),
1856+
spans: Default::default(),
1857+
id: DUMMY_NODE_ID,
1858+
is_placeholder: Default::default(),
1859+
}
1860+
}
1861+
}
1862+
1863+
impl DummyAstNode for P<ast::Ty> {
1864+
fn dummy() -> Self {
1865+
P(ast::Ty {
1866+
id: DUMMY_NODE_ID,
1867+
kind: TyKind::Dummy,
1868+
span: Default::default(),
1869+
tokens: Default::default(),
1870+
})
1871+
}
1872+
}
1873+
1874+
impl DummyAstNode for P<ast::Pat> {
1875+
fn dummy() -> Self {
1876+
P(ast::Pat {
1877+
id: DUMMY_NODE_ID,
1878+
kind: PatKind::Wild,
1879+
span: Default::default(),
1880+
tokens: Default::default(),
1881+
})
1882+
}
1883+
}
1884+
1885+
impl DummyAstNode for P<ast::Expr> {
1886+
fn dummy() -> Self {
1887+
ast::Expr::dummy()
1888+
}
1889+
}
1890+
1891+
impl DummyAstNode for AstNodeWrapper<P<ast::Expr>, MethodReceiverTag> {
1892+
fn dummy() -> Self {
1893+
AstNodeWrapper::new(ast::Expr::dummy(), MethodReceiverTag)
1894+
}
1895+
}
1896+
18551897
struct InvocationCollector<'a, 'b> {
18561898
cx: &'a mut ExtCtxt<'b>,
18571899
invocations: Vec<(Invocation, Option<Arc<SyntaxExtension>>)>,
@@ -2155,18 +2197,19 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
21552197
self.expand_cfg_attr(node, &attr, pos);
21562198
continue;
21572199
}
2158-
_ => visit_clobber(node, |node| {
2159-
self.collect_attr((attr, pos, derives), node.to_annotatable(), Node::KIND)
2200+
_ => {
2201+
let n = mem::replace(node, Node::dummy());
2202+
*node = self
2203+
.collect_attr((attr, pos, derives), n.to_annotatable(), Node::KIND)
21602204
.make_ast::<Node>()
2161-
}),
2205+
}
21622206
},
21632207
None if node.is_mac_call() => {
2164-
visit_clobber(node, |node| {
2165-
// Do not clobber unless it's actually a macro (uncommon case).
2166-
let (mac, attrs, _) = node.take_mac_call();
2167-
self.check_attributes(&attrs, &mac);
2168-
self.collect_bang(mac, Node::KIND).make_ast::<Node>()
2169-
})
2208+
let n = mem::replace(node, Node::dummy());
2209+
let (mac, attrs, _) = n.take_mac_call();
2210+
self.check_attributes(&attrs, &mac);
2211+
2212+
*node = self.collect_bang(mac, Node::KIND).make_ast::<Node>()
21702213
}
21712214
None if node.delegation().is_some() => unreachable!(),
21722215
None => {
@@ -2293,11 +2336,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
22932336
}
22942337

22952338
fn visit_method_receiver_expr(&mut self, node: &mut P<ast::Expr>) {
2296-
visit_clobber(node, |node| {
2297-
let mut wrapper = AstNodeWrapper::new(node, MethodReceiverTag);
2298-
self.visit_node(&mut wrapper);
2299-
wrapper.wrapped
2300-
})
2339+
self.visit_node(AstNodeWrapper::from_mut(node, MethodReceiverTag))
23012340
}
23022341

23032342
fn filter_map_expr(&mut self, node: P<ast::Expr>) -> Option<P<ast::Expr>> {

‎tests/ui-fulldeps/pprust-expr-roundtrip.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ extern crate thin_vec;
3434
extern crate rustc_driver;
3535

3636
use parser::parse_expr;
37-
use rustc_ast::mut_visit::{visit_clobber, MutVisitor};
37+
use rustc_ast::mut_visit::MutVisitor;
3838
use rustc_ast::ptr::P;
3939
use rustc_ast::*;
4040
use rustc_ast_pretty::pprust;
@@ -202,15 +202,9 @@ struct AddParens;
202202
impl MutVisitor for AddParens {
203203
fn visit_expr(&mut self, e: &mut P<Expr>) {
204204
mut_visit::walk_expr(self, e);
205-
visit_clobber(e, |e| {
206-
P(Expr {
207-
id: DUMMY_NODE_ID,
208-
kind: ExprKind::Paren(e),
209-
span: DUMMY_SP,
210-
attrs: AttrVec::new(),
211-
tokens: None,
212-
})
213-
});
205+
let expr = std::mem::replace(e, Expr::dummy());
206+
207+
e.kind = ExprKind::Paren(expr);
214208
}
215209
}
216210

‎tests/ui-fulldeps/pprust-parenthesis-insertion.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ use std::process::ExitCode;
4242

4343
use parser::parse_expr;
4444
use rustc_ast::ast::{Expr, ExprKind};
45-
use rustc_ast::mut_visit::{self, DummyAstNode as _, MutVisitor};
45+
use rustc_ast::mut_visit::{self, MutVisitor};
4646
use rustc_ast::ptr::P;
4747
use rustc_ast_pretty::pprust;
4848
use rustc_session::parse::ParseSess;
@@ -154,7 +154,7 @@ struct Unparenthesize;
154154
impl MutVisitor for Unparenthesize {
155155
fn visit_expr(&mut self, e: &mut P<Expr>) {
156156
while let ExprKind::Paren(paren) = &mut e.kind {
157-
**e = mem::replace(&mut *paren, Expr::dummy());
157+
*e = mem::replace(paren, Expr::dummy());
158158
}
159159
mut_visit::walk_expr(self, e);
160160
}

0 commit comments

Comments
 (0)