feat(minifier): constant addition expression folding by DonIsaac · Pull Request #882 · oxc-project/oxc

DonIsaac · 2023-09-09T23:50:45Z

Fold constant addition expressions. Handles string concatenation and addition, both with implicit casting.

For example,

let x = 1 + 1
let y = "hello " + "world"

now becomes

let x = 2
let y = "hello world"

Extra Goodies

test(minifier): add test_snapshot helper to perform snapshot tests with insta
up(hir): implement std::ops::Add for NumericValue
up(span): impl TryFrom<Cow<'_, &str>> for Atom

codspeed-hq · 2023-09-10T00:05:35Z

CodSpeed Performance Report

Merging #882 will not alter performance

_{Comparing don/feat/minifier/const-add-folding (75a9b3b) with main (a9d36f1)}

Summary

✅ 18 untouched benchmarks

DonIsaac · 2023-09-10T01:26:26Z

This PR causes several Test262 benchmarks to fail since messages in Test262Error are getting concatinated. The minifier is just checked by minifying twice, then comparing the output. Not sure what to do about this one.

Example:

expected:
if(isNaN(Number.NaN+1)!==!0)throw new Test262Error('#1: NaN + 1 === Not-a-Number. Actual: '+NaN+1);if(isNaN(1+Number.NaN)!==!0)throw new Test262Error('#2: 1 + NaN === Not-a-Number. Actual: '+1+NaN);if(isNaN(Number.NaN+Number.POSITIVE_INFINITY)!==!0)throw new Test262Error('#3: NaN + Infinity === Not-a-Number. Actual: '+NaN+Infinity);if(isNaN(Number.POSITIVE_INFINITY+Number.NaN)!==!0)throw new Test262Error('#4: Infinity + NaN === Not-a-Number. Actual: '+Infinity+NaN);if(isNaN(Number.NaN+Number.NEGATIVE_INFINITY)!==!0)throw new Test262Error('#5: NaN + Infinity === Not-a-Number. Actual: '+NaN+Infinity);if(isNaN(Number.NEGATIVE_INFINITY+Number.NaN)!==!0)throw new Test262Error('#6: Infinity + NaN === Not-a-Number. Actual: '+Infinity+NaN)

actual:
if(isNaN(Number.NaN+1)!==!0)throw new Test262Error('#1: NaN + 1 === Not-a-Number. Actual: NaN1');if(isNaN(1+Number.NaN)!==!0)throw new Test262Error('#2: 1 + NaN === Not-a-Number. Actual: 1NaN');if(isNaN(Number.NaN+Number.POSITIVE_INFINITY)!==!0)throw new Test262Error('#3: NaN + Infinity === Not-a-Number. Actual: NaNInfinity');if(isNaN(Number.POSITIVE_INFINITY+Number.NaN)!==!0)throw new Test262Error('#4: Infinity + NaN === Not-a-Number. Actual: InfinityNaN');if(isNaN(Number.NaN+Number.NEGATIVE_INFINITY)!==!0)throw new Test262Error('#5: NaN + Infinity === Not-a-Number. Actual: NaNInfinity');if(isNaN(Number.NEGATIVE_INFINITY+Number.NaN)!==!0)throw new Test262Error('#6: Infinity + NaN === Not-a-Number. Actual: InfinityNaN')

When both outputs are run through prettier, I get

// expected
if (isNaN(Number.NaN + 1) !== !0)
    throw new Test262Error('#1: NaN + 1 === Not-a-Number. Actual: ' + NaN + 1)
if (isNaN(1 + Number.NaN) !== !0)
    throw new Test262Error('#2: 1 + NaN === Not-a-Number. Actual: ' + 1 + NaN)
if (isNaN(Number.NaN + Number.POSITIVE_INFINITY) !== !0)
    throw new Test262Error(
        '#3: NaN + Infinity === Not-a-Number. Actual: ' + NaN + Infinity
    )
if (isNaN(Number.POSITIVE_INFINITY + Number.NaN) !== !0)
    throw new Test262Error(
        '#4: Infinity + NaN === Not-a-Number. Actual: ' + Infinity + NaN
    )
if (isNaN(Number.NaN + Number.NEGATIVE_INFINITY) !== !0)
    throw new Test262Error(
        '#5: NaN + Infinity === Not-a-Number. Actual: ' + NaN + Infinity
    )
if (isNaN(Number.NEGATIVE_INFINITY + Number.NaN) !== !0)
    throw new Test262Error(
        '#6: Infinity + NaN === Not-a-Number. Actual: ' + Infinity + NaN
    )

// actual
if (isNaN(Number.NaN + 1) !== !0)
    throw new Test262Error('#1: NaN + 1 === Not-a-Number. Actual: NaN1')
if (isNaN(1 + Number.NaN) !== !0)
    throw new Test262Error('#2: 1 + NaN === Not-a-Number. Actual: 1NaN')
if (isNaN(Number.NaN + Number.POSITIVE_INFINITY) !== !0)
    throw new Test262Error(
        '#3: NaN + Infinity === Not-a-Number. Actual: NaNInfinity'
    )
if (isNaN(Number.POSITIVE_INFINITY + Number.NaN) !== !0)
    throw new Test262Error(
        '#4: Infinity + NaN === Not-a-Number. Actual: InfinityNaN'
    )
