Remove incorrect optimization from J2clEqualitySameRewriterPass

------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=143215213
google · Jan 3, 2017 · 1eda5f2 · 1eda5f2
1 parent 8cea27f
commit 1eda5f2
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 81 deletions.
diff --git a/src/com/google/javascript/jscomp/J2clEqualitySameRewriterPass.java b/src/com/google/javascript/jscomp/J2clEqualitySameRewriterPass.java
@@ -18,7 +18,6 @@
 import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
 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 AbstractPostOrderCallback
@@ -68,19 +67,6 @@ private void trySubstituteEqualitySame(Node callNode) {
       rewriteToEq(callNode, firstExpr, secondExpr, Eq.TRIPLE);
       return;
     }
-
-    // "--use_types_for_optimization" must be on to enable the following type check.
-    if (!compiler.getOptions().useTypesForLocalOptimization) {
-      return;
-    }
-
-    JSType firstType = getTypeRestrictByNotNullOrUndefined(firstExpr);
-    JSType secondType = getTypeRestrictByNotNullOrUndefined(secondExpr);
-    if (isObjectType(firstType) || isObjectType(secondType) || sameType(firstType, secondType)) {
-      // Typeof is same for both so no coersion danger.
-      rewriteToEq(callNode, firstExpr, secondExpr, Eq.DOUBLE);
-      return;
-    }
   }
 
   private void rewriteToEq(Node callNode, Node firstExpr, Node secondExpr, Eq eq) {
@@ -106,21 +92,4 @@ private static boolean isEqualitySameMethodName(Node fnName) {
     String originalQname = fnName.getOriginalQualifiedName();
     return originalQname.endsWith(".$same") && originalQname.contains("Equality");
   }
-
-  private static JSType getTypeRestrictByNotNullOrUndefined(Node node) {
-    JSType jsType = node.getJSType();
-    return jsType == null ? null : jsType.restrictByNotNullOrUndefined();
-  }
-
-  private static boolean isObjectType(JSType jsType) {
-    return !isUnknownType(jsType) && jsType.isObject();
-  }
-
-  private static boolean sameType(JSType jsType1, JSType jsType2) {
-    return !isUnknownType(jsType1) && !isUnknownType(jsType2) && jsType1.equals(jsType2);
-  }
-
-  private static boolean isUnknownType(JSType jsType) {
-    return jsType == null || jsType.isUnknownType() || jsType.isNoType() || jsType.isAllType();
-  }
 }
diff --git a/test/com/google/javascript/jscomp/J2clEqualitySameRewriterPassTest.java b/test/com/google/javascript/jscomp/J2clEqualitySameRewriterPassTest.java
@@ -37,7 +37,6 @@ protected CompilerPass getProcessor(Compiler compiler) {
   @Override
   protected CompilerOptions getOptions() {
     CompilerOptions options = super.getOptions();
-    options.useTypesForLocalOptimization = useTypesForOptimization;
     options.setJ2clPass(CompilerOptions.J2clPassMode.ON);
     return options;
   }
@@ -52,24 +51,7 @@ public void testRewriteEqualitySame() {
             "Equality.$same(b, 5);",
             "Equality.$same(b, []);",
             "Equality.$same(b, null);",
-            "Equality.$same(null, b);",
-            "/** @type {number|undefined} */",
-            "var num1 = 5;",
-            "/** @type {?number} */",
-            "var num2 = 5;",
-            "Equality.$same(num1, num2);",
-            "/** @type {string} */",
-            "var str1 = '';",
-            "/** @type {string|undefined} */",
-            "var str2 = 'abc';",
-            "Equality.$same(str1, str2);",
-            "/** @type {!Object} */",
-            "var obj1 = {};",
-            "/** @type {Object} */",
-            "var obj2 = null;",
-            "Equality.$same(obj1, obj2);",
-            "Equality.$same(obj1, str2);",
-            "Equality.$same(obj1, num2);"),
+            "Equality.$same(null, b);"),
         LINE_JOINER.join(
             "0 === '';",
             "var a = 'ABC';",
@@ -78,24 +60,7 @@ public void testRewriteEqualitySame() {
             "b === 5;",
             "b === [];",
             "b == null;",
-            "null == b;",
-            "/** @type {number|undefined} */",
-            "var num1 = 5;",
-            "/** @type {?number} */",
-            "var num2 = 5;",
-            "num1 == num2;",
-            "/** @type {string} */",
-            "var str1 = '';",
-            "/** @type {string|undefined} */",
-            "var str2 = 'abc';",
-            "str1 == str2;",
-            "/** @type {!Object} */",
-            "var obj1 = {};",
-            "/** @type {Object} */",
-            "var obj2 = null;",
-            "obj1 == obj2;",
-            "obj1 == str2;",
-            "obj1 == num2;"));
+            "null == b;"));
   }
 
   public void testNotRewriteEqualitySame() {
@@ -113,18 +78,7 @@ public void testNotRewriteEqualitySame() {
             "Equality.$same(str, allType);"));
   }
 
-  public void testNotRewriteEqualitySame_allType() {
-    testSame(
-        LINE_JOINER.join(
-            "/** @type {*} */",
-            "var allType1 = 1;",
-            "/** @type {*} */",
-            "var allType2 = '1';",
-            "Equality.$same(allType1, allType2);"));
-  }
-
-  public void testNotRewriteEqualitySame_noTypeCheck() {
-    useTypesForOptimization = false;
+  public void testNotRewriteEqualitySame_sameTypes() {
     testSame(
         LINE_JOINER.join(
             "/** @type {number|undefined} */",
@@ -143,6 +97,11 @@ public void testNotRewriteEqualitySame_noTypeCheck() {
             "var obj2 = null;",
             "Equality.$same(obj1, obj2);",
             "Equality.$same(obj1, str2);",
-            "Equality.$same(obj1, num2);"));
+            "Equality.$same(obj1, num2);",
+            "/** @type {*} */",
+            "var allType1 = 1;",
+            "/** @type {*} */",
+            "var allType2 = '1';",
+            "Equality.$same(allType1, allType2);"));
   }
 }