Resurrect type based optimizations around null check via == for J2CL …

…if the type is known to be an Object type.

Similar optimization was removed earlier since it included === check that differentiates null and undefined which is not safe. We are only resurrecting == and only for J2CL since J2CL does runtime type checking for development that makes this pretty safe and where the most gains are.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=225921492

src/com/google/javascript/jscomp/DefaultPassConfig.java

@@ -1575,10 +1575,11 @@ protected FeatureSet featureSet() {
       new PassFactory("earlyPeepholeOptimizations", true) {
         @Override
         protected CompilerPass create(AbstractCompiler compiler) {
+          boolean useTypesForOptimization = compiler.getOptions().useTypesForLocalOptimization;
           List<AbstractPeepholeOptimization> peepholeOptimizations = new ArrayList<>();
           peepholeOptimizations.add(new PeepholeRemoveDeadCode());
           if (compiler.getOptions().j2clPassMode.shouldAddJ2clPasses()) {
-            peepholeOptimizations.add(new J2clEqualitySameRewriterPass());
+            peepholeOptimizations.add(new J2clEqualitySameRewriterPass(useTypesForOptimization));
           }
           return new PeepholeOptimizationsPass(compiler, getName(), peepholeOptimizations);
         }
@@ -1622,7 +1623,7 @@ private static CompilerPass createPeepholeOptimizationsPass(
     optimizations.add(new PeepholeReplaceKnownMethods(late, useTypesForOptimization));
     optimizations.add(new PeepholeRemoveDeadCode());
     if (compiler.getOptions().j2clPassMode.shouldAddJ2clPasses()) {
-      optimizations.add(new J2clEqualitySameRewriterPass());
+      optimizations.add(new J2clEqualitySameRewriterPass(useTypesForOptimization));
       optimizations.add(new J2clStringValueOfRewriterPass());
     }
     optimizations.add(new PeepholeFoldConstants(late, useTypesForOptimization));

src/com/google/javascript/jscomp/J2clEqualitySameRewriterPass.java

@@ -17,17 +17,17 @@
 
 import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.Node;
+import com.google.javascript.rhino.jstype.JSType;
 
 /** An optimization pass to re-write J2CL Equality.$same. */
 public class J2clEqualitySameRewriterPass extends AbstractPeepholeOptimization {
 
-  /** Whether to use "==" or "===". */
-  private static enum Eq {
-    DOUBLE,
-    TRIPLE
-  }
+  private final boolean useTypes;
+  private boolean shouldRunJ2clPasses;
 
-  private boolean shouldRunJ2clPasses = false;
+  J2clEqualitySameRewriterPass(boolean useTypes) {
+    this.useTypes = useTypes;
+  }
 
   @Override
   void beginTraversal(AbstractCompiler compiler) {
@@ -58,24 +58,48 @@ private Node trySubstituteEqualitySame(Node callNode) {
     Node firstExpr = callNode.getSecondChild();
     Node secondExpr = callNode.getLastChild();
 
-    if (NodeUtil.isNullOrUndefined(firstExpr) || NodeUtil.isNullOrUndefined(secondExpr)) {
-      // At least one side is null or undefined so no coercion danger.
-      return rewriteToEq(firstExpr, secondExpr, Eq.DOUBLE);
+    if (!NodeUtil.isLiteralValue(firstExpr, true) && !NodeUtil.isLiteralValue(secondExpr, true)) {
+      return callNode;
+    }
+
+    if (NodeUtil.isNullOrUndefined(firstExpr)) {
+      // Note that technically 'undefined' might have side effect (see tests) but since at least one
+      // side is literal, we can still safely re-order.
+      return rewriteNullCheck(secondExpr, firstExpr);
     }
 
-    if (NodeUtil.isLiteralValue(firstExpr, true) || NodeUtil.isLiteralValue(secondExpr, true)) {
-      // There is a coercion danger but since at least one side is not null, we can use === that
-      // will not trigger any coercion.
-      return rewriteToEq(firstExpr, secondExpr, Eq.TRIPLE);
+    if (NodeUtil.isNullOrUndefined(secondExpr)) {
+      return rewriteNullCheck(firstExpr, secondExpr);
     }
 
-    return callNode;
+    // There is a coercion danger (e.g. 0 == null) but since at least one side is not null, we can
+    // safely use === that will not trigger any coercion.
+    return rewriteAsStrictEq(firstExpr, secondExpr);
+  }
+
+  private Node rewriteNullCheck(Node expr, Node nullExpression) {
+    expr.detach();
+    nullExpression.detach();
+    if (useTypes && canOnlyBeObject(expr)) {
+      return IR.not(expr);
+    }
+    // At least one side is null or undefined so no coercion danger with ==.
+    return IR.eq(expr, nullExpression);
+  }
+
+  private boolean canOnlyBeObject(Node n) {
+    JSType type = n.getJSType();
+    if (type == null) {
+      return false;
+    }
+    type = type.restrictByNotNullOrUndefined();
+    return !type.isUnknownType() && !type.isEmptyType() && !type.isAllType() && type.isObjectType();
   }
 
-  private Node rewriteToEq(Node firstExpr, Node secondExpr, Eq eq) {
+  private Node rewriteAsStrictEq(Node firstExpr, Node secondExpr) {
     firstExpr.detach();
     secondExpr.detach();
-    return eq == Eq.DOUBLE ? IR.eq(firstExpr, secondExpr) : IR.sheq(firstExpr, secondExpr);
+    return IR.sheq(firstExpr, secondExpr);
   }
 
   private static boolean isEqualitySameCall(Node node) {

test/com/google/javascript/jscomp/J2clEqualitySameRewriterPassTest.java

@@ -24,6 +24,8 @@
 public class J2clEqualitySameRewriterPassTest extends CompilerTestCase {
   private static final String EXTERN = "Equality.$same = function(opt_a, opt_b) {};";
 
+  private static boolean useTypes;
+
   public J2clEqualitySameRewriterPassTest() {
     super(MINIMAL_EXTERNS + EXTERN);
   }
@@ -32,12 +34,14 @@ public J2clEqualitySameRewriterPassTest() {
   @Before
   public void setUp() throws Exception {
     super.setUp();
+    useTypes = false;
     enableTypeCheck();
   }
 
   @Override
   protected CompilerPass getProcessor(final Compiler compiler) {
-    return new PeepholeOptimizationsPass(compiler, getName(), new J2clEqualitySameRewriterPass());
+    return new PeepholeOptimizationsPass(
+        compiler, getName(), new J2clEqualitySameRewriterPass(useTypes));
   }
 
   @Override
@@ -54,7 +58,7 @@ public void testRewriteEqualitySame() {
             "Equality.$same(0, '');",
             "var a = 'ABC';",
             "Equality.$same(a, 'ABC');",
-            "var b = 5;",
+            "var b = {}",
             "Equality.$same(b, 5);",
             "Equality.$same(b, []);",
             "Equality.$same(b, null);",
@@ -63,11 +67,35 @@ public void testRewriteEqualitySame() {
             "0 === '';",
             "var a = 'ABC';",
             "a === 'ABC';",
-            "var b = 5;",
+            "var b = {};",
             "b === 5;",
             "b === [];",
             "b == null;",
-            "null == b;"));
+            "b == null;"));
+  }
+
+  @Test
+  public void testRewriteEqualitySame_useTypes() {
+    useTypes = true;
+    test(
+        lines(
+            "var b = {};",
+            "Equality.$same(b, null);",
+            "Equality.$same(null, b);",
+            "Equality.$same(b, undefined);",
+            "var c = 5;",
+            "Equality.$same(c, null);",
+            "Equality.$same(null, c);",
+            "Equality.$same(c, undefined);"),
+        lines(
+            "var b = {};",
+            "!b",
+            "!b;",
+            "!b;",
+            "var c = 5;",
+            "c == null;", // Note that the semantics are preserved for number.
+            "c == null;",
+            "c == undefined;"));
   }
 
   @Test
@@ -83,11 +111,16 @@ public void testNotRewriteEqualitySame() {
             "var allType = null;",
             "Equality.$same(num, str);",
             "Equality.$same(num, allType);",
-            "Equality.$same(str, allType);"));
+            "Equality.$same(str, allType);",
+            "function hasSideEffects(){};",
+            // Note that the first parameter has value 'undefined' but it has side effects.
+            "Equality.$same(void hasSideEffects(), hasSideEffects());",
+            "Equality.$same(void hasSideEffects(), {a: hasSideEffects()});"));
   }
 
   @Test
-  public void testNotRewriteEqualitySame_sameTypes() {
+  public void testNotRewriteEqualitySame_useTypes() {
+    useTypes = true;
     testSame(
         lines(
             "/** @type {number|undefined} */",