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

levi-nz · 2024-03-15T08:39:44Z

Description:
This PR is a WIP that fixes all known issues in #8747.

BREAKING CHANGE:

Related issue (if exists):
Closes #8747

changeset-bot · 2024-03-15T08:39:49Z

⚠️ No Changeset found

Latest commit: aa42961

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

levi-nz · 2024-03-15T08:49:38Z

Not totally sure if the changed code for array literals is correct -- the code for this is a little more complicated than strings.

[][0] works correctly for now, but not [][[]]. Will look into this later.

levi-nz · 2024-03-16T06:28:08Z

Another broken test case:

var _ = {0.5: 'test'}[0.5];

Currently simplifies to:

var _ = {
    0.5: 'test'
}[0.5];

Should simplify to:

var _ = 'test';

Not entirely sure if this is correct though because removing the ObjectLit could have side effects, ie if a value is a CallExpr

levi-nz · 2024-03-16T07:40:16Z

Only one potential issue so far, that KnownOp no longer implements Eq because of Index now taking f64 instead of i64, but seems OK so far to me.

levi-nz · 2024-03-18T01:52:54Z

@kdy1 The only remaining issue in terms of code changes is this line. Currently is_literal(props) does not work with the unit test below because the 0.5 is not considered a literal. I'm not sure why this fract() check exists and I'm not sure if changing this would break something.

https://github.com/levi-nz/swc/blob/454378d7eaac82bae40c5a770888e84580a1381b/crates/swc_ecma_utils/src/lib.rs#L1883

fold("({0.5: 'a'})['0.5']", "'a';");

levi-nz · 2024-03-18T01:54:12Z

I'm pretty sure Index(f64) can be changed back to Index(i64) as well, as IndexStr is always used for objects.

crates/swc_ecma_utils/src/lib.rs

levi-nz · 2024-03-18T09:24:39Z

I have ran UPDATE=1 cargo test --test projects --test tsc, but not sure if all of these files should be changed, especially crates/swc/tests/tsc-references/templateStringInIndexExpressionES6.2.minified.js, which is now empty.

kdy1

CI failed

levi-nz · 2024-04-08T08:16:08Z

To do:

Move stuff to ctx
Handle known object properties
Possibly add more unit tests

Will comment here again when everything is finalised

crates/swc_ecma_minifier/tests/benches-full/vue.js

levi-nz · 2024-04-10T03:59:56Z

Is there a reason why stuff like [1][0] was replaced with (0, 1) before instead of just 1? The tests seem to pass fine but I want to be sure. I don't see why it should be replaced with a SeqExpr instead of just a Literal if there are no side effects.

levi-nz · 2024-04-10T19:50:22Z

cargo test -p swc_ecma_minifier terser_exec_tests__terser__compress__evaluate__unsafe_string_bad_index__input_js --features concurrent currently fails with a weird infinite loop bug, doesn't seem to be related to optimize_member_expr, need to investigate further

kdy1 · 2024-04-22T17:00:46Z

(0, foo) is about making this undefined

levi-nz · 2024-04-25T15:27:33Z

(0, foo) is about making this undefined

Does this apply to objects too? Like should ({a: 5}).a be replaced with (0, 5) or just 5? Currently it's replaced with the latter but I want to be sure

levi-nz · 2024-04-25T15:52:12Z

Need to handle known object properties in simplify's optimize_member_expr. Will require a function signature change to accept obj and prop instead of expr. This PR should be ready for review tomorrow.

kdy1 · 2024-05-05T11:34:31Z

(0, foo) is about making this undefined

Does this apply to objects too? Like should ({a: 5}).a be replaced with (0, 5) or just 5? Currently it's replaced with the latter but I want to be sure

I don't think so. this only matters for member exprs or direct eval calls

This fixes unit tests being stuck running for 60s+

levi-nz · 2024-05-24T22:00:01Z

Running cargo test run_fixture_test_tests__exec__issues_6xxx__6878__1__exec_js results in this error on both main and issue-8747 branch. Yarn installed with npm i -g yarn and corepack enable. Not sure how to fix?

