Add an options.assumeAccurateNullUndefinedTypes to gate a local type-…

…based optimization Before, PeepholeMinimizeConditions is folding "if (x === null)" into "if (x)" if it thinks "x" can't be undefined from its type. This is not always sound. For example: /** @type {!Object<string, string>} */ var map = {}; var x = map['foo']; // x can be undefined at runtime, but the compiler can't prove it if (x === null) { // Code relying on the difference between null and undefined can break } ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=141925759
google · Dec 14, 2016 · 14b6188 · 14b6188
1 parent ee6f16f
commit 14b6188
Show file tree

Hide file tree

Showing 10 changed files with 51 additions and 15 deletions.
diff --git a/src/com/google/javascript/jscomp/CommandLineRunner.java b/src/com/google/javascript/jscomp/CommandLineRunner.java
@@ -410,6 +410,12 @@ private static class Flags {
         + "may result in incorrect results.")
     private boolean useTypesForOptimization = true;
 
+    @Option(name = "--assume_accurate_null_undefined_types",
+        handler = BooleanOptionHandler.class,
+        usage = "Enables optimizations which rely on accurate distinction between null and "
+        + "undefined types.")
+    private boolean assumeAccurateNullUndefinedTypes = false;
+
     @Option(name = "--assume_function_wrapper",
         handler = BooleanOptionHandler.class,
         usage = "Enable additional optimizations based on the assumption that the output will be "
@@ -1562,6 +1568,8 @@ protected CompilerOptions createOptions() {
       level.setTypeBasedOptimizationOptions(options);
     }
 
+    options.setAssumeAccurateNullUndefinedTypes(flags.assumeAccurateNullUndefinedTypes);
+
     if (flags.assumeFunctionWrapper) {
       level.setWrappedOutputOptimizations(options);
     }

diff --git a/src/com/google/javascript/jscomp/Compiler.java b/src/com/google/javascript/jscomp/Compiler.java
@@ -337,6 +337,7 @@ public void initOptions(CompilerOptions options) {
       options.setInlineProperties(false);
       options.setUseTypesForLocalOptimization(false);
       options.setUseTypesForOptimization(false);
+      options.setAssumeAccurateNullUndefinedTypes(false);
     }
 
     if (options.legacyCodeCompile) {

diff --git a/src/com/google/javascript/jscomp/CompilerOptions.java b/src/com/google/javascript/jscomp/CompilerOptions.java
@@ -525,6 +525,12 @@ public enum ExtractPrototypeMemberDeclarationsMode {
   /** Use type information to enable additional optimization opportunities. */
   boolean useTypesForLocalOptimization;
 
+  /**
+   * Assume accurate distinction between null and undefined to enable additional optimization
+   * opportunities.
+   */
+  boolean assumeAccurateNullUndefinedTypes;
+
   //--------------------------------
   // Renaming
   //--------------------------------
@@ -2188,6 +2194,10 @@ public void setUseTypesForLocalOptimization(boolean useTypesForLocalOptimization
     this.useTypesForLocalOptimization = useTypesForLocalOptimization;
   }
 
+  public void setAssumeAccurateNullUndefinedTypes(boolean assumeAccurateNullUndefinedTypes) {
+    this.assumeAccurateNullUndefinedTypes = assumeAccurateNullUndefinedTypes;
+  }
+
   @Deprecated
   public void setUseTypesForOptimization(boolean useTypesForOptimization) {
     if (useTypesForOptimization) {

diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java
@@ -1543,10 +1543,12 @@ protected CompilerPass create(AbstractCompiler compiler) {
   /** Various peephole optimizations. */
   private static CompilerPass createPeepholeOptimizationsPass(AbstractCompiler compiler) {
     final boolean late = false;
-    final boolean useTypesForOptimization = compiler.getOptions().useTypesForLocalOptimization;
+    CompilerOptions options = compiler.getOptions();
+    final boolean useTypesForOptimization = options.useTypesForLocalOptimization;
     return new PeepholeOptimizationsPass(compiler,
           new MinimizeExitPoints(compiler),
-          new PeepholeMinimizeConditions(late, useTypesForOptimization),
+          new PeepholeMinimizeConditions(
+              late, useTypesForOptimization, options.assumeAccurateNullUndefinedTypes),
           new PeepholeSubstituteAlternateSyntax(late),
           new PeepholeReplaceKnownMethods(late, useTypesForOptimization),
           new PeepholeRemoveDeadCode(),
@@ -1582,7 +1584,8 @@ protected CompilerPass create(AbstractCompiler compiler) {
       return new PeepholeOptimizationsPass(compiler,
             new StatementFusion(options.aggressiveFusion),
             new PeepholeRemoveDeadCode(),
-            new PeepholeMinimizeConditions(late, useTypesForOptimization),
+            new PeepholeMinimizeConditions(
+                late, useTypesForOptimization, options.assumeAccurateNullUndefinedTypes),
             new PeepholeSubstituteAlternateSyntax(late),
             new PeepholeReplaceKnownMethods(late, useTypesForOptimization),
             new PeepholeFoldConstants(late, useTypesForOptimization),

diff --git a/src/com/google/javascript/jscomp/ExpandJqueryAliases.java b/src/com/google/javascript/jscomp/ExpandJqueryAliases.java
@@ -83,9 +83,11 @@ class ExpandJqueryAliases extends AbstractPostOrderCallback
     // These passes should make the code easier to analyze.
     // Passes, such as StatementFusion, are omitted for this reason.
     final boolean late = false;
-    boolean useTypesForOptimization = compiler.getOptions().useTypesForLocalOptimization;
+    CompilerOptions options = compiler.getOptions();
+    boolean useTypesForOptimization = options.useTypesForLocalOptimization;
     this.peepholePasses = new PeepholeOptimizationsPass(compiler,
-        new PeepholeMinimizeConditions(late, useTypesForOptimization),
+        new PeepholeMinimizeConditions(
+            late, useTypesForOptimization, options.assumeAccurateNullUndefinedTypes),
         new PeepholeSubstituteAlternateSyntax(late),
         new PeepholeReplaceKnownMethods(late, useTypesForOptimization),
         new PeepholeRemoveDeadCode(),

diff --git a/src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java b/src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java
@@ -40,6 +40,7 @@ class PeepholeMinimizeConditions
 
   private final boolean late;
   private final boolean useTypes;
+  private final boolean assumeAccurateNullUndefinedTypes;
 
   /**
    * @param late When late is false, this mean we are currently running before
@@ -48,9 +49,11 @@ class PeepholeMinimizeConditions
    * merging statements with commas, etc). When this is true, we would
    * do anything to minimize for size.
    */
-  PeepholeMinimizeConditions(boolean late, boolean useTypes) {
+  PeepholeMinimizeConditions(boolean late, boolean useTypes,
+      boolean assumeAccurateNullUndefinedTypes) {
     this.late = late;
     this.useTypes = useTypes;
+    this.assumeAccurateNullUndefinedTypes = assumeAccurateNullUndefinedTypes;
   }
 
   /**
@@ -1071,7 +1074,7 @@ private enum BooleanCoercability {
     RIGHT
   }
 
-  private static BooleanCoercability canConvertComparisonToBooleanCoercion(
+  private BooleanCoercability canConvertComparisonToBooleanCoercion(
       Node left, Node right, Token op) {
     // Convert null or undefined check of an object to coercion.
     boolean leftIsNull = left.isNull();
@@ -1084,10 +1087,11 @@ private static BooleanCoercability canConvertComparisonToBooleanCoercion(
     boolean leftIsObjectType = isObjectType(left);
     boolean rightIsObjectType = isObjectType(right);
     if (op == Token.SHEQ || op == Token.SHNE) {
-      if ((leftIsObjectType && !left.getJSType().isVoidable() && rightIsNull)
-          || (leftIsObjectType && !left.getJSType().isNullable() && rightIsUndefined)
-          || (rightIsObjectType && !right.getJSType().isVoidable() && leftIsNull)
-          || (rightIsObjectType && !right.getJSType().isNullable() && leftIsUndefined)) {
+      if ((leftIsObjectType && !left.getJSType().isNullable() && rightIsUndefined)
+          || (rightIsObjectType && !right.getJSType().isNullable() && leftIsUndefined)
+          || (assumeAccurateNullUndefinedTypes
+                 && ((leftIsObjectType && !left.getJSType().isVoidable() && rightIsNull)
+                     || (rightIsObjectType && !right.getJSType().isVoidable() && leftIsNull)))) {
         return leftIsNullOrUndefined ? BooleanCoercability.RIGHT : BooleanCoercability.LEFT;
       }
     } else {

diff --git a/test/com/google/javascript/jscomp/CreateSyntheticBlocksTest.java b/test/com/google/javascript/jscomp/CreateSyntheticBlocksTest.java
@@ -46,7 +46,8 @@ public void process(Node externs, Node js) {
         new MinimizeExitPoints(compiler).asCompilerPass().process(externs, js);
         new PeepholeOptimizationsPass(compiler,
             new PeepholeRemoveDeadCode(),
-            new PeepholeMinimizeConditions(true /* late */, false /* useTypes */),
+            new PeepholeMinimizeConditions(true /* late */, false /* useTypes */,
+                false /* assumeAccurateNullUndefinedTypes */),
             new PeepholeFoldConstants(true, false))
             .process(externs, js);
         new MinimizeExitPoints(compiler).asCompilerPass().process(externs, js);

diff --git a/test/com/google/javascript/jscomp/MultiPassTest.java b/test/com/google/javascript/jscomp/MultiPassTest.java
@@ -188,7 +188,8 @@ private void addPeephole() {
           protected CompilerPass create(AbstractCompiler compiler) {
             final boolean late = false;
             return new PeepholeOptimizationsPass(compiler,
-                new PeepholeMinimizeConditions(late, false /* useTypes */),
+                new PeepholeMinimizeConditions(
+                    late, false /* useTypes */, false /* assumeAccurateNullUndefinedTypes */),
                 new PeepholeSubstituteAlternateSyntax(late),
                 new PeepholeReplaceKnownMethods(late, false),
                 new PeepholeRemoveDeadCode(),

diff --git a/test/com/google/javascript/jscomp/PeepholeIntegrationTest.java b/test/com/google/javascript/jscomp/PeepholeIntegrationTest.java
@@ -33,7 +33,8 @@ public void setUp() throws Exception {
   public CompilerPass getProcessor(final Compiler compiler) {
     PeepholeOptimizationsPass peepholePass =
       new PeepholeOptimizationsPass(compiler,
-        new PeepholeMinimizeConditions(late, false /* useTypes */),
+        new PeepholeMinimizeConditions(
+            late, false /* useTypes */, false /* assumeAccurateNullUndefinedTypes */),
         new PeepholeSubstituteAlternateSyntax(late),
         new PeepholeRemoveDeadCode(),
         new PeepholeFoldConstants(late, false)

diff --git a/test/com/google/javascript/jscomp/PeepholeMinimizeConditionsTest.java b/test/com/google/javascript/jscomp/PeepholeMinimizeConditionsTest.java
@@ -25,18 +25,20 @@ public final class PeepholeMinimizeConditionsTest extends CompilerTestCase {
 
   private boolean late = true;
   private boolean useTypes = true;
+  private boolean assumeAccurateNullUndefinedTypes = true;
 
   @Override
   public void setUp() throws Exception {
     super.setUp();
     late = true;
     useTypes = true;
+    assumeAccurateNullUndefinedTypes = true;
   }
 
   @Override
   public CompilerPass getProcessor(final Compiler compiler) {
     PeepholeOptimizationsPass peepholePass = new PeepholeOptimizationsPass(
-        compiler, new PeepholeMinimizeConditions(late, useTypes));
+        compiler, new PeepholeMinimizeConditions(late, useTypes, assumeAccurateNullUndefinedTypes));
     peepholePass.setRetraverseOnChange(false);
     return peepholePass;
   }
@@ -866,6 +868,9 @@ public void testCoercionSubstitution_nullableType() {
         "if (/** @type {Array|undefined} */ (window['c']) == null) {}",
         "if (!/** @type {Array|undefined} */ (window['c'])) {}");
     testSame("if (/** @type {Array|undefined} */ (window['c']) === null) {}");
+
+    assumeAccurateNullUndefinedTypes = false;
+    testSame("var x = /** @type {?Object} */ ({}); if (x !== null) throw 'a';");
   }
 
   public void testCoercionSubstitution_unknownType() {