Skip to content

remove visit_clobber and DummyAstNode #141430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

remove visit_clobber and DummyAstNode #141430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion compiler/rustc_ast/src/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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)]
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_ast/src/ast_traits.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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> {
Expand Down
105 changes: 0 additions & 105 deletions compiler/rustc_ast/src/mut_visit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()`.
Expand Down
110 changes: 75 additions & 35 deletions compiler/rustc_expand/src/expand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is good, it's simpler and visit_clobber is not used generically here.

}
}
AstFragment::MethodReceiverExpr(expr) => vis.visit_method_receiver_expr(expr),
$($(AstFragment::$Kind(ast) => vis.$mut_visit_ast(ast),)?)*
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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>>)>,
Expand Down Expand Up @@ -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,
) {
Expand All @@ -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));
}
};
}
Expand Down Expand Up @@ -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>) {
Expand All @@ -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>> {
Expand Down
14 changes: 4 additions & 10 deletions tests/ui-fulldeps/pprust-expr-roundtrip.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also looks better without visit_clobber.


e.kind = ExprKind::Paren(expr);
}
}

Expand Down
4 changes: 2 additions & 2 deletions tests/ui-fulldeps/pprust-parenthesis-insertion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
Loading