levi@levi-kde-neon:~/RustroverProjects/levi-nz/swc$ cargo test run_fixture_test_tests__exec__issues_6xxx__6878__1__exec_js
    Finished test [unoptimized + debuginfo] target(s) in 0.39s
     Running unittests src/lib.rs (target/debug/deps/ast_node-e9d29c534642bab4)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/better_scoped_tls-2dc2c5ea5a8d762c)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 2 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/binding_macros-c052f01c3c2f3382)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/main.rs (target/debug/deps/dbg_swc-05912a335a982368)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/from_variant-86e39519e21950c5)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/jsdoc-f39585929aa7c829)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 9 filtered out; finished in 0.00s

     Running tests/fixture.rs (target/debug/deps/fixture-a6c989228dc4b47b)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 235 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/preset_env_base-e8b904940a0c4c1c)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 2 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/string_enum-f4111496889125a4)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/simple.rs (target/debug/deps/simple-07ac1a7eda7c6644)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 2 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/swc-b072fffd3fe75c6a)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 5 filtered out; finished in 0.00s

     Running tests/error_msg.rs (target/debug/deps/error_msg-423e285177753c6d)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 104 filtered out; finished in 0.00s

     Running tests/exec.rs (target/debug/deps/exec-ce806a5ca5bc9fa2)

running 1 test
➤ YN0000: · Yarn 4.0.2
➤ YN0000: ┌ Resolution step
➤ YN0000: └ Completed
➤ YN0000: ┌ Fetch step
➤ YN0000: └ Completed
➤ YN0000: ┌ Link step
➤ YN0000: └ Completed
➤ YN0000: · Done in 0s 194ms
$ dprint fmt
Formatted 193 files.
$ dprint fmt "scripts/*.js" -c scripts/.dprint.json
test run_fixture_test_tests__exec__issues_6xxx__6878__1__exec_js has been running for over 60 seconds.4.2 want: ^5.3.0
npm WARN deprecated @types/terser@3.12.0: This is a stub types definition. terser provides its own type definitions, so you do not need this installed.
npm WARN deprecated inflight@1.0.6: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
npm WARN deprecated @babel/plugin-proposal-class-properties@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-class-properties instead.
npm WARN deprecated @babel/plugin-proposal-object-rest-spread@7.20.7: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-object-rest-spread instead.
npm WARN deprecated core-js@2.6.12: core-js@<3.23.3 is no longer maintained and not recommended for usage due to the number of issues. Because of the V8 engine whims, feature detection in old core-js versions could cause a slowdown up to 100x even if nothing is polyfilled. Some versions have web compatibility issues. Please, upgrade your dependencies to the actual version of core-js.
npm ERR! code 1
npm ERR! path /home/levi/RustroverProjects/levi-nz/swc/node_modules/wasm-sjlj
npm ERR! command failed
npm ERR! command sh -c node-gyp rebuild
npm ERR! gyp info it worked if it ends with ok
npm ERR! gyp info using node-gyp@10.0.1
npm ERR! gyp info using node@20.12.2 | linux | x64
npm ERR! gyp info find Python using Python version 3.10.12 found at "/usr/bin/python3"
npm ERR! gyp info spawn /usr/bin/python3
npm ERR! gyp info spawn args [
npm ERR! gyp info spawn args '/home/levi/.nvm/versions/node/v20.12.2/lib/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py',
npm ERR! gyp info spawn args 'binding.gyp',
npm ERR! gyp info spawn args '-f',
npm ERR! gyp info spawn args 'make',
npm ERR! gyp info spawn args '-I',
npm ERR! gyp info spawn args '/home/levi/RustroverProjects/levi-nz/swc/node_modules/wasm-sjlj/build/config.gypi',
npm ERR! gyp info spawn args '-I',
npm ERR! gyp info spawn args '/home/levi/.nvm/versions/node/v20.12.2/lib/node_modules/npm/node_modules/node-gyp/addon.gypi',
npm ERR! gyp info spawn args '-I',
npm ERR! gyp info spawn args '/home/levi/.cache/node-gyp/20.12.2/include/node/common.gypi',
npm ERR! gyp info spawn args '-Dlibrary=shared_library',
npm ERR! gyp info spawn args '-Dvisibility=default',
npm ERR! gyp info spawn args '-Dnode_root_dir=/home/levi/.cache/node-gyp/20.12.2',
npm ERR! gyp info spawn args '-Dnode_gyp_dir=/home/levi/.nvm/versions/node/v20.12.2/lib/node_modules/npm/node_modules/node-gyp',
npm ERR! gyp info spawn args '-Dnode_lib_file=/home/levi/.cache/node-gyp/20.12.2/<(target_arch)/node.lib',
npm ERR! gyp info spawn args '-Dmodule_root_dir=/home/levi/RustroverProjects/levi-nz/swc/node_modules/wasm-sjlj',
npm ERR! gyp info spawn args '-Dnode_engine=v8',
npm ERR! gyp info spawn args '--depth=.',
npm ERR! gyp info spawn args '--no-parallel',
npm ERR! gyp info spawn args '--generator-output',
npm ERR! gyp info spawn args 'build',
npm ERR! gyp info spawn args '-Goutput_dir=.'
npm ERR! gyp info spawn args ]
npm ERR! gyp: binding.gyp not found (cwd: /home/levi/RustroverProjects/levi-nz/swc/node_modules/wasm-sjlj) while trying to load binding.gyp
npm ERR! gyp ERR! configure error 
npm ERR! gyp ERR! stack Error: `gyp` failed with exit code: 1
npm ERR! gyp ERR! stack at ChildProcess.<anonymous> (/home/levi/.nvm/versions/node/v20.12.2/lib/node_modules/npm/node_modules/node-gyp/lib/configure.js:271:18)
npm ERR! gyp ERR! stack at ChildProcess.emit (node:events:518:28)
npm ERR! gyp ERR! stack at ChildProcess._handle.onexit (node:internal/child_process:294:12)
npm ERR! gyp ERR! System Linux 6.5.0-28-generic
npm ERR! gyp ERR! command "/home/levi/.nvm/versions/node/v20.12.2/bin/node" "/home/levi/.nvm/versions/node/v20.12.2/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
npm ERR! gyp ERR! cwd /home/levi/RustroverProjects/levi-nz/swc/node_modules/wasm-sjlj
npm ERR! gyp ERR! node -v v20.12.2
npm ERR! gyp ERR! node-gyp -v v10.0.1
npm ERR! gyp ERR! not ok

