Skip to content

Commit

Permalink
fix(minifier): do not dce object literals yet (#6839)
Browse files Browse the repository at this point in the history
closes #6769
  • Loading branch information
Boshen committed Oct 24, 2024
1 parent 3923e63 commit ca79993
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 100 deletions.
91 changes: 4 additions & 87 deletions crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use oxc_allocator::Vec;
use oxc_ast::{ast::*, Visit};
use oxc_ecmascript::constant_evaluation::{ConstantEvaluation, IsLiteralValue};
use oxc_ecmascript::side_effects::MayHaveSideEffects;
use oxc_span::SPAN;
use oxc_traverse::{Ancestor, Traverse, TraverseCtx};

Expand Down Expand Up @@ -390,93 +389,10 @@ impl<'a, 'b> PeepholeRemoveDeadCode {

// `{a: 1, b: 2, c: foo()}` -> `foo()`
fn try_fold_object_expression(
object_expr: &mut ObjectExpression<'a>,
ctx: Ctx<'a, 'b>,
_object_expr: &mut ObjectExpression<'a>,
_ctx: Ctx<'a, 'b>,
) -> Option<Statement<'a>> {
let spread_count = object_expr
.properties
.iter()
.filter(|prop| matches!(prop, ObjectPropertyKind::SpreadProperty(_)))
.count();

if spread_count == object_expr.properties.len() {
return None;
}

// if there is a spread, we can't remove the object expression
if spread_count > 0 {
let original_property_count = object_expr.properties.len();

object_expr.properties.retain(|v| match v {
ObjectPropertyKind::ObjectProperty(object_property) => {
object_property.key.may_have_side_effects()
|| object_property.value.may_have_side_effects()
|| object_property.init.as_ref().is_some_and(
oxc_ecmascript::side_effects::MayHaveSideEffects::may_have_side_effects,
)
}
ObjectPropertyKind::SpreadProperty(_) => true,
});

if original_property_count == object_expr.properties.len() {
return None;
}
return Some(ctx.ast.statement_expression(
object_expr.span,
ctx.ast.expression_from_object(ctx.ast.object_expression(
object_expr.span,
ctx.ast.move_vec(&mut object_expr.properties),
None,
)),
));
}

// we can replace the object with a sequence expression
let mut filtered_properties = ctx.ast.vec();

for prop in object_expr.properties.iter_mut() {
match prop {
ObjectPropertyKind::ObjectProperty(object_prop) => {
let key = object_prop.key.as_expression_mut();
if let Some(key) = key {
if key.may_have_side_effects() {
let key_expr = ctx.ast.move_expression(key);
filtered_properties.push(key_expr);
}
}

if object_prop.value.may_have_side_effects() {
let mut expr = ctx.ast.move_expression(&mut object_prop.value);
filtered_properties.push(ctx.ast.move_expression(&mut expr));
}

if object_prop.init.as_ref().is_some_and(
oxc_ecmascript::side_effects::MayHaveSideEffects::may_have_side_effects,
) {
let mut expr = object_prop.init.take().unwrap();
filtered_properties.push(ctx.ast.move_expression(&mut expr));
}
}
ObjectPropertyKind::SpreadProperty(_) => {
unreachable!("spread property should have been filtered out");
}
}
}

if filtered_properties.len() == 0 {
return Some(ctx.ast.statement_empty(object_expr.span));
} else if filtered_properties.len() == 1 {
return Some(
ctx.ast.statement_expression(object_expr.span, filtered_properties.pop().unwrap()),
);
}

Some(ctx.ast.statement_expression(
object_expr.span,
ctx.ast.expression_from_sequence(
ctx.ast.sequence_expression(object_expr.span, filtered_properties),
),
))
None
}

/// Try folding conditional expression (?:) if the condition results of the condition is known.
Expand Down Expand Up @@ -607,6 +523,7 @@ mod test {
}

#[test]
#[ignore]
fn test_object_literal() {
fold("({})", "");
fold("({a:1})", "");
Expand Down
14 changes: 1 addition & 13 deletions tasks/coverage/snapshots/runtime.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ commit: 06454619

runtime Summary:
AST Parsed : 18444/18444 (100.00%)
Positive Passed: 18260/18444 (99.00%)
Positive Passed: 18264/18444 (99.02%)
tasks/coverage/test262/test/annexB/language/function-code/if-decl-else-decl-a-func-existing-block-fn-no-init.js
minify error: Test262Error: Expected SameValue(«function f(){}», «undefined») to be true

Expand Down Expand Up @@ -417,12 +417,6 @@ transform error: Test262Error: Expected SameValue(«true», «false») to be tru
tasks/coverage/test262/test/language/expressions/object/__proto__-permitted-dup-shorthand.js
codegen error: Test262Error: Expected SameValue(«[object Object]», «2») to be true

tasks/coverage/test262/test/language/expressions/object/accessor-name-computed-err-to-prop-key.js
minify error: Test262Error: `get` accessor Expected a TypeError to be thrown but no exception was thrown at all

tasks/coverage/test262/test/language/expressions/object/accessor-name-computed-err-unresolvable.js
minify error: Test262Error: `get` accessor Expected a ReferenceError to be thrown but no exception was thrown at all

tasks/coverage/test262/test/language/expressions/object/object-spread-proxy-get-not-called-on-dontenum-keys.js
transform error: Test262Error: Actual [dontEnumString, 0, enumerableString, 1, Symbol(dont_enum_symbol), Symbol(enumerable_symbol)] and expected [Symbol(dont_enum_symbol), dontEnumString, 0, Symbol(enumerable_symbol), enumerableString, 1] should have the same contents.

Expand All @@ -432,12 +426,6 @@ transform error: TypeError: Cannot read properties of undefined (reading 'enumer
tasks/coverage/test262/test/language/expressions/object/object-spread-proxy-ownkeys-returned-keys-order.js
transform error: TypeError: Cannot read properties of undefined (reading 'enumerable')

tasks/coverage/test262/test/language/expressions/object/prop-def-id-eval-error-2.js
minify error: Test262Error: Expected a Test262Error to be thrown but no exception was thrown at all

tasks/coverage/test262/test/language/expressions/object/prop-def-id-get-error.js
minify error: Test262Error: Expected a ReferenceError to be thrown but no exception was thrown at all

tasks/coverage/test262/test/language/expressions/subtraction/S11.6.2_A4_T5.js
minify error: Test262Error: #3.2: -0 - 0 === - 0. Actual: +0

Expand Down

0 comments on commit ca79993

Please sign in to comment.