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

Update terser to v5.18.2 and verify optional chaining support #19805

Merged
merged 1 commit into from
Jul 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions test/optimizer/applyImportAndExportNameChanges2-output.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ var imports = {

var ___errno_location, _llvm_bswap_i32, _main, _memcpy, _memset, dynCall_ii, dynCall_iiii;

WebAssembly.instantiate(Module["wasm"], imports).then(output => {
WebAssembly.instantiate(Module["wasm"], imports).then((output => {
var asm = output.instance.exports;
___errno_location = asm["j"];
_llvm_bswap_i32 = asm["k"];
Expand All @@ -275,4 +275,4 @@ WebAssembly.instantiate(Module["wasm"], imports).then(output => {
dynCall_iiii = asm["p"];
initRuntime(asm);
ready();
});
}));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these needed?

Copy link
Collaborator Author

@sbc100 sbc100 Jul 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are not needed, but its seems like acorn is producing them now.. I guess its just a slight behaviour change.

As you can see, no actual code size tests regressed so I guess closure is able just erase these.

If we care we would need to investigate why acorn started doing this.. and see if we can disable it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we care about these braces?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I guess closure fixes this up. Odd terser slightly regressed here but probably this is related to some bugfix for them...

Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ var wasmImports = {
"save2": 2
};

WebAssembly.instantiate(Module["wasm"], imports).then(output => {
WebAssembly.instantiate(Module["wasm"], imports).then((output => {
asm = output.instance.exports;
expD1 = asm["expD1"];
expD2 = asm["expD2"];
expD3 = asm["expD3"];
initRuntime(asm);
ready();
});
}));

expD1;

Expand Down
2 changes: 1 addition & 1 deletion test/other/metadce/test_metadce_minimal_pthreads.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
15050
15052
8 changes: 4 additions & 4 deletions third_party/terser/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Bundled version of terser 4.1.0 with emscripten patches
=======================================================
Bundled version of terser 5.18.2 with emscripten patches
========================================================

We maintain a few downstream patches to terser which means we can't use the
version published in npm.
Expand All @@ -13,5 +13,5 @@ To make changes to this code please submit patches to
https://github.com/emscripten-core/terser/ and then re-create this bundle
using the following steps:

$ npx rollup -c
$ cp dist/bundle.js $EMSCRIPTEN_ROOT/third_party/terser/terser.js
$ CI=1 npx rollup -c
$ cp dist/bundle.min.js $EMSCRIPTEN_ROOT/third_party/terser/terser.js
52,561 changes: 30,951 additions & 21,610 deletions third_party/terser/terser.js

Large diffs are not rendered by default.

28 changes: 10 additions & 18 deletions tools/acorn-optimizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1945,24 +1945,16 @@ function reattachComments(ast, comments) {
symbols[j].start.comments_before = [];
}
symbols[j].start.comments_before.push(
new terser.AST_Token({
end: undefined,
quote: undefined,
raw: undefined,
file: '0',
comments_after: undefined,
comments_before: undefined,
nlb: false,
endpos: undefined,
endcol: undefined,
endline: undefined,
pos: undefined,
col: undefined,
line: undefined,
value: comments[i].value,
type: comments[i].type == 'Line' ? 'comment' : 'comment2',
flags: 0,
})
new terser.AST_Token(
comments[i].type == 'Line' ? 'comment' : 'comment2',
comments[i].value,
undefined,
undefined,
false,
undefined,
undefined,
'0'
)
);
}
}
Expand Down
9 changes: 6 additions & 3 deletions tools/unsafe_optimizations.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ function optPassMergeVarInitializationAssignments(ast) {
}

function runOnJsText(js, pretty = false) {
const ast = acorn.parse(js, {ecmaVersion: 6});
const ast = acorn.parse(js, {ecmaVersion: 2020});

optPassSimplifyModuleInitialization(ast);
optPassRemoveRedundantOperatorNews(ast);
Expand Down Expand Up @@ -255,7 +255,7 @@ let numTestFailures = 0;
function test(input, expected) {
const observed = runOnJsText(input);
if (observed != expected) {
console.error(`Input: ${input}\nobserved: ${observed}\nexpected: ${expected}\n`);
console.error(`ERROR: Input: ${input}\nobserved: ${observed}\nexpected: ${expected}\n`);
++numTestFailures;
} else {
console.log(`OK: ${input} -> ${expected}`);
Expand All @@ -279,7 +279,7 @@ function runTests() {
test("new function(a) {new TextDecoder(a);}('utf8');", '');
test(
'WebAssembly.instantiate(c.wasm,{}).then((a) => {new Int8Array(b);});',
'WebAssembly.instantiate(c.wasm,{}).then(a=>{});'
'WebAssembly.instantiate(c.wasm,{}).then((a=>{}));'
);
test('let x=new Uint16Array(a);', 'let x=new Uint16Array(a);');

Expand All @@ -305,6 +305,9 @@ function runTests() {
// Test that arrays containing nulls don't cause issues
test('[,];', '[,];');

// Test optional chaining operator
test('console?.log("");', 'console?.log("");');

process.exit(numTestFailures);
}

Expand Down