if (isNaN(Number.NaN + Number.NEGATIVE_INFINITY) !== !0)
    throw new Test262Error(
        '#5: NaN + Infinity === Not-a-Number. Actual: NaNInfinity'
    )
if (isNaN(Number.NEGATIVE_INFINITY + Number.NaN) !== !0)
    throw new Test262Error(
        '#6: Infinity + NaN === Not-a-Number. Actual: InfinityNaN'
    )

This is, however, expected behavior.

Boshen · 2023-09-10T02:27:27Z

Terser has the evaluate option for the compressor https://github.com/terser/terser#compress-options

To get this PR merged, we can add this option and turn if off for the conformance test for now.

We can figure out how to make the algorithm pass the test, or change the test later.

Boshen

The code itself is good.

I'm not sure about adding snapshot testing because this deviates from our current testing strategy, which will cause future contributors confused on how to test the minifier. But I can fix this later.

The only blocker is adding the evaluate option to get the conformance test passing.

Boshen · 2023-09-10T04:28:15Z

The test is still failing 🤔

crates/oxc_minifier/src/compressor/fold.rs

mysteryven

Just two nitpicks.

crates/oxc_minifier/src/compressor/fold.rs

…t/no-redeclare * 'feat/no-redeclare' of github.com:eryue0220/oxc: (22 commits) deps: remove default-features from codspeed-criterion-compat chore(deps): bump the dependencies group with 4 updates (oxc-project#893) chore(deps): bump actions/checkout from 3 to 4 (oxc-project#894) feat(minifier): constant addition expression folding (oxc-project#882) chore(benchmark): turn on all lints (oxc-project#892) feat(linter): eslint-plugin-import(no-cycle) (oxc-project#890) chore: fix typo (oxc-project#889) perf(linter): early bail out if not jest fn (oxc-project#885) feat(linter): add typescript/no-explicit-any (oxc-project#881) feat(website): Hide error panel when query view is shown (oxc-project#884) fix(website): fix run_query call arguments (oxc-project#880) feat(linter): eslint-plugin-import/no-self-import (oxc-project#878) feat(linter): implement re-exports (oxc-project#877) ci(benchmark): install toolchain first refactor(resolver): clean up `load_alias` (oxc-project#875) fix(parser): parse [+In] in object binding initializer (oxc-project#874) chore(fuzz): add --sanitizer none command chore(fuzz): add nightly instructions refactor: clean up fuzzer, move it to repo root (oxc-project#872) perf(resolver): avoid double hashing by memoizing the hash (oxc-project#871) ...

feat(minifier): constant addition expression folding

ac59a67

DonIsaac added the A-minifier Area - Minifier label Sep 9, 2023

style: cargo fmt

05dd32d

DonIsaac added 2 commits September 9, 2023 21:36

test(minifier): more folding tests

cf5ff4e

test(minifier): update fold snapshot

db748e2

Boshen requested a review from mysteryven September 10, 2023 02:29

Boshen requested changes Sep 10, 2023

View reviewed changes

DonIsaac added 3 commits September 9, 2023 23:33

fix(minifier): support compress.evaluate option

a75cd4b

Merge branch 'main' into don/feat/minifier/const-add-folding

3f52c0c

style: cargo fmt

f4b5cba

mysteryven reviewed Sep 10, 2023

View reviewed changes

crates/oxc_minifier/src/compressor/fold.rs Outdated Show resolved Hide resolved

mysteryven reviewed Sep 10, 2023

View reviewed changes

crates/oxc_minifier/src/compressor/fold.rs Show resolved Hide resolved

DonIsaac added 4 commits September 10, 2023 11:59

Merge branch 'main' into don/feat/minifier/const-add-folding

464bbd9

fix(minifier): int handling in constant add folding

e1b3352

test(minifier): set compress.evaluate to false for test262 benchmarks

6965d86

style(coverage): run cargo fmt

75a9b3b

DonIsaac enabled auto-merge (squash) September 10, 2023 21:32

Boshen approved these changes Sep 11, 2023

View reviewed changes

DonIsaac merged commit 027a67d into main Sep 11, 2023

DonIsaac deleted the don/feat/minifier/const-add-folding branch September 11, 2023 02:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(minifier): constant addition expression folding#882

feat(minifier): constant addition expression folding#882
DonIsaac merged 11 commits intomainfrom
don/feat/minifier/const-add-folding

DonIsaac commented Sep 9, 2023 •

edited

Loading

Uh oh!

codspeed-hq bot commented Sep 10, 2023 •

edited

Loading

Uh oh!

DonIsaac commented Sep 10, 2023 •

edited

Loading

Uh oh!

Boshen commented Sep 10, 2023

Uh oh!

Boshen left a comment

Uh oh!

Boshen commented Sep 10, 2023

Uh oh!

Uh oh!

mysteryven left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

DonIsaac commented Sep 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Extra Goodies

Uh oh!

codspeed-hq bot commented Sep 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #882 will not alter performance

Summary

Uh oh!

DonIsaac commented Sep 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Example:

Uh oh!

Boshen commented Sep 10, 2023

Uh oh!

Boshen left a comment

Choose a reason for hiding this comment

Uh oh!

Boshen commented Sep 10, 2023

Uh oh!

Uh oh!

mysteryven left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

DonIsaac commented Sep 9, 2023 •

edited

Loading

codspeed-hq bot commented Sep 10, 2023 •

edited

Loading

DonIsaac commented Sep 10, 2023 •

edited

Loading