Skip to content

refactor(ast): replace AstBuilder::move_* methods with take #9970

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

Closed
overlookmotel wants to merge 1 commit into from
Closed

refactor(ast): replace AstBuilder::move_* methods with take #9970

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
122 changes: 5 additions & 117 deletions crates/oxc_ast/src/ast_builder_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#![warn(missing_docs)]

use std::{borrow::Cow, mem};
use std::borrow::Cow;

use oxc_allocator::{Allocator, Box, FromIn, IntoIn, String, Vec};
use oxc_allocator::{Allocator, Box, FromIn, IntoIn, String, TakeIn, Vec};
use oxc_span::{Atom, SPAN, Span};
use oxc_syntax::{number::NumberBase, operator::UnaryOperator, scope::ScopeId};

Expand Down Expand Up @@ -101,121 +101,9 @@ impl<'a> AstBuilder<'a> {
}
}

/// Moves the expression out by replacing it with an [`Expression::NullLiteral`].
#[inline]
pub fn move_expression(self, expr: &mut Expression<'a>) -> Expression<'a> {
let null_expr = self.expression_null_literal(SPAN);
mem::replace(expr, null_expr)
}

/// Moves the statement out by replacing it with a [`Statement::EmptyStatement`].
#[inline]
pub fn move_statement(self, stmt: &mut Statement<'a>) -> Statement<'a> {
let empty_stmt = self.empty_statement(SPAN);
mem::replace(stmt, Statement::EmptyStatement(self.alloc(empty_stmt)))
}

/// Moves the assignment target out by replacing it with a dummy
/// [`AssignmentTarget::AssignmentTargetIdentifier`] with no name and an empty [`Span`].
#[inline]
pub fn move_assignment_target(self, target: &mut AssignmentTarget<'a>) -> AssignmentTarget<'a> {
let dummy =
self.simple_assignment_target_assignment_target_identifier(SPAN, Atom::from(""));
mem::replace(target, dummy.into())
}

/// Moves the property key out by replacing it with a [`PropertyKey::NullLiteral`].
pub fn move_property_key(self, key: &mut PropertyKey<'a>) -> PropertyKey<'a> {
let null_expr = PropertyKey::from(self.expression_null_literal(SPAN));
mem::replace(key, null_expr)
}

/// Move a declaration out by replacing it with an empty [`Declaration::VariableDeclaration`].
#[inline]
pub fn move_declaration(self, decl: &mut Declaration<'a>) -> Declaration<'a> {
let empty_decl =
self.declaration_variable(SPAN, VariableDeclarationKind::Var, self.vec(), false);
mem::replace(decl, empty_decl)
}

/// Move a variable declaration out by replacing it with an empty [`VariableDeclaration`].
#[inline]
pub fn move_variable_declaration(
self,
decl: &mut VariableDeclaration<'a>,
) -> VariableDeclaration<'a> {
let empty_decl =
self.variable_declaration(SPAN, VariableDeclarationKind::Var, self.vec(), false);
mem::replace(decl, empty_decl)
}

/// Move a formal parameters out by replacing it with an empty [`FormalParameters`].
#[inline]
pub fn move_formal_parameters(self, params: &mut FormalParameters<'a>) -> FormalParameters<'a> {
let empty_params = self.formal_parameters(SPAN, params.kind, self.vec(), NONE);
mem::replace(params, empty_params)
}

/// Move a function body out by replacing it with an empty [`FunctionBody`].
#[inline]
pub fn move_function_body(self, body: &mut FunctionBody<'a>) -> FunctionBody<'a> {
let empty_body = self.function_body(SPAN, self.vec(), self.vec());
mem::replace(body, empty_body)
}

/// Move a function out by replacing it with an empty [`Function`].
#[inline]
pub fn move_function(self, function: &mut Function<'a>) -> Function<'a> {
let params =
self.formal_parameters(SPAN, FormalParameterKind::FormalParameter, self.vec(), NONE);
let empty_function = self.function(
SPAN,
FunctionType::FunctionDeclaration,
None,
false,
false,
false,
NONE,
NONE,
params,
NONE,
NONE,
);
mem::replace(function, empty_function)
}

/// Move a class out by replacing it with an empty [`Class`].
pub fn move_class(self, class: &mut Class<'a>) -> Class<'a> {
let empty_class = self.class(
SPAN,
ClassType::ClassDeclaration,
self.vec(),
None,
NONE,
None,
NONE,
None,
self.class_body(SPAN, self.vec()),
false,
false,
);
mem::replace(class, empty_class)
}

/// Move an array element out by replacing it with an [`ArrayExpressionElement::Elision`].
pub fn move_array_expression_element(
self,
element: &mut ArrayExpressionElement<'a>,
) -> ArrayExpressionElement<'a> {
let elision = self.array_expression_element_elision(SPAN);
mem::replace(element, elision)
}

/// Take the contents of a arena-allocated [`Vec`], leaving an empty [`Vec`] in its place.
/// This is akin to [`std::mem::take`].
#[inline]
pub fn move_vec<T>(self, vec: &mut Vec<'a, T>) -> Vec<'a, T> {
mem::replace(vec, self.vec())
/// Take the AST node and substitute a dummy node in its place.
pub fn take<T: TakeIn<'a>>(self, value: &mut T) -> T {
value.take_in(self.allocator)
}

/* ---------- Constructors ---------- */
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_isolated_declarations/src/module.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ impl<'a> IsolatedDeclarations<'a> {
stmts.iter_mut().for_each(|stmt| {
if let Statement::ExportNamedDeclaration(decl) = stmt {
if let Some(declaration) = &mut decl.declaration {
*stmt = Statement::from(self.ast.move_declaration(declaration));
*stmt = Statement::from(self.ast.take(declaration));
}
}
});
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_minifier/src/peephole/convert_to_dotted_properties.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ impl<'a> LatePeepholeOptimizations {
let Expression::StringLiteral(s) = &e.expression else { return };
if is_identifier_name(&s.value) {
let property = ctx.ast.identifier_name(s.span, s.value);
let object = ctx.ast.move_expression(&mut e.object);
let object = ctx.ast.take(&mut e.object);
*expr = MemberExpression::StaticMemberExpression(
ctx.ast.alloc_static_member_expression(e.span, object, property, e.optional),
);
Expand Down
42 changes: 21 additions & 21 deletions crates/oxc_minifier/src/peephole/fold_constants.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ impl<'a> PeepholeOptimizations {
// (TRUE || x) => TRUE (also, (3 || x) => 3)
// (FALSE && x) => FALSE
if if lval { op.is_or() } else { op.is_and() } {
return Some(ctx.ast.move_expression(&mut logical_expr.left));
return Some(ctx.ast.take(&mut logical_expr.left));
} else if !left.may_have_side_effects(&ctx) {
let should_keep_indirect_access =
Self::should_keep_indirect_access(&logical_expr.right, ctx);
Expand All @@ -146,19 +146,19 @@ impl<'a> PeepholeOptimizations {
None,
NumberBase::Decimal,
),
ctx.ast.move_expression(&mut logical_expr.right),
ctx.ast.take(&mut logical_expr.right),
]),
));
}
// (FALSE || x) => x
// (TRUE && x) => x
return Some(ctx.ast.move_expression(&mut logical_expr.right));
return Some(ctx.ast.take(&mut logical_expr.right));
}
// Left side may have side effects, but we know its boolean value.
// e.g. true_with_sideeffects || foo() => true_with_sideeffects, foo()
// or: false_with_sideeffects && foo() => false_with_sideeffects, foo()
let left = ctx.ast.move_expression(&mut logical_expr.left);
let right = ctx.ast.move_expression(&mut logical_expr.right);
let left = ctx.ast.take(&mut logical_expr.left);
let right = ctx.ast.take(&mut logical_expr.right);
let vec = ctx.ast.vec_from_array([left, right]);
let sequence_expr = ctx.ast.expression_sequence(logical_expr.span, vec);
return Some(sequence_expr);
Expand All @@ -173,8 +173,8 @@ impl<'a> PeepholeOptimizations {
if !right_boolean && left_child_op.is_or()
|| right_boolean && left_child_op.is_and()
{
let left = ctx.ast.move_expression(&mut left_child.left);
let right = ctx.ast.move_expression(&mut logical_expr.right);
let left = ctx.ast.take(&mut left_child.left);
let right = ctx.ast.take(&mut logical_expr.right);
let logic_expr = ctx.ast.expression_logical(
logical_expr.span,
left,
Expand Down Expand Up @@ -203,8 +203,8 @@ impl<'a> PeepholeOptimizations {
Some(if left.may_have_side_effects(&ctx) {
// e.g. `(a(), null) ?? 1` => `(a(), null, 1)`
let expressions = ctx.ast.vec_from_array([
ctx.ast.move_expression(&mut logical_expr.left),
ctx.ast.move_expression(&mut logical_expr.right),
ctx.ast.take(&mut logical_expr.left),
ctx.ast.take(&mut logical_expr.right),
]);
ctx.ast.expression_sequence(logical_expr.span, expressions)
} else {
Expand All @@ -221,12 +221,12 @@ impl<'a> PeepholeOptimizations {
None,
NumberBase::Decimal,
),
ctx.ast.move_expression(&mut logical_expr.right),
ctx.ast.take(&mut logical_expr.right),
]),
));
}
// nullish condition => this expression evaluates to the right side.
ctx.ast.move_expression(&mut logical_expr.right)
ctx.ast.take(&mut logical_expr.right)
})
}
ValueType::Number
Expand All @@ -247,12 +247,12 @@ impl<'a> PeepholeOptimizations {
None,
NumberBase::Decimal,
),
ctx.ast.move_expression(&mut logical_expr.left),
ctx.ast.take(&mut logical_expr.left),
]),
));
}
// non-nullish condition => this expression evaluates to the left side.
Some(ctx.ast.move_expression(&mut logical_expr.left))
Some(ctx.ast.take(&mut logical_expr.left))
}
ValueType::Undetermined => None,
}
Expand Down Expand Up @@ -390,14 +390,14 @@ impl<'a> PeepholeOptimizations {
let span = Span::new(left_binary_expr.right.span().start, e.right.span().end);
let value = left_str.into_owned() + &right_str;
let right = ctx.ast.expression_string_literal(span, value, None);
let left = ctx.ast.move_expression(&mut left_binary_expr.left);
let left = ctx.ast.take(&mut left_binary_expr.left);
return Some(ctx.ast.expression_binary(e.span, left, e.operator, right));
}

