From 1eda5f233a39803b0527649edddb4ca0dbb99c9a Mon Sep 17 00:00:00 2001 From: goktug Date: Thu, 29 Dec 2016 16:29:24 -0800 Subject: [PATCH] Remove incorrect optimization from J2clEqualitySameRewriterPass ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=143215213 --- .../jscomp/J2clEqualitySameRewriterPass.java | 31 ---------- .../J2clEqualitySameRewriterPassTest.java | 59 +++---------------- 2 files changed, 9 insertions(+), 81 deletions(-) diff --git a/src/com/google/javascript/jscomp/J2clEqualitySameRewriterPass.java b/src/com/google/javascript/jscomp/J2clEqualitySameRewriterPass.java index a81c1b0df35..634d8b207ae 100644 --- 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 index ce80de94562..3f581503521 100644 --- 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);")); } }