From 14b6188777cacfa904c0aaf24f0e3a18c29c1296 Mon Sep 17 00:00:00 2001 From: moz Date: Tue, 13 Dec 2016 12:35:34 -0800 Subject: [PATCH] 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} */ 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 --- .../javascript/jscomp/CommandLineRunner.java | 8 ++++++++ src/com/google/javascript/jscomp/Compiler.java | 1 + .../javascript/jscomp/CompilerOptions.java | 10 ++++++++++ .../javascript/jscomp/DefaultPassConfig.java | 9 ++++++--- .../javascript/jscomp/ExpandJqueryAliases.java | 6 ++++-- .../jscomp/PeepholeMinimizeConditions.java | 16 ++++++++++------ .../jscomp/CreateSyntheticBlocksTest.java | 3 ++- .../google/javascript/jscomp/MultiPassTest.java | 3 ++- .../jscomp/PeepholeIntegrationTest.java | 3 ++- .../jscomp/PeepholeMinimizeConditionsTest.java | 7 ++++++- 10 files changed, 51 insertions(+), 15 deletions(-) diff --git a/src/com/google/javascript/jscomp/CommandLineRunner.java b/src/com/google/javascript/jscomp/CommandLineRunner.java index d5d8b64265b..174821cf1ab 100644 --- 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 index 27cd98e6140..bf6168b4daa 100644 --- 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 index 9dcb02919af..92126feb9e1 100644 --- 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 index 1957171af62..d60da03275d 100644 --- 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 index cae8a74297d..a8b890c1614 100644 --- 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 index 4f64debc6b4..c259658f189 100644 --- 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 index 626517d17f1..2c2dc938f06 100644 --- 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 index 1487921b28f..94a0675575b 100644 --- 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 index c9b443e12c7..ee7e2805587 100644 --- 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 index 2b8d3b7fc24..2e385688f74 100644 --- 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() {