npm ERR! A complete log of this run can be found in: /home/levi/.npm/_logs/2024-05-24T21_56_02_426Z-debug-0.log
test run_fixture_test_tests__exec__issues_6xxx__6878__1__exec_js ... FAILED

failures:

---- run_fixture_test_tests__exec__issues_6xxx__6878__1__exec_js stdout ----
Input: /home/levi/RustroverProjects/levi-nz/swc/crates/swc/tests/exec/issues-6xxx/6878/1/exec.js
thread 'run_fixture_test_tests__exec__issues_6xxx__6878__1__exec_js' panicked at crates/swc/tests/exec.rs:109:13:
assertion failed: status.success()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    run_fixture_test_tests__exec__issues_6xxx__6878__1__exec_js

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 546 filtered out; finished in 108.41s

error: test failed, to rerun pass `-p swc --test exec`

kdy1 · 2024-05-25T00:55:22Z

What's wasm-sjlj? I don't think SWC uses it.

Initial commit: string literals

84ec94e

levi-nz requested a review from a team as a code owner March 15, 2024 08:39

WIP: array literals

7f02ace

levi-nz marked this pull request as draft March 16, 2024 03:28

Correctly handle array indexing

2b94d3d

levi-nz added 3 commits March 17, 2024 14:31

Update unit test

efec60b

Fix broken index logic

72537ac

Fix indexing ObjectLits

454378d

kdy1 self-assigned this Mar 18, 2024

kdy1 added this to the Planned milestone Mar 18, 2024

levi-nz marked this pull request as ready for review March 18, 2024 01:50

levi-nz changed the title ~~(WIP) Make expr_simplifier return undefined when indexing out of bounds~~ Make expr_simplifier handle expressions when indexing strings, arrays and objects Mar 18, 2024

