Skip to content
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.

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

Make expr_simplifier handle expressions when indexing strings, arrays and objects #8750

Open
wants to merge 74 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
74 commits
Select commit Hold shift + click to select a range
84ec94e
Initial commit: string literals
levi-nz Mar 15, 2024
7f02ace
WIP: array literals
levi-nz Mar 15, 2024
2b94d3d
Correctly handle array indexing
levi-nz Mar 16, 2024
efec60b
Update unit test
levi-nz Mar 17, 2024
72537ac
Fix broken index logic
levi-nz Mar 17, 2024
454378d
Fix indexing `ObjectLit`s
levi-nz Mar 17, 2024
9b26630
Remove fraction check from `LiteralVisitor`
levi-nz Mar 18, 2024
677a437
cargo fmt
levi-nz Mar 18, 2024
3aa713e
Update test files
levi-nz Mar 18, 2024
8450f8b
Handle `x['0']`
levi-nz Mar 31, 2024
2b8709f
Don't return `KnownOp::Len` if `obj` is an ObjectLiteral
levi-nz Mar 31, 2024
dd4336b
Handle `x['length']`
levi-nz Mar 31, 2024
2fe5f09
Correctly handle builtin properties
levi-nz Mar 31, 2024
c902739
Don't replace ArrayLiteral if doing so may have side effects
levi-nz Mar 31, 2024
b4455c9
Update unit test
levi-nz Mar 31, 2024
ae673de
Update minifier test files
levi-nz Mar 31, 2024
4143a27
cargo fmt
levi-nz Mar 31, 2024
4b151fc
Minifier shouldn't handle potential `pristine_globals` cases
levi-nz Apr 1, 2024
c154047
cargo fmt
levi-nz Apr 1, 2024
647cfd3
Update test files
levi-nz Apr 1, 2024
9c9d141
Don't add a dummy value if no side effects exist
levi-nz Apr 1, 2024
4424755
Update test files
levi-nz Apr 1, 2024
96a1c0b
Oops
levi-nz Apr 1, 2024
5104b9a
Update unit test
levi-nz Apr 1, 2024
bfd376b
Add test case for side effects
levi-nz Apr 1, 2024
9296237
Add compression for `MemberExpr`
levi-nz Apr 1, 2024
2b26790
cargo fmt
levi-nz Apr 1, 2024
4f2c69f
String does not appear to inherit Function
levi-nz Apr 2, 2024
43fe050
Array also does not inherit Function
levi-nz Apr 2, 2024
70f6f64
Remove comment
levi-nz Apr 2, 2024
d19e907
`FUNCTION_SYMBOLS` is no longer required
levi-nz Apr 2, 2024
10c4681
Add unit test files
levi-nz Apr 2, 2024
240713a
Update test files
levi-nz Apr 2, 2024
ec03388
Update test config
levi-nz Apr 3, 2024
bfcb5cf
Better test dir names
levi-nz Apr 3, 2024
17439b9
Remove evil clone
levi-nz Apr 3, 2024
67474ef
I don't know why this changed, but it did.
levi-nz Apr 3, 2024
c92508f
Add array side effect unit test
levi-nz Apr 3, 2024
9407838
Don't optimize if inside left-hand side of AssignExpr
levi-nz Apr 3, 2024
942a343
Remove redundant import
levi-nz Apr 3, 2024
5424fed
Move call to end
levi-nz Apr 3, 2024
c6029c4
Cleanup iterator call
levi-nz Apr 3, 2024
ea89ec2
Handle side effects and `__proto__` in object compression
levi-nz Apr 8, 2024
976ddb7
Remove redundant return statement
levi-nz Apr 8, 2024
a819d98
cargo fmt
levi-nz Apr 8, 2024
b41617c
Remove redundant `Box::new` call
levi-nz Apr 8, 2024
a5df0a5
Use `preserve_effects`
levi-nz Apr 8, 2024
e47bff3
cargo fmt
levi-nz Apr 8, 2024
52dfab4
Exclude `watch` and `unwatch`
levi-nz Apr 9, 2024
4a1b6db
UPDATE=1 cargo test --test projects --test tsc
levi-nz Apr 9, 2024
d4696ed
UPDATE=1 cargo test -p swc_ecma_minifier --features concurrent
levi-nz Apr 9, 2024
7dba5ac
git merge upstream/main issue-8747
levi-nz Apr 10, 2024
00cb04f
Remove redundant import
levi-nz Apr 10, 2024
6477fbf
Fix `test_fold_array_lit_spread_get_elem`
levi-nz Apr 10, 2024
9b5239d
Fix `test_fold_get_elem1`
levi-nz Apr 10, 2024
f3cfa71
Add `self.changed = true;`
levi-nz Apr 10, 2024
b8cd66b
Use `ctx`
levi-nz Apr 22, 2024
b2a2cc6
Fix doc
levi-nz Apr 25, 2024
5458077
Handle SeqExpr in `optimize_member_expr`
levi-nz Apr 25, 2024
5e1819f
Remove dup `self.changed = true;`
levi-nz May 10, 2024
4a2d3a9
git merge upstream/main issue-8747
levi-nz May 10, 2024
45d23d3
Apply clippy suggestions
levi-nz May 10, 2024
52869c4
Cleanup `optimize_member_expr`
levi-nz May 12, 2024
aa42961
Fix optimization tests
levi-nz May 12, 2024
2b7ab8e
Fix optimization with `OptCall`
levi-nz May 25, 2024
03c7dd5
Move code into its own `eval` function
levi-nz May 25, 2024
60bcf71
Fix error
levi-nz May 25, 2024
d613a05
Remove redundant imports
levi-nz May 25, 2024
2b5da79
Optimize imports
levi-nz May 25, 2024
66c51a4
Update parserForStatement9.2.minified.js
levi-nz May 25, 2024
cc2dcab
Cleanup simplify
levi-nz May 27, 2024
dca62ff
Handle `__proto__` object when indexing known object keys
levi-nz May 30, 2024
c913a15
cargo fmt
levi-nz May 30, 2024
591cbb0
Remove `is_opt_call`
levi-nz Jun 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//// [destructuringArrayBindingPatternAndAssignment2.ts]
import { _ as _sliced_to_array } from "@swc/helpers/_/_sliced_to_array";
import { _ as _to_consumable_array } from "@swc/helpers/_/_to_consumable_array";
var _ref_1 = (_sliced_to_array([][0], 1)[0], _sliced_to_array([][1], 1));
var _ref_1 = (_sliced_to_array(void 0, 1)[0], _sliced_to_array(void 0, 1));
_sliced_to_array(_ref_1[0], 1)[0];
var _undefined = _sliced_to_array(void 0, 2), _undefined_1 = (_sliced_to_array(_undefined[0], 1)[0], _sliced_to_array(_undefined[1], 1));
_sliced_to_array(_undefined_1[0], 1)[0];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
//// [destructuringControlFlow.ts]
import "@swc/helpers/_/_sliced_to_array";
[
"foo"
][1].toUpperCase();
(void 0).toUpperCase();
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { _ as _sliced_to_array } from "@swc/helpers/_/_sliced_to_array";
import { _ as _to_property_key } from "@swc/helpers/_/_to_property_key";
var trace = [], order = function(n) {
return trace.push(n);
}, tmp = [][0];
}, tmp = void 0;
(void 0 === tmp ? order(0) : tmp)[order(1)];
var tmp1 = {};
(void 0 === tmp1 ? order(0) : tmp1)[order(1)];
Expand Down
2 changes: 1 addition & 1 deletion crates/swc/tests/tsc-references/enumBasics.2.minified.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
//// [parserForStatement9.ts]
for(var tmp = [][0], x = void 0 === tmp ? ("a" in {}) : tmp; !x; x = !x)console.log(x);
for(var tmp = void 0, x = void 0 === tmp ? ("a" in {}) : tmp; !x; x = !x)console.log(x);
for(var _ref_x = {}.x, x1 = void 0 === _ref_x ? ("a" in {}) : _ref_x; !x1; x1 = !x1)console.log(x1);
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
//// [templateStringInIndexExpressionES6.ts]
"abc0abc"["0"];
181 changes: 108 additions & 73 deletions crates/swc_ecma_transforms_optimization/src/simplify/expr/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{borrow::Cow, iter, iter::once};

