From a596821d859b49d2ba8203937048900a947993e0 Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Thu, 9 Jan 2025 03:47:11 +0000 Subject: [PATCH] feat(minifier): compress `a.b = a.b + c` to `a.b += c` (#8367) The simplified version of the evaluation of `a += b` is: > AssignmentExpression : LeftHandSideExpression AssignmentOperator AssignmentExpression > 1. Let lRef be ? Evaluation of LeftHandSideExpression. > 2. Let lVal be ? GetValue(lRef). > 3. Let rRef be ? Evaluation of AssignmentExpression. > 4. Let rVal be ? GetValue(rRef). > 5. Let r be ? ApplyStringOrNumericBinaryOperator(lVal, opText, rVal). > 6. Perform ? PutValue(lRef, r). > 7. Return r. The simplified version of the evaluation of `a = a + b` is: > AssignmentExpression : LeftHandSideExpression = AssignmentExpressionLeft + AssignmentExpressionRight > 1. Let lRef be ? Evaluation of LeftHandSideExpression. > 2. Let alRef be ? Evaluation of AssignmentExpressionLeft. > 3. Let alVal be ? GetValue(alRef). > 4. Let arRef be ? Evaluation of AssignmentExpressionRight. > 5. Let arVal be ? GetValue(arRef). > 6. Let rRef be ? ApplyStringOrNumericBinaryOperator(alVal, opText, arVal). > 7. Let rVal be ? GetValue(rRef). [Note GetValue(rRef) returns rRef itself] > 8. Perform ? PutValue(lRef, rVal). > 9. Return rVal. The difference of these is that the evaluation of `a` is done twice for `a = a + b`, one with `1. Let lRef be ? Evaluation of LeftHandSideExpression` and one with `2. Let alRef be ? Evaluation of AssignmentExpressionLeft.` So this is same with #8366 and can be compressed similarly when the conditions are met (`a.b = a.b + c` -> `a.b += c`). **References** - [Spec of `=`, `+=`](https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-assignment-operators-runtime-semantics-evaluation) - [Spec of `+`](https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-addition-operator-plus-runtime-semantics-evaluation) --- .../peephole_substitute_alternate_syntax.rs | 78 ++++++++++++------- tasks/minsize/minsize.snap | 4 +- 2 files changed, 53 insertions(+), 29 deletions(-) diff --git a/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs b/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs index 3a02a892611bc..2ba0b8aa4842b 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs @@ -481,18 +481,39 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { let new_op = expr.operator.to_assignment_operator(); - match (&assignment_expr.left, &expr.left) { - // `a || (a = b)` -> `a ||= b` + if !Self::has_no_side_effect_for_evaluation_same_target( + &assignment_expr.left, + &expr.left, + ctx, + ) { + return None; + } + + assignment_expr.span = expr.span; + assignment_expr.operator = new_op; + Some(ctx.ast.move_expression(&mut expr.right)) + } + + /// Returns `true` if the assignment target and expression have no side effect for *evaluation* and points to the same reference. + /// + /// Evaluation here means `Evaluation` in the spec. + /// + /// + /// Matches the following cases: + /// + /// - `a`, `a` + /// - `a.b`, `a.b` + /// - `a.#b`, `a.#b` + fn has_no_side_effect_for_evaluation_same_target( + assignment_target: &AssignmentTarget, + expr: &Expression, + ctx: Ctx<'a, 'b>, + ) -> bool { + match (&assignment_target, &expr) { ( AssignmentTarget::AssignmentTargetIdentifier(write_id_ref), Expression::Identifier(read_id_ref), - ) => { - if write_id_ref.name != read_id_ref.name { - return None; - } - } - // `a.b || (a.b = c)` -> `a.b ||= c` - // `a.#b || (a.#b = c)` -> `a.#b ||= c` + ) => write_id_ref.name == read_id_ref.name, ( AssignmentTarget::StaticMemberExpression(_), Expression::StaticMemberExpression(_), @@ -501,24 +522,17 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { AssignmentTarget::PrivateFieldExpression(_), Expression::PrivateFieldExpression(_), ) => { - let write_expr = assignment_expr.left.to_member_expression(); - let read_expr = expr.left.to_member_expression(); + let write_expr = assignment_target.to_member_expression(); + let read_expr = expr.to_member_expression(); let Expression::Identifier(write_expr_object_id) = &write_expr.object() else { - return None; + return false; }; - // It should also return None when the reference might refer to a reference value created by a with statement + // It should also return false when the reference might refer to a reference value created by a with statement // when the minifier supports with statements - if ctx.is_global_reference(write_expr_object_id) || write_expr.content_ne(read_expr) - { - return None; - } + !ctx.is_global_reference(write_expr_object_id) && write_expr.content_eq(read_expr) } - _ => return None, + _ => false, } - - assignment_expr.span = expr.span; - assignment_expr.operator = new_op; - Some(ctx.ast.move_expression(&mut expr.right)) } fn commutative_pair( @@ -603,14 +617,12 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { if !matches!(expr.operator, AssignmentOperator::Assign) { return; } - let AssignmentTarget::AssignmentTargetIdentifier(write_id_ref) = &mut expr.left else { - return; - }; let Expression::BinaryExpression(binary_expr) = &mut expr.right else { return }; let Some(new_op) = binary_expr.operator.to_assignment_operator() else { return }; - let Expression::Identifier(read_id_ref) = &mut binary_expr.left else { return }; - if write_id_ref.name != read_id_ref.name { + + if !Self::has_no_side_effect_for_evaluation_same_target(&expr.left, &binary_expr.left, ctx) + { return; } @@ -1184,6 +1196,18 @@ mod test { test_same("x = g() & x"); test_same("x = (x -= 2) ^ x"); + + // GetValue(x) has no sideeffect when x is a resolved identifier + test("var x; x.y = x.y + 3", "var x; x.y += 3"); + test("var x; x.#y = x.#y + 3", "var x; x.#y += 3"); + test_same("x.y = x.y + 3"); + // this can be compressed if `y` does not have side effect + test_same("var x; x[y] = x[y] + 3"); + // GetValue(x) has a side effect in this case + // Example case: `var a = { get b() { console.log('b'); return { get c() { console.log('c') } } } }; a.b.c = a.b.c + 1` + test_same("var x; x.y.z = x.y.z + 3"); + // This case is not supported, since the minifier does not support with statements + // test_same("var x; with (z) { x.y || (x.y = 3) }"); } #[test] diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index 013f70c8088ee..c4776eedb9276 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -15,7 +15,7 @@ Original | minified | minified | gzip | gzip | Fixture 1.01 MB | 460.18 kB | 458.89 kB | 126.76 kB | 126.71 kB | bundle.min.js -1.25 MB | 652.84 kB | 646.76 kB | 163.52 kB | 163.73 kB | three.js +1.25 MB | 652.82 kB | 646.76 kB | 163.51 kB | 163.73 kB | three.js 2.14 MB | 726.27 kB | 724.14 kB | 180.14 kB | 181.07 kB | victory.js @@ -23,5 +23,5 @@ Original | minified | minified | gzip | gzip | Fixture 6.69 MB | 2.32 MB | 2.31 MB | 492.65 kB | 488.28 kB | antd.js -10.95 MB | 3.49 MB | 3.49 MB | 907.50 kB | 915.50 kB | typescript.js +10.95 MB | 3.49 MB | 3.49 MB | 907.49 kB | 915.50 kB | typescript.js