Skip to content

avoid &mut P<T> in visit_expr etc methods #142371

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.

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

Merged
merged 1 commit into from
Jun 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 22 additions & 4 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,12 @@ impl Pat {
}
}

impl From<P<Pat>> for Pat {
fn from(value: P<Pat>) -> Self {
*value
}
}

/// A single field in a struct pattern.
///
/// Patterns like the fields of `Foo { x, ref y, ref mut z }`
Expand Down Expand Up @@ -1522,17 +1528,23 @@ impl Expr {
)
}

/// Creates a dummy `P<Expr>`.
/// Creates a dummy `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 {
pub fn dummy() -> Expr {
Expr {
id: DUMMY_NODE_ID,
kind: ExprKind::Dummy,
span: DUMMY_SP,
attrs: ThinVec::new(),
tokens: None,
})
}
}
}

impl From<P<Expr>> for Expr {
fn from(value: P<Expr>) -> Self {
*value
}
}

Expand Down Expand Up @@ -2343,6 +2355,12 @@ impl Clone for Ty {
}
}

impl From<P<Ty>> for Ty {
fn from(value: P<Ty>) -> Self {
*value
}
}

impl Ty {
pub fn peel_refs(&self) -> &Self {
let mut final_ty = self;
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,15 @@ pub trait MutVisitor: Sized + MutVisitorResult<Result = ()> {
walk_flat_map_arm(self, arm)
}

fn visit_pat(&mut self, p: &mut P<Pat>) {
fn visit_pat(&mut self, p: &mut Pat) {
walk_pat(self, p);
}

fn visit_anon_const(&mut self, c: &mut AnonConst) {
walk_anon_const(self, c);
}

fn visit_expr(&mut self, e: &mut P<Expr>) {
fn visit_expr(&mut self, e: &mut Expr) {
walk_expr(self, e);
}

Expand All @@ -194,7 +194,7 @@ pub trait MutVisitor: Sized + MutVisitorResult<Result = ()> {
walk_generic_arg(self, arg);
}

fn visit_ty(&mut self, t: &mut P<Ty>) {
fn visit_ty(&mut self, t: &mut Ty) {
walk_ty(self, t);
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/cfg_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl CfgEval<'_> {

impl MutVisitor for CfgEval<'_> {
#[instrument(level = "trace", skip(self))]
fn visit_expr(&mut self, expr: &mut P<ast::Expr>) {
fn visit_expr(&mut self, expr: &mut ast::Expr) {
self.0.configure_expr(expr, false);
mut_visit::walk_expr(self, expr);
}
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_builtin_macros/src/deriving/coerce_pointee.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use ast::HasAttrs;
use ast::ptr::P;
use rustc_ast::mut_visit::MutVisitor;
use rustc_ast::visit::BoundKind;
use rustc_ast::{
Expand Down Expand Up @@ -378,11 +377,11 @@ struct TypeSubstitution<'a> {
}

impl<'a> ast::mut_visit::MutVisitor for TypeSubstitution<'a> {
fn visit_ty(&mut self, ty: &mut P<ast::Ty>) {
fn visit_ty(&mut self, ty: &mut ast::Ty) {
if let Some(name) = ty.kind.is_simple_path()
&& name == self.from_name
{
**ty = self.to_ty.clone();
*ty = self.to_ty.clone();
self.rewritten = true;
} else {
ast::mut_visit::walk_ty(self, ty);
Expand Down
35 changes: 18 additions & 17 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1668,7 +1668,7 @@ impl InvocationCollectorNode for ast::Crate {
}
}

impl InvocationCollectorNode for P<ast::Ty> {
impl InvocationCollectorNode for ast::Ty {
type OutputTy = P<ast::Ty>;
const KIND: AstFragmentKind = AstFragmentKind::Ty;
fn to_annotatable(self) -> Annotatable {
Expand All @@ -1691,7 +1691,7 @@ impl InvocationCollectorNode for P<ast::Ty> {
}
}

impl InvocationCollectorNode for P<ast::Pat> {
impl InvocationCollectorNode for ast::Pat {
type OutputTy = P<ast::Pat>;
const KIND: AstFragmentKind = AstFragmentKind::Pat;
fn to_annotatable(self) -> Annotatable {
Expand All @@ -1714,11 +1714,11 @@ impl InvocationCollectorNode for P<ast::Pat> {
}
}

impl InvocationCollectorNode for P<ast::Expr> {
impl InvocationCollectorNode for ast::Expr {
type OutputTy = P<ast::Expr>;
const KIND: AstFragmentKind = AstFragmentKind::Expr;
fn to_annotatable(self) -> Annotatable {
Annotatable::Expr(self)
Annotatable::Expr(P(self))
}
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_expr()
Expand Down Expand Up @@ -1855,37 +1855,37 @@ impl DummyAstNode for ast::Crate {
}
}

impl DummyAstNode for P<ast::Ty> {
impl DummyAstNode for ast::Ty {
fn dummy() -> Self {
P(ast::Ty {
ast::Ty {
id: DUMMY_NODE_ID,
kind: TyKind::Dummy,
span: Default::default(),
tokens: Default::default(),
})
}
}
}

impl DummyAstNode for P<ast::Pat> {
impl DummyAstNode for ast::Pat {
fn dummy() -> Self {
P(ast::Pat {
ast::Pat {
id: DUMMY_NODE_ID,
kind: PatKind::Wild,
span: Default::default(),
tokens: Default::default(),
})
}
}
}

impl DummyAstNode for P<ast::Expr> {
impl DummyAstNode for ast::Expr {
fn dummy() -> Self {
ast::Expr::dummy()
}
}

impl DummyAstNode for AstNodeWrapper<P<ast::Expr>, MethodReceiverTag> {
fn dummy() -> Self {
AstNodeWrapper::new(ast::Expr::dummy(), MethodReceiverTag)
AstNodeWrapper::new(P(ast::Expr::dummy()), MethodReceiverTag)
}
}

Expand Down Expand Up @@ -2172,7 +2172,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
}
}

fn visit_node<Node: InvocationCollectorNode<OutputTy = Node> + DummyAstNode>(
fn visit_node<Node: InvocationCollectorNode<OutputTy: Into<Node>> + DummyAstNode>(
&mut self,
node: &mut Node,
) {
Expand All @@ -2197,14 +2197,15 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
*node = self
.collect_attr((attr, pos, derives), n.to_annotatable(), Node::KIND)
.make_ast::<Node>()
.into()
}
},
None if node.is_mac_call() => {
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>()
*node = self.collect_bang(mac, Node::KIND).make_ast::<Node>().into()
}
None if node.delegation().is_some() => unreachable!(),
None => {
Expand Down Expand Up @@ -2314,15 +2315,15 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
self.visit_node(node)
}

fn visit_ty(&mut self, node: &mut P<ast::Ty>) {
fn visit_ty(&mut self, node: &mut ast::Ty) {
self.visit_node(node)
}

fn visit_pat(&mut self, node: &mut P<ast::Pat>) {
fn visit_pat(&mut self, node: &mut ast::Pat) {
self.visit_node(node)
}

fn visit_expr(&mut self, node: &mut P<ast::Expr>) {
fn visit_expr(&mut self, node: &mut ast::Expr) {
// FIXME: Feature gating is performed inconsistently between `Expr` and `OptExpr`.
if let Some(attr) = node.attrs.first() {
self.cfg().maybe_emit_expr_attr_err(attr);
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_expand/src/placeholders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,9 +332,9 @@ impl MutVisitor for PlaceholderExpander {
}
}

fn visit_expr(&mut self, expr: &mut P<ast::Expr>) {
fn visit_expr(&mut self, expr: &mut ast::Expr) {
match expr.kind {
ast::ExprKind::MacCall(_) => *expr = self.remove(expr.id).make_expr(),
ast::ExprKind::MacCall(_) => *expr = *self.remove(expr.id).make_expr(),
_ => walk_expr(self, expr),
}
}
Expand Down Expand Up @@ -399,16 +399,16 @@ impl MutVisitor for PlaceholderExpander {
stmts
}

fn visit_pat(&mut self, pat: &mut P<ast::Pat>) {
fn visit_pat(&mut self, pat: &mut ast::Pat) {
match pat.kind {
ast::PatKind::MacCall(_) => *pat = self.remove(pat.id).make_pat(),
ast::PatKind::MacCall(_) => *pat = *self.remove(pat.id).make_pat(),
_ => walk_pat(self, pat),
}
}

fn visit_ty(&mut self, ty: &mut P<ast::Ty>) {
fn visit_ty(&mut self, ty: &mut ast::Ty) {
match ty.kind {
ast::TyKind::MacCall(_) => *ty = self.remove(ty.id).make_ty(),
ast::TyKind::MacCall(_) => *ty = *self.remove(ty.id).make_ty(),
_ => walk_ty(self, ty),
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4087,7 +4087,7 @@ impl<'a> CondChecker<'a> {
}

impl MutVisitor for CondChecker<'_> {
fn visit_expr(&mut self, e: &mut P<Expr>) {
fn visit_expr(&mut self, e: &mut Expr) {
self.depth += 1;
use ForbiddenLetReason::*;

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1094,7 +1094,7 @@ impl<'a> Parser<'a> {
fn make_all_value_bindings_mutable(pat: &mut P<Pat>) -> bool {
struct AddMut(bool);
impl MutVisitor for AddMut {
fn visit_pat(&mut self, pat: &mut P<Pat>) {
fn visit_pat(&mut self, pat: &mut Pat) {
if let PatKind::Ident(BindingMode(ByRef::No, m @ Mutability::Not), ..) =
&mut pat.kind
{
Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/clippy_lints/src/unnested_or_patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn remove_all_parens(pat: &mut P<Pat>) {
}

impl MutVisitor for Visitor {
fn visit_pat(&mut self, pat: &mut P<Pat>) {
fn visit_pat(&mut self, pat: &mut Pat) {
let is_inner = mem::replace(&mut self.is_inner, true);
walk_pat(self, pat);
let inner = match &mut pat.kind {
Expand All @@ -145,7 +145,7 @@ fn remove_all_parens(pat: &mut P<Pat>) {
fn insert_necessary_parens(pat: &mut P<Pat>) {
struct Visitor;
impl MutVisitor for Visitor {
fn visit_pat(&mut self, pat: &mut P<Pat>) {
fn visit_pat(&mut self, pat: &mut Pat) {
use ast::BindingMode;
walk_pat(self, pat);
let target = match &mut pat.kind {
Expand All @@ -167,7 +167,7 @@ fn unnest_or_patterns(pat: &mut P<Pat>) -> bool {
changed: bool,
}
impl MutVisitor for Visitor {
fn visit_pat(&mut self, p: &mut P<Pat>) {
fn visit_pat(&mut self, p: &mut Pat) {
// This is a bottom up transformation, so recurse first.
walk_pat(self, p);

Expand Down
8 changes: 4 additions & 4 deletions tests/ui-fulldeps/pprust-expr-roundtrip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,9 @@ fn iter_exprs(depth: usize, f: &mut dyn FnMut(P<Expr>)) {
struct RemoveParens;

impl MutVisitor for RemoveParens {
fn visit_expr(&mut self, e: &mut P<Expr>) {
fn visit_expr(&mut self, e: &mut Expr) {
match e.kind.clone() {
ExprKind::Paren(inner) => *e = inner,
ExprKind::Paren(inner) => *e = *inner,
_ => {}
};
mut_visit::walk_expr(self, e);
Expand All @@ -200,11 +200,11 @@ impl MutVisitor for RemoveParens {
struct AddParens;

impl MutVisitor for AddParens {
fn visit_expr(&mut self, e: &mut P<Expr>) {
fn visit_expr(&mut self, e: &mut Expr) {
mut_visit::walk_expr(self, e);
let expr = std::mem::replace(e, Expr::dummy());

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

Expand Down
3 changes: 1 addition & 2 deletions tests/ui-fulldeps/pprust-parenthesis-insertion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ use std::process::ExitCode;
use parser::parse_expr;
use rustc_ast::ast::{Expr, ExprKind};
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 @@ -152,7 +151,7 @@ static EXPRS: &[&str] = &[
struct Unparenthesize;

impl MutVisitor for Unparenthesize {
fn visit_expr(&mut self, e: &mut P<Expr>) {
fn visit_expr(&mut self, e: &mut Expr) {
while let ExprKind::Paren(paren) = &mut e.kind {
*e = mem::replace(paren, Expr::dummy());
}
Expand Down
Loading