use swc_atoms::JsWord;
use swc_atoms::{Atom, JsWord};
use swc_common::{
pass::{CompilerPass, Repeated},
util::take::Take,
Expand Down Expand Up @@ -107,12 +107,21 @@ impl SimplifyExpr {
return;
}

#[derive(Clone, PartialEq, Eq)]
#[derive(Clone, PartialEq)]
enum KnownOp {
/// [a, b].length
Len,

Index(i64),
// [a, b][0]
//
// ({0.5: 'test'})[0.5]
/// Note: callers need to check `v.fract() == 0.0` in some cases.
/// ie non-integer indexes for arrays always result in `undefined`
/// but not for objects (because indexing an object
/// returns the value of the key, ie `0.5` will not
/// return `undefined` if a key `0.5` exists
/// and its value is not `undefined`).
Index(f64),

/// ({}).foo
IndexStr(JsWord),
Expand All @@ -127,16 +136,16 @@ impl SimplifyExpr {
}
}
MemberProp::Computed(ComputedPropName { expr, .. }) => {
if !self.in_callee {
if let Expr::Lit(Lit::Num(Number { value, .. })) = &**expr {
if value.fract() == 0.0 {
KnownOp::Index(*value as _)
} else {
return;
}
} else {
return;
}
if self.in_callee {
return;
}

if let Expr::Lit(Lit::Num(Number { value, .. })) = &**expr {
// x[5]
KnownOp::Index(*value)
} else if let Known(s) = expr.as_pure_string(&self.expr_ctx) {
// x[''] or x[...] where ... is an expression like [], ie x[[]]
KnownOp::IndexStr(JsWord::from(s))
} else {
return;
}
Expand All @@ -158,10 +167,10 @@ impl SimplifyExpr {
}

// 'foo'[1]
KnownOp::Index(idx) if (idx as usize) < value.len() => {
KnownOp::Index(idx) => {
self.changed = true;

if idx < 0 {
if idx.fract() != 0.0 || idx < 0.0 || idx as usize >= value.len() {
*expr = *undefined(*span)
} else {
let value = nth_char(value, idx as _);
Expand All @@ -173,10 +182,19 @@ impl SimplifyExpr {
}))
};
}
_ => {}

// 'foo'['']
KnownOp::IndexStr(_) => {
self.changed = true;
*expr = *undefined(*span);
}
},

// [1, 2, 3].length
//
// [1, 2, 3][0]
//
// [1, 2, 3]['']
Expr::Array(ArrayLit { elems, span }) => {
// do nothing if spread exists
let has_spread = elems.iter().any(|elem| {
Expand Down Expand Up @@ -207,17 +225,25 @@ impl SimplifyExpr {
_ => unreachable!(),
};

if elems.len() > idx as _ && idx >= 0 {
let after_has_side_effect =
elems.iter().skip((idx + 1) as _).any(|elem| match elem {
Some(elem) => elem.expr.may_have_side_effects(&self.expr_ctx),
None => false,
});
// If the fraction part is non-zero, or if the index is out of bounds,
// then the result is always undefined.
if idx.fract() != 0.0 || idx < 0.0 || idx as usize >= elems.len() {
self.changed = true;
*expr = *undefined(*span);
return;
}
// idx is treated as an integer from this point.
//
// We also know for certain the index is not out of bounds.
let idx = idx as i64;

if after_has_side_effect {
return;
}
} else {
let after_has_side_effect =
elems.iter().skip((idx + 1) as _).any(|elem| match elem {
Some(elem) => elem.expr.may_have_side_effects(&self.expr_ctx),
None => false,
});

if after_has_side_effect {
return;
}

Expand Down Expand Up @@ -265,63 +291,72 @@ impl SimplifyExpr {
exprs.push(val);

*expr = Expr::Seq(SeqExpr { span: *span, exprs });
} else if matches!(op, KnownOp::IndexStr(..)) {
self.changed = true;
*expr = *undefined(*span);
return;
}
}

// { foo: true }['foo']
Expr::Object(ObjectLit { props, span }) => match op {
KnownOp::IndexStr(key) if is_literal(props) && key != *"yield" => {
// do nothing if spread exists
let has_spread = props
.iter()
.any(|prop| matches!(prop, PropOrSpread::Spread(..)));
//
// { 0.5: true }[0.5]
Expr::Object(ObjectLit { props, span }) => {
// get key
let key = match op {
KnownOp::Index(i) => Atom::from(i.to_string()),
KnownOp::IndexStr(key) if key != *"yield" && is_literal(props) => key,
_ => return,
};

if has_spread {
return;
}
// do nothing if spread exists
let has_spread = props
.iter()
.any(|prop| matches!(prop, PropOrSpread::Spread(..)));

let idx = props.iter().rev().position(|p| match p {
PropOrSpread::Prop(p) => match &**p {
Prop::Shorthand(i) => i.sym == key,
Prop::KeyValue(k) => prop_name_eq(&k.key, &key),
Prop::Assign(p) => p.key.sym == key,
Prop::Getter(..) => false,
Prop::Setter(..) => false,
// TODO
Prop::Method(..) => false,
},
_ => unreachable!(),
});
let idx = idx.map(|idx| props.len() - 1 - idx);
//
if has_spread {
return;
}

if let Some(i) = idx {
let v = props.remove(i);
self.changed = true;
let idx = props.iter().rev().position(|p| match p {
PropOrSpread::Prop(p) => match &**p {
Prop::Shorthand(i) => i.sym == key,
Prop::KeyValue(k) => prop_name_eq(&k.key, &key),
Prop::Assign(p) => p.key.sym == key,
Prop::Getter(..) => false,
Prop::Setter(..) => false,
// TODO
Prop::Method(..) => false,
},
_ => unreachable!(),
});
let idx = idx.map(|idx| props.len() - 1 - idx);

*expr = self.expr_ctx.preserve_effects(
*span,
match v {
PropOrSpread::Prop(p) => match *p {
Prop::Shorthand(i) => Expr::Ident(i),
Prop::KeyValue(p) => *p.value,
Prop::Assign(p) => *p.value,
Prop::Getter(..) => unreachable!(),
Prop::Setter(..) => unreachable!(),
// TODO
Prop::Method(..) => unreachable!(),
},
_ => unreachable!(),
if let Some(i) = idx {
let v = props.remove(i);
self.changed = true;

*expr = self.expr_ctx.preserve_effects(
*span,
match v {
PropOrSpread::Prop(p) => match *p {
Prop::Shorthand(i) => Expr::Ident(i),
Prop::KeyValue(p) => *p.value,
Prop::Assign(p) => *p.value,
Prop::Getter(..) => unreachable!(),
Prop::Setter(..) => unreachable!(),
// TODO
Prop::Method(..) => unreachable!(),
},
once(Box::new(Expr::Object(ObjectLit {
props: props.take(),
span: *span,
}))),
);
}
_ => unreachable!(),
},
once(Box::new(Expr::Object(ObjectLit {
props: props.take(),
span: *span,
}))),
);
}
_ => {}
},
}

_ => {}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1579,3 +1579,25 @@ fn test_export_default_paren_expr() {
fold_same("import fn from './b'; export default (function fn1 () {});");
fold("export default ((foo));", "export default foo;");
}

#[test]
fn test_issue8747() {
// Indexing a string with an out-of-bounds index returns undefined.
fold("''[0]", "void 0");
// Indexing a string with a non-integer index returns undefined.
fold("'a'[0.5]", "void 0");
// Index with an expression.
fold("''[[]]", "void 0");
fold("'a'[[]]", "void 0");

// Indexing an array has the same logic as indexing a string.
fold("[][0]", "void 0");
fold("[1][0.5]", "void 0");
fold("[][[]]", "void 0");
fold("[1][[]]", "void 0");

// Indexing objects
fold("({0.5: 'a'})[0.5]", "'a';");
fold("({'0.5': 'a'})[0.5]", "'a';");
fold("({0.5: 'a'})['0.5']", "'a';");
}
12 changes: 4 additions & 8 deletions crates/swc_ecma_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1879,13 +1879,9 @@ impl Visit for LiteralVisitor {
match node {
PropName::Str(ref s) => self.cost += 2 + s.value.len(),
PropName::Ident(ref id) => self.cost += 2 + id.sym.len(),
PropName::Num(n) => {
if n.value.fract() < 1e-10 {
// TODO: Count digits
self.cost += 5;
} else {
self.is_lit = false
}
PropName::Num(..) => {
// TODO: Count digits
self.cost += 5;
}
PropName::BigInt(_) => self.is_lit = false,
PropName::Computed(..) => self.is_lit = false,
Expand Down Expand Up @@ -2673,7 +2669,7 @@ pub fn prop_name_eq(p: &PropName, key: &str) -> bool {
match p {
PropName::Ident(i) => i.sym == *key,
PropName::Str(s) => s.value == *key,
PropName::Num(_) => false,
PropName::Num(n) => n.value.to_string() == *key,
PropName::BigInt(_) => false,
PropName::Computed(e) => match &*e.expr {
Expr::Lit(Lit::Str(Str { value, .. })) => *value == *key,
Expand Down