kdy1 reviewed Mar 18, 2024

View reviewed changes

crates/swc_ecma_utils/src/lib.rs Outdated Show resolved Hide resolved

levi-nz added 3 commits March 18, 2024 18:13

Remove fraction check from LiteralVisitor

9b26630

cargo fmt

677a437

Update test files

3aa713e

kdy1 requested changes Mar 20, 2024

View reviewed changes

levi-nz added 6 commits April 1, 2024 05:47

Handle x['0']

8450f8b

Don't return KnownOp::Len if obj is an ObjectLiteral

2b8709f

Handle x['length']

dd4336b

Correctly handle builtin properties

2fe5f09

Don't replace ArrayLiteral if doing so may have side effects

c902739

Update unit test

b4455c9

levi-nz added 3 commits April 8, 2024 17:19

Remove redundant Box::new call

b41617c

Use preserve_effects

a5df0a5

cargo fmt

e47bff3

kdy1 reviewed Apr 9, 2024

View reviewed changes

crates/swc_ecma_minifier/tests/benches-full/vue.js Outdated Show resolved Hide resolved

levi-nz added 7 commits April 10, 2024 11:23

Exclude watch and unwatch

52dfab4

UPDATE=1 cargo test --test projects --test tsc

4a1b6db

UPDATE=1 cargo test -p swc_ecma_minifier --features concurrent

d4696ed

git merge upstream/main issue-8747

7dba5ac

Remove redundant import

00cb04f

Fix test_fold_array_lit_spread_get_elem

6477fbf

Fix test_fold_get_elem1

9b5239d

Add self.changed = true;

f3cfa71

Use ctx

b8cd66b

Fix doc

b2a2cc6

Handle SeqExpr in optimize_member_expr

5458077

levi-nz added 4 commits May 10, 2024 15:07

Remove dup self.changed = true;

5e1819f

git merge upstream/main issue-8747

4a2d3a9

Apply clippy suggestions

45d23d3

Cleanup optimize_member_expr

52869c4

This fixes unit tests being stuck running for 60s+

levi-nz marked this pull request as draft May 12, 2024 11:32

Fix optimization tests

aa42961

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

levi-nz commented Mar 15, 2024 •

edited

changeset-bot bot commented Mar 15, 2024 •

edited

levi-nz commented Mar 15, 2024

levi-nz commented Mar 16, 2024 •

edited

levi-nz commented Mar 16, 2024

levi-nz commented Mar 18, 2024

levi-nz commented Mar 18, 2024

levi-nz commented Mar 18, 2024

kdy1 left a comment

levi-nz commented Apr 8, 2024

levi-nz commented Apr 10, 2024

levi-nz commented Apr 10, 2024

kdy1 commented Apr 22, 2024

levi-nz commented Apr 25, 2024

levi-nz commented Apr 25, 2024

kdy1 commented May 5, 2024

levi-nz commented May 24, 2024

kdy1 commented May 25, 2024

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

Are you sure you want to change the base?

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

Conversation

levi-nz commented Mar 15, 2024 • edited

changeset-bot bot commented Mar 15, 2024 • edited

⚠️ No Changeset found

levi-nz commented Mar 15, 2024

levi-nz commented Mar 16, 2024 • edited

levi-nz commented Mar 16, 2024

levi-nz commented Mar 18, 2024

levi-nz commented Mar 18, 2024

levi-nz commented Mar 18, 2024

kdy1 left a comment

Choose a reason for hiding this comment

levi-nz commented Apr 8, 2024

levi-nz commented Apr 10, 2024

levi-nz commented Apr 10, 2024

kdy1 commented Apr 22, 2024

levi-nz commented Apr 25, 2024

levi-nz commented Apr 25, 2024

kdy1 commented May 5, 2024

levi-nz commented May 24, 2024

kdy1 commented May 25, 2024

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

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

levi-nz commented Mar 15, 2024 •

edited

changeset-bot bot commented Mar 15, 2024 •

edited

levi-nz commented Mar 16, 2024 •

edited