Skip to content

Commit

Permalink
fix(transformer): correct all ReferenceFlags (#7410)
Browse files Browse the repository at this point in the history
  • Loading branch information
Dunqing committed Nov 22, 2024
1 parent 6f0fe38 commit 7ff9f13
Show file tree
Hide file tree
Showing 14 changed files with 52 additions and 17,286 deletions.
15 changes: 7 additions & 8 deletions crates/oxc_transformer/src/es2016/exponentiation_operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use oxc_ast::{ast::*, NONE};
use oxc_semantic::{ReferenceFlags, SymbolFlags};
use oxc_span::SPAN;
use oxc_syntax::operator::{AssignmentOperator, BinaryOperator};
use oxc_traverse::{Ancestor, BoundIdentifier, Traverse, TraverseCtx};
use oxc_traverse::{BoundIdentifier, Traverse, TraverseCtx};

use crate::TransformCtx;

Expand Down Expand Up @@ -155,15 +155,14 @@ impl<'a, 'ctx> ExponentiationOperator<'a, 'ctx> {

// Make sure side-effects of evaluating `left` only happen once
let reference = ctx.scoping.symbols_mut().get_reference_mut(ident.reference_id());

// `left **= right` is being transformed to `left = Math.pow(left, right)`,
// so if `left` is no longer being read from, update its `ReferenceFlags`.
*reference.flags_mut() = ReferenceFlags::Write;

let pow_left = if let Some(symbol_id) = reference.symbol_id() {
// This variable is declared in scope so evaluating it multiple times can't trigger a getter.
// No need for a temp var.
// `left **= right` is being transformed to `left = Math.pow(left, right)`,
// so if `left` is no longer being read from, update its `ReferenceFlags`.
if matches!(ctx.ancestry.parent(), Ancestor::ExpressionStatementExpression(_)) {
*reference.flags_mut() = ReferenceFlags::Write;
}

ctx.create_bound_ident_expr(SPAN, ident.name.clone(), symbol_id, ReferenceFlags::Read)
} else {
// Unbound reference. Could possibly trigger a getter so we need to only evaluate it once.
Expand Down Expand Up @@ -571,7 +570,7 @@ impl<'a, 'ctx> ExponentiationOperator<'a, 'ctx> {
temp_var_inits.push(ctx.ast.expression_assignment(
SPAN,
AssignmentOperator::Assign,
binding.create_read_write_target(ctx),
binding.create_write_target(ctx),
expr,
));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ impl<'a, 'ctx> AsyncGeneratorFunctions<'a, 'ctx> {
Some(ctx.ast.expression_assignment(
SPAN,
AssignmentOperator::Assign,
iterator_abrupt_completion.create_read_write_target(ctx),
iterator_abrupt_completion.create_write_target(ctx),
ctx.ast.expression_unary(
SPAN,
UnaryOperator::LogicalNot,
Expand All @@ -293,7 +293,7 @@ impl<'a, 'ctx> AsyncGeneratorFunctions<'a, 'ctx> {
ctx.ast.expression_assignment(
SPAN,
AssignmentOperator::Assign,
step_key.create_read_write_target(ctx),
step_key.create_write_target(ctx),
ctx.ast.expression_await(
SPAN,
ctx.ast.expression_call(
Expand All @@ -319,7 +319,7 @@ impl<'a, 'ctx> AsyncGeneratorFunctions<'a, 'ctx> {
Some(ctx.ast.expression_assignment(
SPAN,
AssignmentOperator::Assign,
iterator_abrupt_completion.create_read_write_target(ctx),
iterator_abrupt_completion.create_write_target(ctx),
ctx.ast.expression_boolean_literal(SPAN, false),
)),
{
Expand Down
10 changes: 2 additions & 8 deletions crates/oxc_transformer/src/es2018/object_rest_spread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use oxc_allocator::{CloneIn, GetAddress, Vec as ArenaVec};
use oxc_ast::{ast::*, NONE};
use oxc_diagnostics::OxcDiagnostic;
use oxc_ecmascript::{BoundNames, ToJsString};
use oxc_semantic::{IsGlobalReference, ReferenceFlags, ScopeFlags, ScopeId, SymbolFlags};
use oxc_semantic::{IsGlobalReference, ScopeFlags, ScopeId, SymbolFlags};
use oxc_span::{GetSpan, SPAN};
use oxc_traverse::{Ancestor, MaybeBoundIdentifier, Traverse, TraverseCtx};

Expand Down Expand Up @@ -244,7 +244,7 @@ impl<'a, 'ctx> ObjectRestSpread<'a, 'ctx> {
expressions.push(ctx.ast.expression_assignment(
SPAN,
op,
reference_builder.maybe_bound_identifier.create_read_write_target(ctx),
reference_builder.maybe_bound_identifier.create_write_target(ctx),
expr,
));
}
Expand All @@ -266,12 +266,6 @@ impl<'a, 'ctx> ObjectRestSpread<'a, 'ctx> {
ctx,
);
if let BindingPatternOrAssignmentTarget::AssignmentTarget(lhs) = lhs {
if let AssignmentTarget::AssignmentTargetIdentifier(ident) = &lhs {
ctx.symbols_mut()
.get_reference_mut(ident.reference_id())
.flags_mut()
.insert(ReferenceFlags::Read);
}
expressions.push(ctx.ast.expression_assignment(SPAN, op, lhs, rhs));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ impl<'a, 'ctx> NullishCoalescingOperator<'a, 'ctx> {
let assignment = ctx.ast.expression_assignment(
SPAN,
AssignmentOperator::Assign,
binding.create_read_write_target(ctx),
binding.create_write_target(ctx),
logical_expr.left,
);
let mut new_expr = Self::create_conditional_expression(
Expand Down
18 changes: 6 additions & 12 deletions crates/oxc_transformer/src/es2020/optional_chaining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ impl<'a, 'ctx> OptionalChaining<'a, 'ctx> {
// `foo.bar` -> `_foo$bar = foo.bar`
let binding = self.generate_binding(object, ctx);
*object = Self::create_assignment_expression(
binding.create_read_write_target(ctx),
binding.create_write_target(ctx),
ctx.ast.move_expression(object),
ctx,
);
Expand Down Expand Up @@ -572,7 +572,7 @@ impl<'a, 'ctx> OptionalChaining<'a, 'ctx> {
let binding = self.generate_binding(object, ctx);
// `(_foo = foo)`
*object = Self::create_assignment_expression(
binding.create_read_write_target(ctx),
binding.create_write_target(ctx),
ctx.ast.move_expression(object),
ctx,
);
Expand All @@ -590,11 +590,8 @@ impl<'a, 'ctx> OptionalChaining<'a, 'ctx> {
// Replace the expression with the temp binding and assign the original expression to the temp binding
let expr = mem::replace(expr, temp_binding.create_read_expression(ctx));
// `(binding = expr)`
let assignment_expression = Self::create_assignment_expression(
temp_binding.create_read_write_target(ctx),
expr,
ctx,
);
let assignment_expression =
Self::create_assignment_expression(temp_binding.create_write_target(ctx), expr, ctx);
// `(binding = expr) === null || binding === void 0`
let expr = self.wrap_optional_check(
assignment_expression,
Expand Down Expand Up @@ -640,11 +637,8 @@ impl<'a, 'ctx> OptionalChaining<'a, 'ctx> {
// Replace the expression with the temp binding and assign the original expression to the temp binding
let expr = mem::replace(expr, temp_binding.create_read_expression(ctx));
// `(binding = expr)`
let assignment_expression = Self::create_assignment_expression(
temp_binding.create_read_write_target(ctx),
expr,
ctx,
);
let assignment_expression =
Self::create_assignment_expression(temp_binding.create_write_target(ctx), expr, ctx);

let reference = temp_binding.create_read_expression(ctx);
// `left || (binding = expr) === null`
Expand Down
16 changes: 6 additions & 10 deletions crates/oxc_transformer/src/es2021/logical_assignment_operators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,8 @@ impl<'a, 'ctx> LogicalAssignmentOperators<'a, 'ctx> {
let symbol_id = reference.symbol_id();
let left_expr = Expression::Identifier(ctx.alloc(ident.clone()));

let ident = ctx.create_ident_reference(
SPAN,
ident.name.clone(),
symbol_id,
ReferenceFlags::read_write(),
);
let ident =
ctx.create_ident_reference(SPAN, ident.name.clone(), symbol_id, ReferenceFlags::Write);
let assign_target = AssignmentTarget::AssignmentTargetIdentifier(ctx.alloc(ident));
(left_expr, assign_target)
}
Expand All @@ -152,7 +148,7 @@ impl<'a, 'ctx> LogicalAssignmentOperators<'a, 'ctx> {
if let Some(ident) = self.maybe_generate_memoised(&static_expr.object, ctx) {
// (_o = o).a
let right = ctx.ast.move_expression(&mut static_expr.object);
let target = ident.create_read_write_target(ctx);
let target = ident.create_write_target(ctx);
let object =
ctx.ast.expression_assignment(SPAN, AssignmentOperator::Assign, target, right);
let left_expr = Expression::from(ctx.ast.member_expression_static(
Expand Down Expand Up @@ -207,7 +203,7 @@ impl<'a, 'ctx> LogicalAssignmentOperators<'a, 'ctx> {
if let Some(ident) = self.maybe_generate_memoised(&computed_expr.object, ctx) {
// (_o = object)
let right = ctx.ast.move_expression(&mut computed_expr.object);
let target = ident.create_read_write_target(ctx);
let target = ident.create_write_target(ctx);
let object =
ctx.ast.expression_assignment(SPAN, AssignmentOperator::Assign, target, right);

Expand All @@ -220,7 +216,7 @@ impl<'a, 'ctx> LogicalAssignmentOperators<'a, 'ctx> {
expression = ctx.ast.expression_assignment(
SPAN,
AssignmentOperator::Assign,
property.create_read_write_target(ctx),
property.create_write_target(ctx),
expression,
);
}
Expand Down Expand Up @@ -259,7 +255,7 @@ impl<'a, 'ctx> LogicalAssignmentOperators<'a, 'ctx> {
ctx.ast.expression_assignment(
SPAN,
AssignmentOperator::Assign,
property_ident.create_read_write_target(ctx),
property_ident.create_write_target(ctx),
ctx.ast.move_expression(&mut expression),
)
} else {
Expand Down
36 changes: 2 additions & 34 deletions crates/oxc_transformer/src/es2022/class_static_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,9 @@
use itoa::Buffer as ItoaBuffer;

use oxc_allocator::String as ArenaString;
use oxc_ast::{ast::*, Visit, NONE};
use oxc_semantic::SymbolTable;
use oxc_ast::{ast::*, NONE};
use oxc_span::SPAN;
use oxc_syntax::{
reference::ReferenceFlags,
scope::{ScopeFlags, ScopeId},
};
use oxc_syntax::scope::{ScopeFlags, ScopeId};
use oxc_traverse::{Traverse, TraverseCtx};

pub struct ClassStaticBlock;
Expand Down Expand Up @@ -191,38 +187,10 @@ impl ClassStaticBlock {
// Remove the scope for the static block from the scope chain
ctx.remove_scope_for_expression(scope_id, &expr);

// If expression is an assignment, left side has moved from a write-only position to a read + write one.
// `static { x = 1; }` -> `static #_ = x = 1;`
// So set `ReferenceFlags::Read` on the left side.
if let Expression::AssignmentExpression(assign_expr) = &expr {
if assign_expr.operator == AssignmentOperator::Assign {
let mut setter = ReferenceFlagsSetter { symbols: ctx.symbols_mut() };
setter.visit_assignment_target(&assign_expr.left);
}
}

expr
}
}

/// Visitor which sets `ReferenceFlags::Read` flag on all `IdentifierReference`s.
/// It skips `MemberExpression`s, because their flags are not affected by the change in position.
struct ReferenceFlagsSetter<'s> {
symbols: &'s mut SymbolTable,
}

impl<'a, 's> Visit<'a> for ReferenceFlagsSetter<'s> {
fn visit_identifier_reference(&mut self, ident: &IdentifierReference<'a>) {
let reference_id = ident.reference_id();
let reference = self.symbols.get_reference_mut(reference_id);
*reference.flags_mut() |= ReferenceFlags::Read;
}

fn visit_member_expression(&mut self, _member_expr: &MemberExpression<'a>) {
// Don't traverse further
}
}

/// Store of private identifier keys matching `#_` or `#_[1-9]...`.
///
/// Most commonly there will be no existing keys matching this pattern
Expand Down
11 changes: 3 additions & 8 deletions crates/oxc_transformer/src/jsx/refresh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,11 +392,10 @@ impl<'a, 'ctx> ReactRefresh<'a, 'ctx> {
fn create_registration(
&mut self,
persistent_id: Atom<'a>,
reference_flags: ReferenceFlags,
ctx: &mut TraverseCtx<'a>,
) -> AssignmentTarget<'a> {
let binding = ctx.generate_uid_in_root_scope("c", SymbolFlags::FunctionScopedVariable);
let target = binding.create_target(reference_flags, ctx);
let target = binding.create_target(ReferenceFlags::Write, ctx);
self.registrations.push((binding, persistent_id));
target
}
Expand Down Expand Up @@ -478,11 +477,7 @@ impl<'a, 'ctx> ReactRefresh<'a, 'ctx> {
*expr = ctx.ast.expression_assignment(
SPAN,
AssignmentOperator::Assign,
self.create_registration(
ctx.ast.atom(inferred_name),
ReferenceFlags::read_write(),
ctx,
),
self.create_registration(ctx.ast.atom(inferred_name), ctx),
ctx.ast.move_expression(expr),
);
}
Expand All @@ -496,7 +491,7 @@ impl<'a, 'ctx> ReactRefresh<'a, 'ctx> {
id: &BindingIdentifier<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Statement<'a> {
let left = self.create_registration(id.name.clone(), ReferenceFlags::Write, ctx);
let left = self.create_registration(id.name.clone(), ctx);
let right = ctx.create_bound_ident_expr(
SPAN,
id.name.clone(),
Expand Down
Loading

0 comments on commit 7ff9f13

Please sign in to comment.