if let Some(new_right) =
Self::try_fold_add_op(&mut left_binary_expr.right, &mut e.right, ctx)
{
let left = ctx.ast.move_expression(&mut left_binary_expr.left);
let left = ctx.ast.take(&mut left_binary_expr.left);
return Some(ctx.ast.expression_binary(e.span, left, e.operator, new_right));
}
}
Expand Down Expand Up @@ -436,7 +436,7 @@ impl<'a> PeepholeOptimizations {
}
left.quasis.extend(right.quasis.drain(1..)); // first quasi is already handled
left.expressions.extend(right.expressions.drain(..));
return Some(ctx.ast.move_expression(left_expr));
return Some(ctx.ast.take(left_expr));
}

// "`${x}y` + 'z'" => "`${x}yz`"
Expand All @@ -452,7 +452,7 @@ impl<'a> PeepholeOptimizations {
.cooked
.map(|cooked| ctx.ast.atom(&(cooked.as_str().to_string() + &right_str)));
last_quasi.value.cooked = new_cooked;
return Some(ctx.ast.move_expression(left_expr));
return Some(ctx.ast.take(left_expr));
}
} else if let Expression::TemplateLiteral(right) = right_expr {
// "'x' + `y${z}`" => "`xy${z}`"
Expand All @@ -470,17 +470,17 @@ impl<'a> PeepholeOptimizations {
.cooked
.map(|cooked| ctx.ast.atom(&(left_str.into_owned() + cooked.as_str())));
first_quasi.value.cooked = new_cooked;
return Some(ctx.ast.move_expression(right_expr));
return Some(ctx.ast.take(right_expr));
}
}

// remove useless `+ ""` (e.g. `typeof foo + ""` -> `typeof foo`)
if Self::evaluates_to_empty_string(left_expr) && right_expr.value_type(&ctx).is_string() {
return Some(ctx.ast.move_expression(right_expr));
return Some(ctx.ast.take(right_expr));
} else if Self::evaluates_to_empty_string(right_expr)
&& left_expr.value_type(&ctx).is_string()
{
return Some(ctx.ast.move_expression(left_expr));
return Some(ctx.ast.take(left_expr));
}

None
Expand Down Expand Up @@ -522,7 +522,7 @@ impl<'a> PeepholeOptimizations {

Some(ctx.ast.expression_binary(
e.span,
ctx.ast.move_expression(expr_to_move),
ctx.ast.take(expr_to_move),
op,
ctx.value_to_expr(Span::new(left.right.span().start, e.right.span().end), v),
))
Expand Down
Loading