google / closure-compiler

A JavaScript checker and optimizer.
https://developers.google.com/closure/compiler/
Apache License 2.0
7.41k stars 1.15k forks source link

Compiler incorrectly inlines temporary variable with optional chaining #4187

Closed tommywilkinson closed 3 months ago

tommywilkinson commented 3 months ago

Interesting issue where the presence of an optional chaining syntax appears to confuse inlining logic. Verified behavior in advanced mode as of version 20210907.0.0

repro input:

function swap(node) {
  const y = node.right;
  const t2 = y?.left;

  y.left = node;
  node.right = t2;

  return node;
}

// Export to make sure we don't get eliminated
module.exports = { swap };

problematic output:

'use strict';module.exports={g:function(a){const b=a.right;b.left=a;a.right=b?.left;return a}};

Notice here that the temporary "t2" variable here has been inlined, resulting in the sequence b.left=a; a.right=b?.left;, which does not correctly represent the original function's behavior. Removing the ?. optional chaining operator and having const t2 = y.left will correct the problem.

tommywilkinson commented 3 months ago

Also confirmed the behavior is still broken as of latest npm release (20240317)

Output looks slightly different, but same problem:

module.exports={g:function(a){const b=a.right;b.left=a;a.right=b?.left;return a}};
rahul-kamat commented 3 months ago

Thanks for catching this Tommy! Here's the fix I submitted this morning: https://github.com/google/closure-compiler/commit/9adfeb5f85e80fc32af74156668b5b2083ea22a7