From a750ebc324f3abd66b754e9bc6d55096dc13cddd Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Tue, 10 Dec 2024 02:28:24 +0000 Subject: [PATCH] refactor(transformer): `duplicate_expression` take `mutated_symbol_needs_temp_var` param (#7756) `TransformCtx::duplicate_expression` method introduced in #7754 take a param `mutated_symbol_needs_temp_var`. If `true`, `duplicate_expression` will create a temp var for an `IdentifierReference` whose's symbol is assigned to elsewhere in the AST. If `false`, it won't. This is required because different transforms which use `duplicate_expression` have different requirements. --- .../oxc_transformer/src/common/duplicate.rs | 36 ++++++++++++++----- .../src/es2022/class_properties/private.rs | 4 +-- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/crates/oxc_transformer/src/common/duplicate.rs b/crates/oxc_transformer/src/common/duplicate.rs index 2060c547dfe07..c44a95bf3f192 100644 --- a/crates/oxc_transformer/src/common/duplicate.rs +++ b/crates/oxc_transformer/src/common/duplicate.rs @@ -22,14 +22,19 @@ impl<'a> TransformCtx<'a> { /// * Unbound identifier `foo` -> `_foo = foo`, `_foo` /// * Anything else `foo()` -> `_foo = foo()`, `_foo` /// + /// If `mutated_symbol_needs_temp_var` is `true`, temp var will be created for a bound identifier, + /// if it's mutated (assigned to) anywhere in AST. + /// /// Returns 2 `Expression`s. The first may be an `AssignmentExpression`, /// and must be inserted into output first. pub(crate) fn duplicate_expression( &self, expr: Expression<'a>, + mutated_symbol_needs_temp_var: bool, ctx: &mut TraverseCtx<'a>, ) -> (Expression<'a>, Expression<'a>) { - let (maybe_assignment, references) = self.duplicate_expression_multiple::<1>(expr, ctx); + let (maybe_assignment, references) = + self.duplicate_expression_multiple::<1>(expr, mutated_symbol_needs_temp_var, ctx); let [reference] = references; (maybe_assignment, reference) } @@ -43,15 +48,20 @@ impl<'a> TransformCtx<'a> { /// * Unbound identifier `foo` -> `_foo = foo`, `_foo`, `_foo` /// * Anything else `foo()` -> `_foo = foo()`, `_foo`, `_foo` /// + /// If `mutated_symbol_needs_temp_var` is `true`, temp var will be created for a bound identifier, + /// if it's mutated (assigned to) anywhere in AST. + /// /// Returns 3 `Expression`s. The first may be an `AssignmentExpression`, /// and must be inserted into output first. #[expect(clippy::similar_names)] pub(crate) fn duplicate_expression_twice( &self, expr: Expression<'a>, + mutated_symbol_needs_temp_var: bool, ctx: &mut TraverseCtx<'a>, ) -> (Expression<'a>, Expression<'a>, Expression<'a>) { - let (maybe_assignment, references) = self.duplicate_expression_multiple::<2>(expr, ctx); + let (maybe_assignment, references) = + self.duplicate_expression_multiple::<2>(expr, mutated_symbol_needs_temp_var, ctx); let [reference1, reference2] = references; (maybe_assignment, reference1, reference2) } @@ -65,26 +75,36 @@ impl<'a> TransformCtx<'a> { /// * Unbound identifier `foo` -> `_foo = foo`, [`_foo`; N] /// * Anything else `foo()` -> `_foo = foo()`, [`_foo`; N] /// + /// If `mutated_symbol_needs_temp_var` is `true`, temp var will be created for a bound identifier, + /// if it's mutated (assigned to) anywhere in AST. + /// /// Returns `N + 1` x `Expression`s. The first may be an `AssignmentExpression`, /// and must be inserted into output first. pub(crate) fn duplicate_expression_multiple( &self, expr: Expression<'a>, + mutated_symbol_needs_temp_var: bool, ctx: &mut TraverseCtx<'a>, ) -> (Expression<'a>, [Expression<'a>; N]) { // TODO: Handle if in a function's params let temp_var_binding = match &expr { Expression::Identifier(ident) => { - let reference = ctx.symbols_mut().get_reference_mut(ident.reference_id()); + let reference_id = ident.reference_id(); + let reference = ctx.symbols().get_reference(reference_id); if let Some(symbol_id) = reference.symbol_id() { - // Reading bound identifier cannot have side effects, so no need for temp var - let binding = BoundIdentifier::new(ident.name.clone(), symbol_id); - let references = - create_array(|| binding.create_spanned_read_expression(ident.span, ctx)); - return (expr, references); + if !mutated_symbol_needs_temp_var || !ctx.symbols().symbol_is_mutated(symbol_id) + { + // Reading bound identifier cannot have side effects, so no need for temp var + let binding = BoundIdentifier::new(ident.name.clone(), symbol_id); + let references = create_array(|| { + binding.create_spanned_read_expression(ident.span, ctx) + }); + return (expr, references); + } } // Previously `x += 1` (`x` read + write), but moving to `_x = x` (`x` read only) + let reference = ctx.symbols_mut().get_reference_mut(reference_id); *reference.flags_mut() = ReferenceFlags::Read; self.var_declarations.create_uid_var(&ident.name, ctx) diff --git a/crates/oxc_transformer/src/es2022/class_properties/private.rs b/crates/oxc_transformer/src/es2022/class_properties/private.rs index 98332469f1348..feae10b758bf4 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/private.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/private.rs @@ -1463,7 +1463,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { ctx: &mut TraverseCtx<'a>, ) -> (Expression<'a>, Expression<'a>) { assert_expr_neither_parenthesis_nor_typescript_syntax(&object); - self.ctx.duplicate_expression(object, ctx) + self.ctx.duplicate_expression(object, false, ctx) } /// Duplicate object to be used in triple. @@ -1482,7 +1482,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { ctx: &mut TraverseCtx<'a>, ) -> (Expression<'a>, Expression<'a>, Expression<'a>) { assert_expr_neither_parenthesis_nor_typescript_syntax(&object); - self.ctx.duplicate_expression_twice(object, ctx) + self.ctx.duplicate_expression_twice(object, false, ctx) } /// `_classPrivateFieldGet2(_prop, object)`