Minor acorn-optimizer simplifications. NFC (#24348)

- Remove unused argument from `dump`.
 - Use coalescing operator where possible.
- Remove usage of the `Node` class from acorn - it's not necessary as
ESTree tools only care about plain JS properties and don't care about
subclassing.
- Remove optional no-op properties in created nodes like `raw`, `start`,
`end`.
- Remove unnecessary checks for both `!memberExpr.computed` and
`memberExpr.property.type === 'Identifier'` - the former implies the
latter.
 - Simplify directive check for 'use strict' and the like.
 - Use native Node APIs instead of helpers where possible.
 - Use shorter ES6+ syntax where possible.
 - Unify `assertAt` errors format with parsing errors.
- Target a narrower `VariableDeclarator` instead of
`VariableDeclaration` where we don't care about the parent.

Author: RReverser

test/test_other.py

@@ -10497,8 +10497,7 @@ def test_js_optimizer_parse_error(self):
     stderr = self.expect_fail([EMCC, 'src.c', '-O2'] + self.get_emcc_args())
     self.assertContained(('''
 function js() { var x = !<->5.; }
-                         ^
-'''), stderr)
+                         ^ '''), stderr)
 
   @crossplatform
   def test_js_optimizer_chunk_size_determinism(self):

tools/acorn-optimizer.mjs

@@ -3,29 +3,22 @@
 import * as acorn from 'acorn';
 import * as terser from '../third_party/terser/terser.js';
 import * as fs from 'node:fs';
+import assert from 'node:assert';
 
 // Utilities
 
-function print(x) {
-  process.stdout.write(x + '\n');
-}
-
 function read(x) {
-  return fs.readFileSync(x).toString();
-}
-
-function assert(condition, text) {
-  if (!condition) {
-    throw new Error(text);
-  }
+  return fs.readFileSync(x, 'utf-8');
 }
 
 function assertAt(condition, node, message = '') {
   if (!condition) {
-    const loc = acorn.getLineInfo(input, node.start);
-    throw new Error(
-      `${infile}:${loc.line}: ${message} (use EMCC_DEBUG_SAVE=1 to preserve temporary inputs)`,
-    );
+    if (!process.env.EMCC_DEBUG_SAVE) {
+      message += ' (use EMCC_DEBUG_SAVE=1 to preserve temporary inputs)';
+    }
+    let err = new Error(message);
+    err['loc'] = acorn.getLineInfo(input, node.start);
+    throw err;
   }
 }
 
@@ -39,7 +32,7 @@ function visitChildren(node, c) {
     return;
   }
   function maybeChild(child) {
-    if (child && typeof child === 'object' && typeof child.type === 'string') {
+    if (typeof child?.type === 'string') {
       c(child);
       return true;
     }
@@ -93,22 +86,17 @@ function emptyOut(node) {
   node.type = 'EmptyStatement';
 }
 
-function isUseStrict(node) {
-  return node.type === 'Literal' && node.value === 'use strict';
-}
-
 function setLiteralValue(item, value) {
   item.value = value;
-  item.raw = "'" + value + "'";
+  item.raw = null;
 }
 
 function isLiteralString(node) {
-  return node.type === 'Literal' && (node.raw[0] === '"' || node.raw[0] === "'");
+  return node.type === 'Literal' && typeof node.value === 'string';
 }
 
-function dump(node, text) {
-  if (text) print(text);
-  print(JSON.stringify(node, null, ' '));
+function dump(node) {
+  console.log(JSON.stringify(node, null, ' '));
 }
 
 // Mark inner scopes temporarily as empty statements. Returns
@@ -117,7 +105,7 @@ function ignoreInnerScopes(node) {
   const map = new WeakMap();
   function ignore(node) {
     map.set(node, node.type);
-    node.type = 'EmptyStatement';
+    emptyOut(node);
   }
   simpleWalk(node, {
     FunctionDeclaration(node) {
@@ -151,13 +139,17 @@ function hasSideEffects(node) {
   let has = false;
   fullWalk(node, (node) => {
     switch (node.type) {
+      case 'ExpressionStatement':
+        if (node.directive) {
+          has = true;
+        }
+        break;
       // TODO: go through all the ESTree spec
       case 'Literal':
       case 'Identifier':
       case 'UnaryExpression':
       case 'BinaryExpression':
       case 'LogicalExpression':
-      case 'ExpressionStatement':
       case 'UpdateOperator':
       case 'ConditionalExpression':
       case 'FunctionDeclaration':
@@ -284,7 +276,7 @@ function JSDCE(ast, aggressive) {
           }
         },
         ExpressionStatement(node, _c) {
-          if (aggressive && !hasSideEffects(node) && !isUseStrict(node.expression)) {
+          if (aggressive && !hasSideEffects(node)) {
             emptyOut(node);
             removed++;
           }
@@ -954,7 +946,7 @@ function emitDCEGraph(ast) {
     info.reaches = sortedNamesFromMap(info.reaches);
     graph.push(info);
   });
-  print(JSON.stringify(graph, null, ' '));
+  dump(graph);
 }
 
 // Apply graph removals from running wasm-metadce. This only removes imports and
@@ -1042,31 +1034,21 @@ function applyDCEGraphRemovals(ast) {
   }
 }
 
-// Need a parser to pass to acorn.Node constructor.
-// Create it once and reuse it.
-const stubParser = new acorn.Parser({ecmaVersion: 2021});
-
-function createNode(props) {
-  const node = new acorn.Node(stubParser);
-  Object.assign(node, props);
-  return node;
-}
-
 function createLiteral(value) {
-  return createNode({
+  return {
     type: 'Literal',
     value: value,
     raw: '' + value,
-  });
+  };
 }
 
 function makeCallExpression(node, name, args) {
   Object.assign(node, {
     type: 'CallExpression',
-    callee: createNode({
+    callee: {
       type: 'Identifier',
       name: name,
-    }),
+    },
     arguments: args,
   });
 }
@@ -1095,7 +1077,7 @@ function isEmscriptenHEAP(name) {
 // LE byte order for HEAP buffer
 function littleEndianHeap(ast) {
   recursiveWalk(ast, {
-    FunctionDeclaration: (node, c) => {
+    FunctionDeclaration(node, c) {
       // do not recurse into LE_HEAP_STORE, LE_HEAP_LOAD functions
       if (
         !(
@@ -1106,13 +1088,13 @@ function littleEndianHeap(ast) {
         c(node.body);
       }
     },
-    VariableDeclarator: (node, c) => {
+    VariableDeclarator(node, c) {
       if (!(node.id.type === 'Identifier' && node.id.name.startsWith('LE_ATOMICS_'))) {
         c(node.id);
         if (node.init) c(node.init);
       }
     },
-    AssignmentExpression: (node, c) => {
+    AssignmentExpression(node, c) {
       const target = node.left;
       const value = node.right;
       c(value);
@@ -1172,7 +1154,7 @@ function littleEndianHeap(ast) {
         }
       }
     },
-    CallExpression: (node, c) => {
+    CallExpression(node, c) {
       if (node.arguments) {
         for (var a of node.arguments) c(a);
       }
@@ -1181,7 +1163,6 @@ function littleEndianHeap(ast) {
         node.callee.type === 'MemberExpression' &&
         node.callee.object.type === 'Identifier' &&
         node.callee.object.name === 'Atomics' &&
-        node.callee.property.type === 'Identifier' &&
         !node.callee.computed
       ) {
         makeCallExpression(
@@ -1193,7 +1174,7 @@ function littleEndianHeap(ast) {
         c(node.callee);
       }
     },
-    MemberExpression: (node, c) => {
+    MemberExpression(node, c) {
       c(node.property);
       if (!isHEAPAccess(node)) {
         // not accessing the HEAP
@@ -1269,39 +1250,37 @@ function growableHeap(ast) {
         c(node.body);
       }
     },
-    AssignmentExpression: (node) => {
+    AssignmentExpression(node) {
       if (node.left.type !== 'Identifier') {
         // Don't transform `HEAPxx =` assignments.
         growableHeap(node.left);
       }
       growableHeap(node.right);
     },
-    VariableDeclaration: (node) => {
+    VariableDeclarator(node) {
       // Don't transform the var declarations for HEAP8 etc
-      node.declarations.forEach((decl) => {
-        // but do transform anything that sets a var to
-        // something from HEAP8 etc
-        if (decl.init) {
-          growableHeap(decl.init);
-        }
-      });
+      // but do transform anything that sets a var to
+      // something from HEAP8 etc
+      if (node.init) {
+        growableHeap(node.init);
+      }
     },
-    Identifier: (node) => {
+    Identifier(node) {
       if (isEmscriptenHEAP(node.name)) {
         // Transform `HEAPxx` into `(growMemViews(), HEAPxx)`.
         // Important: don't just do `growMemViews(HEAPxx)` because `growMemViews` reassigns `HEAPxx`
         // and we want to get an updated value after that reassignment.
         Object.assign(node, {
           type: 'SequenceExpression',
           expressions: [
-            createNode({
+            {
               type: 'CallExpression',
-              callee: createNode({
+              callee: {
                 type: 'Identifier',
                 name: 'growMemViews',
-              }),
+              },
               arguments: [],
-            }),
+            },
             {...node},
           ],
         });
@@ -1337,12 +1316,7 @@ function unsignPointers(ast) {
       right: {
         type: 'Literal',
         value: 0,
-        raw: '0',
-        start: 0,
-        end: 0,
       },
-      start: 0,
-      end: 0,
     };
   }
 
@@ -1357,7 +1331,6 @@ function unsignPointers(ast) {
         node.callee.type === 'MemberExpression' &&
         node.callee.object.type === 'Identifier' &&
         isHeap(node.callee.object.name) &&
-        node.callee.property.type === 'Identifier' &&
         !node.callee.computed
       ) {
         // This is a call on HEAP*.?. Specific things we need to fix up are
@@ -1430,12 +1403,12 @@ function asanify(ast) {
 }
 
 function multiply(value, by) {
-  return createNode({
+  return {
     type: 'BinaryExpression',
     left: value,
     operator: '*',
     right: createLiteral(by),
-  });
+  };
 }
 
 // Replace direct heap access with SAFE_HEAP* calls.
@@ -1523,7 +1496,7 @@ function ensureMinifiedNames(n) {
 
 function minifyLocals(ast) {
   // We are given a mapping of global names to their minified forms.
-  assert(extraInfo && extraInfo.globals);
+  assert(extraInfo?.globals);
 
   for (const fun of ast.body) {
     if (fun.type !== 'FunctionDeclaration') {
@@ -1787,7 +1760,7 @@ function reattachComments(ast, commentsMap) {
   // Collect all code symbols
   ast.walk(
     new terser.TreeWalker((node) => {
-      if (node.start && node.start.pos) {
+      if (node.start?.pos) {
         symbols.push(node);
       }
     }),
@@ -1853,11 +1826,6 @@ function trace(...args) {
   }
 }
 
-function error(...args) {
-  console.error(...args);
-  throw new Error(...args);
-}
-
 // If enabled, output retains parentheses and comments so that the
 // output can further be passed out to Closure.
 const closureFriendly = getArg('--closure-friendly');
@@ -1894,29 +1862,14 @@ if (closureFriendly) {
   const currentComments = [];
   Object.assign(params, {
     preserveParens: true,
-    onToken: (token) => {
+    onToken(token) {
       // Associate comments with the start position of the next token.
       sourceComments[token.start] = currentComments.slice();
       currentComments.length = 0;
     },
     onComment: currentComments,
   });
 }
-let ast;
-try {
-  ast = acorn.parse(input, params);
-} catch (err) {
-  err.message += (() => {
-    let errorMessage = '\n' + input.split(acorn.lineBreak)[err.loc.line - 1] + '\n';
-    let column = err.loc.column;
-    while (column--) {
-      errorMessage += ' ';
-    }
-    errorMessage += '^\n';
-    return errorMessage;
-  })();
-  throw err;
-}
 
 const registry = {
   JSDCE,
@@ -1934,13 +1887,23 @@ const registry = {
   minifyGlobals,
 };
 
-passes.forEach((pass) => {
-  trace(`running AST pass: ${pass}`);
-  if (!(pass in registry)) {
-    error(`unknown optimizer pass: ${pass}`);
+let ast;
+try {
+  ast = acorn.parse(input, params);
+  for (let pass of passes) {
+    const resolvedPass = registry[pass];
+    assert(resolvedPass, `unknown optimizer pass: ${pass}`);
+    resolvedPass(ast);
+  }
+} catch (err) {
+  if (err.loc) {
+    err.message +=
+      '\n' +
+      `${input.split(acorn.lineBreak)[err.loc.line - 1]}\n` +
+      `${' '.repeat(err.loc.column)}^ ${infile}:${err.loc.line}:${err.loc.column + 1}`;
   }
-  registry[pass](ast);
-});
+  throw err;
+}
 
 if (!noPrint) {
   const terserAst = terser.AST_Node.from_mozilla_ast(ast);