Fix nullness analysis in TypeTightener.
(1) Makes the CanPotentiallySeeUninitializedFields analysis available
in TypeTightener to fix an unsafe assumption when computing
nullability.
(2) Fixes a bug in CanPotentiallySeeUninitializedFields:
- instance initializer was incorrectly analyzed; the analysis
assumed $$init was the instance initializer whereas it was
the devirtualized instance initializer.
(3) Improves the strength of the analysis by explicitly checking whether
a "this" reference (or its devirtualized version) is used for a
polymorphic call, involved in the parameters to static calls or
involved in an expression that may create an alias to it.
(4) Adds some missing optimizations related to nullability.
Change-Id: I7a14112af353d50c4d83a2a31f47b1138488586e
diff --git a/dev/core/src/com/google/gwt/dev/javac/testing/impl/JavaResourceBase.java b/dev/core/src/com/google/gwt/dev/javac/testing/impl/JavaResourceBase.java
index 88d1278..3f7f7c9 100644
--- a/dev/core/src/com/google/gwt/dev/javac/testing/impl/JavaResourceBase.java
+++ b/dev/core/src/com/google/gwt/dev/javac/testing/impl/JavaResourceBase.java
@@ -314,6 +314,7 @@
" public int hashCode() { return 0; }",
" public String replace(char c1, char c2) { return null; }",
" public boolean startsWith(String str) { return false; }",
+ " public native String substring(int start, int len) /*-{ return \"\"; }-*/;",
" public String toLowerCase() { return null; }",
" public String toString() { return this; }",
" public static String valueOf(boolean b) { return null; }",
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java
index a013234..9d80893 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java
@@ -847,6 +847,16 @@
return runtimeRebindRegistratorTypeName;
}
+ public Collection<JType> getSubclasses(JType type) {
+ return Collections2.transform(typeOracle.getSubTypeNames(type.getName()),
+ new Function<String, JType>() {
+ public JType apply(String typeName) {
+ return getFromTypeMap(typeName);
+ }
+ }
+ );
+ }
+
public JMethod getStaticImpl(JMethod method) {
JMethod staticImpl = instanceToStaticMap.get(method);
assert staticImpl == null || staticImpl.getEnclosingType().getMethods().contains(staticImpl);
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JThisRef.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JThisRef.java
index 5c4d9d6..0778d07 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JThisRef.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JThisRef.java
@@ -18,7 +18,7 @@
import com.google.gwt.dev.jjs.SourceInfo;
/**
- * Java method this expression.
+ * Java method this (or super) expression.
*/
public class JThisRef extends JExpression {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/js/JMultiExpression.java b/dev/core/src/com/google/gwt/dev/jjs/ast/js/JMultiExpression.java
index da4b558..4a8d6a5 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/js/JMultiExpression.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/js/JMultiExpression.java
@@ -24,6 +24,7 @@
import com.google.gwt.thirdparty.guava.common.collect.Lists;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.List;
@@ -45,7 +46,7 @@
/**
* Construct a multi expression containing {@code expressions}.
*/
- public JMultiExpression(SourceInfo info, List<JExpression> expressions) {
+ public JMultiExpression(SourceInfo info, Collection<JExpression> expressions) {
super(info);
this.expressions.addAll(expressions);
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ComputePotentiallyObservableUninitializedValues.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ComputePotentiallyObservableUninitializedValues.java
new file mode 100644
index 0000000..556415e
--- /dev/null
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ComputePotentiallyObservableUninitializedValues.java
@@ -0,0 +1,255 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package com.google.gwt.dev.jjs.impl;
+
+import com.google.gwt.dev.jjs.ast.Context;
+import com.google.gwt.dev.jjs.ast.JClassType;
+import com.google.gwt.dev.jjs.ast.JConstructor;
+import com.google.gwt.dev.jjs.ast.JField;
+import com.google.gwt.dev.jjs.ast.JFieldRef;
+import com.google.gwt.dev.jjs.ast.JInterfaceType;
+import com.google.gwt.dev.jjs.ast.JMethod;
+import com.google.gwt.dev.jjs.ast.JMethodCall;
+import com.google.gwt.dev.jjs.ast.JParameter;
+import com.google.gwt.dev.jjs.ast.JParameterRef;
+import com.google.gwt.dev.jjs.ast.JProgram;
+import com.google.gwt.dev.jjs.ast.JThisRef;
+import com.google.gwt.dev.jjs.ast.JType;
+import com.google.gwt.dev.jjs.ast.JVisitor;
+import com.google.gwt.dev.util.log.speedtracer.CompilerEventType;
+import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger;
+import com.google.gwt.thirdparty.guava.common.base.Predicate;
+import com.google.gwt.thirdparty.guava.common.collect.Sets;
+
+import java.util.Set;
+
+/**
+ * Determines conservatively which classes can potentially see uninitialized values of their
+ * subclasses' fields.
+ * <p>
+ * This simple conservative analysis relies on the fact that when: <ul>
+ * <li> (1) there are no virtual calls on "this" in any of the initialization methods
+ * (constructors, init) of all the superclasses, and </li>
+ * <li> (2) "this" does not escape through a parameter to other methods, and </li>
+ * <li>(3) "this" is not aliased (stored into another field, variable or array)</li>
+ * </ul>
+ * then uninitialized values for subclass fields can never be seen.
+ * <p>
+ * This analysis is used to strengthen the nullness analysis performed by {@link TypeTightener} and
+ * to hoist initialization of instance fields to the prototype in {@link GenerateJavaScriptAST}.
+ */
+public class ComputePotentiallyObservableUninitializedValues {
+
+ private static final String NAME =
+ ComputePotentiallyObservableUninitializedValues.class.getSimpleName();
+ private final JProgram program;
+ private final Set<JType> classesWhoseFieldsCanBeObservedUninitialized = Sets.newHashSet();
+
+ private ComputePotentiallyObservableUninitializedValues(JProgram program) {
+ this.program = program;
+ }
+
+ /**
+ * Perform the analysis to compute which fields can be observed uninitialized.
+ */
+ public static Predicate<JField> analyze(JProgram program) {
+ return new ComputePotentiallyObservableUninitializedValues(program).analyzeImpl();
+ }
+
+ private Predicate<JField> analyzeImpl() {
+ SpeedTracerLogger.Event optimizeEvent =
+ SpeedTracerLogger.start(CompilerEventType.OPTIMIZE, "optimizer", NAME);
+ CanObserveSubclassUninitializedFieldsVisitor visitor =
+ new CanObserveSubclassUninitializedFieldsVisitor();
+ visitor.accept(program);
+ Set<JType> classesThatCanPotentiallyObserveUninitializedSubclassFields =
+ visitor.classesThatCanPotentiallyObserveUninitializedSubclassFields;
+
+ for (JType type : classesThatCanPotentiallyObserveUninitializedSubclassFields) {
+ if (classesWhoseFieldsCanBeObservedUninitialized.contains(type)) {
+ // Already processed.
+ continue;
+ }
+ classesWhoseFieldsCanBeObservedUninitialized.addAll(program.getSubclasses(type));
+ }
+
+ optimizeEvent.end();
+
+ return new Predicate<JField>() {
+ @Override
+ public boolean apply(JField field) {
+ return isUninitializedValueObservable(field);
+ }
+ };
+ }
+
+ private boolean isUninitializedValueObservable(JField x) {
+ if (x.getLiteralInitializer() != null && (x.isFinal() || x.isStatic())) {
+ // Static and final fields that are initialized to a (value) literal can not be observed in
+ // uninitialized state.
+ return false;
+ }
+
+ if (x.isStatic()) {
+ // Static fields can potentially be observed uninitialized if clinit dependencies are
+ // cyclical.
+ return true;
+ }
+
+ return classesWhoseFieldsCanBeObservedUninitialized.contains(x.getEnclosingType());
+ }
+
+ private class CanObserveSubclassUninitializedFieldsVisitor extends JVisitor {
+ private JClassType currentClass;
+ private JParameter devirtualizedThis;
+ private Set<JType> classesThatCanPotentiallyObserveUninitializedSubclassFields =
+ Sets.newHashSet();
+
+ @Override
+ public void endVisit(JClassType x, Context ctx) {
+ assert currentClass == x;
+ currentClass = null;
+ }
+
+ @Override
+ public void endVisit(JConstructor x, Context ctx) {
+ assert currentClass == x.getEnclosingType();
+ assert devirtualizedThis == null;
+ }
+
+ @Override
+ public void endVisit(JMethod x, Context ctx) {
+ assert currentClass == x.getEnclosingType();
+ devirtualizedThis = null;
+ }
+
+ @Override
+ public void endVisit(JThisRef x, Context ctx) {
+ // Seen a reference to "this" that can potentially escape or be used as instance in a
+ // method call.
+ classesThatCanPotentiallyObserveUninitializedSubclassFields.add(currentClass);
+ }
+
+ public void endVisit(JParameterRef x, Context ctx) {
+ if (x.getParameter() == devirtualizedThis) {
+ // Seen a reference to devirtualized "this" that can potentially escape or be used as
+ // instance in a method call.
+ classesThatCanPotentiallyObserveUninitializedSubclassFields.add(currentClass);
+ }
+ }
+
+ @Override
+ public boolean visit(JClassType x, Context ctx) {
+ assert currentClass == null;
+ currentClass = x;
+ return true;
+ }
+
+ @Override
+ public boolean visit(JConstructor x, Context ctx) {
+ // Only look at constructor bodies.
+ assert currentClass == x.getEnclosingType();
+ return true;
+ }
+
+ @Override
+ public boolean visit(JFieldRef x, Context ctx) {
+ if (isFieldReferenceThroughThis(x) && isFieldDeclaredInCurrentClassOrSuper(x)) {
+ // Accessing fields through this (or devirtualized this) from the current class or
+ // any super is ok, no further checks are needed.
+ // A subclass field ref can leak into superclass methods when optimizations are enabled.
+ return false;
+ }
+ return true;
+ }
+
+ @Override
+ public boolean visit(JInterfaceType x, Context ctx) {
+ // No need to examine interfaces.
+ return false;
+ }
+
+ @Override
+ public boolean visit(JMethod x, Context ctx) {
+ assert currentClass == x.getEnclosingType();
+ if (isInitMethod(x)) {
+ return true;
+ }
+
+ if (isDevirtualizedInitMethod(x) && x.getParams().size() > 0
+ && x.getParams().get(0).getType() == currentClass) {
+ devirtualizedThis = x.getParams().get(0);
+ }
+ // Do not explore the method body if it is not a constructor or the instance initializer.
+ return false;
+ }
+
+ @Override
+ public boolean visit(JMethodCall x, Context ctx) {
+ // This is a method call inside a constructor.
+ assert currentClass != null;
+ // Calls to this/super constructors and instance initializers are okay, as they will also
+ // get examined.
+ if ((x.getTarget().isConstructor()) && x.getInstance() instanceof JThisRef ||
+ isInitMethod(x.getTarget())) {
+ // Make sure "this" references do not escape through parameters
+ accept(x.getArgs());
+ return false;
+ }
+
+ // The instance initializers are always devirtualized, handle them specially.
+ if (isDevirtualizedInitMethod(x.getTarget()) && x.getArgs().size() > 0 &&
+ x.getArgs().get(0) instanceof JThisRef) {
+ // Make sure "this" references do not escape through parameters other than the first.
+ accept(x.getArgs().subList(1, x.getArgs().size()));
+ return false;
+ }
+
+ if (!x.getTarget().isStatic() && !x.getTarget().isFinal() &&
+ x.getInstance() instanceof JThisRef) {
+ // This is polymorphic method call on this, hence it is potentially unsafe.
+ classesThatCanPotentiallyObserveUninitializedSubclassFields.add(currentClass);
+ return false;
+ }
+
+ // This is a static call, if there is no "this" references in its parameters then it might
+ // be ok.
+ return true;
+ }
+
+ private boolean isDevirtualizedInitMethod(JMethod method) {
+ return method.isStatic() && method.getName().equals("$$init") &&
+ method.getEnclosingType() == currentClass;
+ }
+
+ private boolean isInitMethod(JMethod method) {
+ return !method.isStatic() && method.getName().equals("$init") &&
+ method.getEnclosingType() == currentClass;
+ }
+
+ private boolean isFieldReferenceThroughThis(JFieldRef x) {
+ return x.getInstance() instanceof JThisRef || x.getInstance() instanceof JParameterRef &&
+ ((JParameterRef) x.getInstance()).getParameter() == devirtualizedThis;
+ }
+
+ private boolean isFieldDeclaredInCurrentClassOrSuper(JFieldRef x) {
+ JClassType enclosingClass = (JClassType) x.getField().getEnclosingType();
+ return currentClass == enclosingClass ||
+ program.typeOracle.isSuperClass(enclosingClass, currentClass);
+ }
+ }
+}
\ No newline at end of file
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java
index 2b5afe5..b5b0779 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java
@@ -71,8 +71,6 @@
import com.google.gwt.dev.util.log.speedtracer.CompilerEventType;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.Event;
-import com.google.gwt.thirdparty.guava.common.base.Predicate;
-import com.google.gwt.thirdparty.guava.common.collect.Iterables;
import com.google.gwt.thirdparty.guava.common.collect.Lists;
import com.google.gwt.thirdparty.guava.common.collect.Sets;
@@ -149,11 +147,48 @@
JBinaryOperator op = x.getOp();
JExpression lhs = x.getLhs();
JExpression rhs = x.getRhs();
+
+ // If either parameter is a multiexpression, restructure if possible.
+ if (isNonEmptyMultiExpression(lhs)) {
+ // Push the operation inside the multiexpression. This exposes other optimization
+ // opportunities for latter passes e.g:
+ //
+ // (a(), b(), 1) + 2 ==> (a(), b(), 1 + 2) ==> (a(), b(), 3)
+ //
+ // There is no need to consider the symmetric case as it requires that all expression in the
+ // rhs (except the last one) to be side effect free, in which case they will end up being
+ // removed anyway.
+
+ List<JExpression> expressions = ((JMultiExpression) lhs).getExpressions();
+ JMultiExpression result = new JMultiExpression(lhs.getSourceInfo(),
+ expressions.subList(0, expressions.size() - 1));
+ result.addExpressions(new JBinaryOperation(x.getSourceInfo(), x.getType(), x.getOp(),
+ expressions.get(expressions.size() - 1), rhs));
+ ctx.replaceMe(result);
+ return;
+ }
+
+ if (isNonEmptyMultiExpression(rhs) && lhs instanceof JValueLiteral) {
+ // Push the operation inside the multiexpression if the lhs is a value literal.
+ // This exposes other optimization opportunities for latter passes e.g:
+ //
+ // 2 + (a(), b(), 1) ==> (a(), b(), 2 + 1) ==> (a(), b(), 3)
+ //
+ List<JExpression> expressions = ((JMultiExpression) rhs).getExpressions();
+ JMultiExpression result = new JMultiExpression(rhs.getSourceInfo(),
+ expressions.subList(0, expressions.size() - 1));
+ result.addExpressions(new JBinaryOperation(x.getSourceInfo(), x.getType(), x.getOp(),
+ lhs, expressions.get(expressions.size() - 1)));
+ ctx.replaceMe(result);
+ return;
+ }
+
if ((lhs instanceof JValueLiteral) && (rhs instanceof JValueLiteral)) {
if (evalOpOnLiterals(op, (JValueLiteral) lhs, (JValueLiteral) rhs, ctx)) {
return;
}
}
+
switch (op) {
case AND:
maybeReplaceMe(x, Simplifier.and(x), ctx);
@@ -200,6 +235,11 @@
}
}
+ private boolean isNonEmptyMultiExpression(JExpression expression) {
+ return expression instanceof JMultiExpression &&
+ !((JMultiExpression) expression).getExpressions().isEmpty();
+ }
+
/**
* Prune dead statements and empty blocks.
*/
@@ -386,7 +426,6 @@
*/
@Override
public void endVisit(JInstanceOf x, Context ctx) {
-
if (ignoringExpressionOutput.contains(x)) {
ctx.replaceMe(x.getExpr());
ignoringExpressionOutput.remove(x);
@@ -514,7 +553,7 @@
super.endVisit(x, ctx);
/*
* If the result of a new operation is ignored, we can remove it, provided
- * / it has no side effects.
+ * it has no side effects.
*/
if (ignoringExpressionOutput.contains(x)) {
if (!x.getTarget().isEmpty()) {
@@ -549,6 +588,17 @@
if (x.getOp().isModifying()) {
lvalues.remove(x.getArg());
}
+
+ if (isNonEmptyMultiExpression(x.getArg())) {
+ List<JExpression> expressions = ((JMultiExpression) x.getArg()).getExpressions();
+ JMultiExpression result = new JMultiExpression(x.getArg().getSourceInfo(),
+ expressions.subList(0, expressions.size() - 1));
+ result.addExpressions(new JPostfixOperation(x.getSourceInfo(), x.getOp(),
+ expressions.get(expressions.size() - 1)));
+ ctx.replaceMe(result);
+ return;
+ }
+
if (ignoringExpressionOutput.contains(x)) {
JPrefixOperation newOp = new JPrefixOperation(x.getSourceInfo(), x.getOp(), x.getArg());
ctx.replaceMe(newOp);
@@ -563,11 +613,24 @@
if (x.getOp().isModifying()) {
lvalues.remove(x.getArg());
}
+
+ // If the argument is a multiexpression restructure it if possible.
+ if (isNonEmptyMultiExpression(x.getArg())) {
+ List<JExpression> expressions = ((JMultiExpression) x.getArg()).getExpressions();
+ JMultiExpression result = new JMultiExpression(x.getArg().getSourceInfo(),
+ expressions.subList(0, expressions.size() - 1));
+ result.addExpressions(new JPrefixOperation(x.getSourceInfo(), x.getOp(),
+ expressions.get(expressions.size() - 1)));
+ ctx.replaceMe(result);
+ return;
+ }
+
if (x.getArg() instanceof JValueLiteral) {
if (evalOpOnLiteral(x.getOp(), (JValueLiteral) x.getArg(), ctx)) {
return;
}
}
+
if (x.getOp() == JUnaryOperator.NOT) {
maybeReplaceMe(x, Simplifier.not(x), ctx);
return;
@@ -1522,26 +1585,6 @@
return AnalysisResult.UNKNOWN;
}
- private JExpression simplifyNonSideEffects(JExpression result,
- JExpression... evaluateIfSideEffects) {
-
- List<JExpression> expressionsWithSideEffects = Lists.newArrayList(Iterables.filter(
- Arrays.asList(evaluateIfSideEffects), new Predicate<JExpression>() {
- @Override
- public boolean apply(JExpression expression) {
- return expression.hasSideEffects();
- }
- }));
-
- if (expressionsWithSideEffects.isEmpty()) {
- return result;
- }
-
- expressionsWithSideEffects.add(result);
- return new JMultiExpression(expressionsWithSideEffects.get(0).getSourceInfo(),
- expressionsWithSideEffects);
- }
-
/**
* Simplify <code>lhs == rhs</code>. If <code>negate</code> is true, then
* it's actually static evaluation of <code>lhs != rhs</code>.
@@ -1550,8 +1593,8 @@
// simplify: null == null -> true
AnalysisResult analysisResult = staticallyEvaluateEq(lhs, rhs);
if (analysisResult != AnalysisResult.UNKNOWN) {
- ctx.replaceMe(simplifyNonSideEffects(
- program.getLiteralBoolean(negate ^ (analysisResult == AnalysisResult.TRUE)), lhs, rhs));
+ ctx.replaceMe(JjsUtils.createOptimizedMultiExpression(
+ lhs, rhs, program.getLiteralBoolean(negate ^ (analysisResult == AnalysisResult.TRUE))));
return;
}
@@ -1971,5 +2014,5 @@
return stats;
}
- private enum AnalysisResult { TRUE, FALSE, UNKNOWN };
+ private enum AnalysisResult { TRUE, FALSE, UNKNOWN }
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
index acc97c0..3c05506 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
@@ -170,6 +170,7 @@
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.Event;
import com.google.gwt.thirdparty.guava.common.base.Function;
+import com.google.gwt.thirdparty.guava.common.base.Predicate;
import com.google.gwt.thirdparty.guava.common.base.Predicates;
import com.google.gwt.thirdparty.guava.common.collect.ImmutableList;
import com.google.gwt.thirdparty.guava.common.collect.ImmutableMap;
@@ -2949,16 +2950,8 @@
// we can definitely initialize at top-scope, as JVM does so as well
return true;
}
- // if the superclass can observe the field, we need to leave the default value
- JDeclaredType current = x.getEnclosingType().getSuperClass();
- while (current != null) {
- if (canObserveSubclassFields.contains(current)) {
- return false;
- }
- current = current.getSuperClass();
- }
- // should be safe to initialize at top-scope, as no one can observe the difference
- return true;
+
+ return !uninitializedValuePotentiallyObservable.apply(x);
}
// Keep track of a translation stack.
@@ -3041,70 +3034,6 @@
}
}
- /**
- * Determines which classes can potentially see uninitialized values of their subclasses' fields.
- *
- * If a class can not observe subclass uninitialized fields then the initialization of those could
- * be hoisted to the prototype.
- */
- private class CanObserveSubclassUninitializedFieldsVisitor extends JVisitor {
- private JDeclaredType currentClass;
-
- @Override
- public boolean visit(JConstructor x, Context ctx) {
- // Only look at constructor bodies.
- assert currentClass == null;
- currentClass = x.getEnclosingType();
- return true;
- }
-
- @Override
- public void endVisit(JConstructor x, Context ctx) {
- currentClass = null;
- }
-
- @Override
- public boolean visit(JMethod x, Context ctx) {
- if (x.getName().equals("$$init")) {
- assert currentClass == null;
- currentClass = x.getEnclosingType();
- return true;
- }
- // Do not traverse the method body if it is not a constructor.
- return false;
- }
-
- @Override
- public void endVisit(JMethod x, Context ctx) {
- currentClass = null;
- }
-
- @Override
- public void endVisit(JMethodCall x, Context ctx) {
- // This is a method call inside a constructor.
- assert currentClass != null;
- // Calls to this/super constructors are okay, as they will also get examined
- if (x.getTarget().isConstructor() && x.getInstance() instanceof JThisRef) {
- return;
- }
- // Calls to the instance initializer are okay, as execution will not escape it
- if (x.getTarget().getName().equals("$$init")) {
- return;
- }
- // Calls to static methods with no arguments are safe, because they have no
- // way to trace back into our new instance.
- if (x.getTarget().isStatic() && x.getTarget().getOriginalParamTypes().size() == 0) {
- return;
- }
- // Technically we could get fancier about trying to track the "this" reference
- // through other variable assignments, field assignments, and methods calls, to
- // see if it calls a polymorphic method against ourself (which would let subclasses
- // observe their fields), but that gets tricky, so we'll use the above heuristics
- // for now.
- canObserveSubclassFields.add(currentClass);
- }
- }
-
private class RecordJSInlinableMethods extends JVisitor {
private JMethod currentMethod;
@@ -3314,7 +3243,7 @@
* Classes that could potentially see uninitialized values for fields that are initialized in the
* declaration.
*/
- private final Set<JDeclaredType> canObserveSubclassFields = Sets.newHashSet();
+ private Predicate<JField> uninitializedValuePotentiallyObservable;
private final Map<JAbstractMethodBody, JsFunction> methodBodyMap = Maps.newIdentityHashMap();
private final Map<HasName, JsName> names = Maps.newIdentityHashMap();
@@ -3579,18 +3508,10 @@
}
}
- private void assumeAllClassesCanObserveUninitializedSubclassFields(JProgram program) {
- canObserveSubclassFields.addAll(program.getDeclaredTypes());
- }
-
private Pair<JavaToJavaScriptMap, Set<JsNode>> execImpl() {
new FixNameClashesVisitor().accept(program);
- if (optimize) {
- new CanObserveSubclassUninitializedFieldsVisitor().accept(program);
- } else {
- // Assume the worst case.
- assumeAllClassesCanObserveUninitializedSubclassFields(program);
- }
+ uninitializedValuePotentiallyObservable = optimize ?
+ ComputePotentiallyObservableUninitializedValues.analyze(program) : Predicates.<JField>alwaysTrue();
new FindNameOfTargets().accept(program);
new SortVisitor().accept(program);
if (hasWholeWorldKnowledge) {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/JjsUtils.java b/dev/core/src/com/google/gwt/dev/jjs/impl/JjsUtils.java
index 14f3f5e..feeba91 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/JjsUtils.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/JjsUtils.java
@@ -16,6 +16,7 @@
package com.google.gwt.dev.jjs.impl;
import com.google.gwt.dev.jjs.SourceInfo;
+import com.google.gwt.dev.jjs.SourceOrigin;
import com.google.gwt.dev.jjs.ast.JBooleanLiteral;
import com.google.gwt.dev.jjs.ast.JCharLiteral;
import com.google.gwt.dev.jjs.ast.JDoubleLiteral;
@@ -26,6 +27,7 @@
import com.google.gwt.dev.jjs.ast.JLongLiteral;
import com.google.gwt.dev.jjs.ast.JNullLiteral;
import com.google.gwt.dev.jjs.ast.JStringLiteral;
+import com.google.gwt.dev.jjs.ast.js.JMultiExpression;
import com.google.gwt.dev.js.ast.JsBooleanLiteral;
import com.google.gwt.dev.js.ast.JsExpression;
import com.google.gwt.dev.js.ast.JsLiteral;
@@ -35,15 +37,72 @@
import com.google.gwt.dev.js.ast.JsObjectLiteral;
import com.google.gwt.dev.js.ast.JsStringLiteral;
import com.google.gwt.lang.LongLib;
+import com.google.gwt.thirdparty.guava.common.base.Predicate;
+import com.google.gwt.thirdparty.guava.common.base.Predicates;
+import com.google.gwt.thirdparty.guava.common.collect.Collections2;
import com.google.gwt.thirdparty.guava.common.collect.ImmutableMap;
+import com.google.gwt.thirdparty.guava.common.collect.Lists;
+import java.util.Arrays;
+import java.util.List;
import java.util.Map;
/**
- * Translates Java literals into JavaScript literals.
+ * General utilities related to Java AST manipulation.
*/
public class JjsUtils {
+ /**
+ * Creates a multi expression from a list of expressions, removing expressions that do
+ * not have side effects if possible.
+ */
+ public static JExpression createOptimizedMultiExpression(JExpression... expressions) {
+ return createOptimizedMultiExpression(false, Arrays.asList(expressions));
+ }
+
+ /**
+ * Creates a multi expression from a list of expressions, removing expressions that do
+ * not have side effects if possible.
+ */
+ public static JExpression createOptimizedMultiExpression(boolean ignoringResult,
+ List<JExpression> expressions) {
+
+ int numberOfExpressions = expressions.size();
+ JExpression result = expressions.get(numberOfExpressions - 1);
+
+ numberOfExpressions = expressions.size();
+ if (numberOfExpressions == 0) {
+ return new JMultiExpression(SourceOrigin.UNKNOWN);
+ }
+
+ expressions = Lists.newArrayList(Collections2.filter(
+ expressions.subList(0, numberOfExpressions - 1),
+ Predicates.and(
+ Predicates.notNull(),
+ new Predicate<JExpression>() {
+ @Override
+ public boolean apply(JExpression expression) {
+ return expression.hasSideEffects();
+ }
+ })));
+
+ if (result != null && (!ignoringResult || result.hasSideEffects())) {
+ expressions.add(result);
+ }
+
+ if (expressions.size() == 1) {
+ // Do not create a multi expression if it consists only of the result.
+ return expressions.iterator().next();
+ }
+
+ SourceInfo info = expressions.size() > 0 ? expressions.get(0).getSourceInfo() :
+ SourceOrigin.UNKNOWN;
+ return new JMultiExpression(info, expressions);
+ }
+
+ /**
+ * Translates a Java literal into a JavaScript literal.
+ */
public static JsLiteral translateLiteral(JLiteral literal) {
return translatorByLiteralClass.get(literal.getClass()).translate(literal);
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java b/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java
index 819a35f..de4aca5 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java
@@ -37,9 +37,11 @@
import com.google.gwt.dev.jjs.ast.JType;
import com.google.gwt.dev.jjs.ast.JVisitor;
import com.google.gwt.dev.jjs.ast.js.JMultiExpression;
+import com.google.gwt.dev.util.collect.Stack;
import com.google.gwt.dev.util.log.speedtracer.CompilerEventType;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.Event;
+import com.google.gwt.thirdparty.guava.common.collect.Lists;
import com.google.gwt.thirdparty.guava.common.collect.Sets;
import java.util.LinkedHashSet;
@@ -84,7 +86,12 @@
* inlined before might become inlinable.
*/
private final Set<JMethod> cannotInline = Sets.newHashSet();
- private JExpression ignoringReturnValueFor;
+ private final Stack<JExpression> expressionsWhoseValuesAreIgnored = Stack.create();
+
+ @Override
+ public void endVisit(JExpressionStatement x, Context ctx) {
+ expressionsWhoseValuesAreIgnored.pop();
+ }
@Override
public void endVisit(JMethod x, Context ctx) {
@@ -104,29 +111,36 @@
return;
}
- if (!tryInlineMethodCall(x, ctx)) {
+ if (tryInlineMethodCall(x, ctx) == InlineResult.BLACKLIST) {
// Do not try to inline this method again
cannotInline.add(method);
}
}
- private boolean tryInlineMethodCall(JMethodCall x, Context ctx) {
+ @Override
+ public void endVisit(JMultiExpression x, Context ctx) {
+ for (int i = 0; i < x.getExpressions().size() - 1; i++) {
+ expressionsWhoseValuesAreIgnored.pop();
+ }
+ }
+
+ private InlineResult tryInlineMethodCall(JMethodCall x, Context ctx) {
JMethod method = x.getTarget();
if (program.isJsTypePrototype(method.getEnclosingType())) {
/*
* Don't inline calls to JsType Prototype methods, since these are merely stubs to
* preserve super calls to JS prototype methods.
*/
- return false;
+ return InlineResult.BLACKLIST;
}
if (!method.isStatic() || method.isNative()) {
// Only inline static methods that are not native.
- return false;
+ return InlineResult.BLACKLIST;
}
if (!program.isInliningAllowed(method)) {
- return false;
+ return InlineResult.BLACKLIST;
}
JMethodBody body = (JMethodBody) method.getBody();
@@ -135,24 +149,23 @@
if (method.getEnclosingType() != null
&& method.getEnclosingType().getClinitMethod() == method && !stmts.isEmpty()) {
// clinit() calls cannot be inlined unless they are empty
- return false;
+ return InlineResult.BLACKLIST;
}
if (!body.getLocals().isEmpty()) {
// methods with local variables cannot be inlined
- return false;
+ return InlineResult.BLACKLIST;
}
// try to inline
- JMultiExpression multi = createMultiExpressionFromBody(body, ignoringReturnValueFor == x);
- if (multi == null || !tryInlineExpression(x, ctx, multi)) {
+ List<JExpression> expressions = extractExpressionsFromBody(body);
+ if (expressions == null) {
// If it will never be possible to inline the method, add it to a
// blacklist
- return false;
+ return InlineResult.BLACKLIST;
}
- // Successfully inlined.
- return true;
+ return tryInlineBody(x, ctx, expressions, expressionsWhoseValuesAreIgnored.contains(x));
}
@Override
@@ -162,7 +175,7 @@
@Override
public boolean visit(JExpressionStatement x, Context ctx) {
- ignoringReturnValueFor = x.getExpr();
+ expressionsWhoseValuesAreIgnored.push(x.getExpr());
return true;
}
@@ -186,6 +199,14 @@
return true;
}
+ @Override
+ public boolean visit(JMultiExpression x, Context ctx) {
+ for (int i = 0; i < x.getExpressions().size() - 1; i++) {
+ expressionsWhoseValuesAreIgnored.push(x.getExpression(i));
+ }
+ return true;
+ }
+
private JMethodCall createClinitCall(JMethodCall x) {
JDeclaredType targetType = x.getTarget().getEnclosingType().getClinitTarget();
if (!currentMethod.getEnclosingType().checkClinitTo(targetType)) {
@@ -213,27 +234,6 @@
}
/**
- * Creates a multi expression for evaluating a method call instance and
- * possible clinit. This is a precursor for inlining the remainder of a
- * method.
- */
- private JMultiExpression createMultiExpressionForInstanceAndClinit(JMethodCall x) {
- JMultiExpression multi = new JMultiExpression(x.getSourceInfo());
-
- // Any instance expression goes first (this can happen even with statics).
- if (x.getInstance() != null) {
- multi.addExpressions(x.getInstance());
- }
-
- // If we need a clinit call, add it first
- JMethodCall clinit = createClinitCall(x);
- if (clinit != null) {
- multi.addExpressions(clinit);
- }
- return multi;
- }
-
- /**
* Creates a JMultiExpression from a set of JExpressionStatements,
* optionally terminated by a JReturnStatement. If the method doesn't match
* this pattern, it returns <code>null</code>.
@@ -243,9 +243,8 @@
* expression of the method. If the method is void, the output of the
* multi-expression should be considered undefined.
*/
- private JMultiExpression createMultiExpressionFromBody(JMethodBody body,
- boolean ignoringReturnValue) {
- JMultiExpression multi = new JMultiExpression(body.getSourceInfo());
+ private List<JExpression> extractExpressionsFromBody(JMethodBody body) {
+ List<JExpression> expressions = Lists.newArrayList();
CloneCalleeExpressionVisitor cloner = new CloneCalleeExpressionVisitor();
for (JStatement stmt : body.getStatements()) {
@@ -253,16 +252,14 @@
JExpressionStatement exprStmt = (JExpressionStatement) stmt;
JExpression expr = exprStmt.getExpr();
JExpression clone = cloner.cloneExpression(expr);
- multi.addExpressions(clone);
+ expressions.add(clone);
} else if (stmt instanceof JReturnStatement) {
JReturnStatement returnStatement = (JReturnStatement) stmt;
JExpression expr = returnStatement.getExpr();
if (expr != null) {
- if (!ignoringReturnValue || expr.hasSideEffects()) {
- JExpression clone = cloner.cloneExpression(expr);
- clone = maybeCast(clone, body.getMethod().getType());
- multi.addExpressions(clone);
- }
+ JExpression clone = cloner.cloneExpression(expr);
+ clone = maybeCast(clone, body.getMethod().getType());
+ expressions.add(clone);
}
// We hit an unconditional return; no need to evaluate anything else.
break;
@@ -272,16 +269,18 @@
}
}
- return multi;
+ return expressions;
}
/**
- * Creates a multi expression for evaluating a method call instance,
+ * Creates a lists of expression for evaluating a method call instance,
* possible clinit, and all arguments. This is a precursor for inlining the
* remainder of a method that does not reference any parameters.
*/
- private JMultiExpression createMultiExpressionIncludingArgs(JMethodCall x) {
- JMultiExpression multi = createMultiExpressionForInstanceAndClinit(x);
+ private List<JExpression> expressionsIncludingArgs(JMethodCall x) {
+ List<JExpression> expressions = Lists.newArrayListWithCapacity(x.getArgs().size() + 2);
+ expressions.add(x.getInstance());
+ expressions.add(createClinitCall(x));
for (int i = 0, c = x.getArgs().size(); i < c; ++i) {
JExpression arg = x.getArgs().get(i);
@@ -289,50 +288,46 @@
analyzer.accept(arg);
if (analyzer.hasAssignment() || analyzer.canThrowException()) {
- multi.addExpressions(arg);
+ expressions.add(arg);
}
}
- return multi;
+ return expressions;
}
/**
- * Replace the current expression with a given multi-expression and mark the
- * method as modified. The dead-code elimination pass will optimize this if
- * necessary.
+ * Replace the current expression with a given replacement and mark the
+ * method as modified.
*/
- private void replaceWithMulti(Context ctx, JMultiExpression multi) {
- ctx.replaceMe(multi);
+ private void replaceMeAndRecordModification(Context ctx, JExpression replacement) {
+ ctx.replaceMe(replacement);
modifiedMethods.add(currentMethod);
}
/**
- * Inline a call to an expression.
+ * Inline a call to an expression. Returns {@code InlineResult.BLACKLIST} if the method is
+ * deemed not inlineable regardless of call site; {@code InlineResult.DO_NOT_BLACKLIST}
+ * otherwise.
*/
- private boolean tryInlineExpression(JMethodCall x, Context ctx, JMultiExpression targetExpr) {
- /*
- * Limit inlined methods to multiexpressions of length 2 for now. This
- * handles the simple { return JVariableRef; } or { expression; return
- * something; } cases.
- *
- * TODO: add an expression complexity analyzer.
- */
- if (targetExpr.getNumberOfExpressions() > 2) {
- return false;
+ private InlineResult tryInlineBody(JMethodCall x, Context ctx,
+ List<JExpression> bodyAsExpressionList, boolean ignoringReturn) {
+
+ if (isTooComplexToInline(bodyAsExpressionList, ignoringReturn)) {
+ return InlineResult.BLACKLIST;
}
// Do not inline anything that modifies one of its params.
ExpressionAnalyzer targetAnalyzer = new ExpressionAnalyzer();
- targetAnalyzer.accept(targetExpr);
+ targetAnalyzer.accept(bodyAsExpressionList);
if (targetAnalyzer.hasAssignmentToParameter()) {
- return false;
+ return InlineResult.BLACKLIST;
}
// Make sure the expression we're about to inline doesn't include a call
// to the target method!
RecursionCheckVisitor recursionCheckVisitor = new RecursionCheckVisitor(x.getTarget());
- recursionCheckVisitor.accept(targetExpr);
+ recursionCheckVisitor.accept(bodyAsExpressionList);
if (recursionCheckVisitor.isRecursive()) {
- return false;
+ return InlineResult.BLACKLIST;
}
/*
@@ -348,7 +343,8 @@
* TODO: would this be possible in the trivial delegation case?
*/
if (x.getTarget().getParams().size() != x.getArgs().size()) {
- return true;
+ // Could not inline this call but the method might be inlineable at a different call site.
+ return InlineResult.DO_NOT_BLACKLIST;
}
// Run the order check. This verifies that all the parameters are
@@ -363,17 +359,18 @@
* other expressions.
*/
OrderVisitor orderVisitor = new OrderVisitor(x.getTarget().getParams());
- orderVisitor.accept(targetExpr);
+ orderVisitor.accept(bodyAsExpressionList);
/*
* A method that doesn't touch any parameters is trivially inlinable (this
* covers the empty method case)
*/
if (orderVisitor.checkResults() == SideEffectCheck.NO_REFERENCES) {
- JMultiExpression multi = createMultiExpressionIncludingArgs(x);
- multi.addExpressions(targetExpr);
- replaceWithMulti(ctx, multi);
- return true;
+ List<JExpression> expressions = expressionsIncludingArgs(x);
+ expressions.addAll(bodyAsExpressionList);
+ replaceMeAndRecordModification(ctx,
+ JjsUtils.createOptimizedMultiExpression(ignoringReturn, expressions));
+ return InlineResult.DO_NOT_BLACKLIST;
}
/*
@@ -393,22 +390,44 @@
* This argument evaluation could affect or be affected by the
* callee so we cannot inline here.
*/
- return true;
+ // Could not inline this call but the method is potentially inlineable.
+ return InlineResult.DO_NOT_BLACKLIST;
}
}
}
// We're safe to inline.
- JMultiExpression multi = createMultiExpressionForInstanceAndClinit(x);
// Replace all params in the target expression with the actual arguments.
ParameterReplacer replacer = new ParameterReplacer(x);
- replacer.accept(targetExpr);
+ replacer.accept(bodyAsExpressionList);
+ bodyAsExpressionList.add(0, x.getInstance());
+ bodyAsExpressionList.add(1, createClinitCall(x));
+ replaceMeAndRecordModification(ctx, JjsUtils.createOptimizedMultiExpression(ignoringReturn,
+ bodyAsExpressionList));
+ return InlineResult.DO_NOT_BLACKLIST;
+ }
+ }
- multi.addExpressions(targetExpr);
- replaceWithMulti(ctx, multi);
+ private static boolean isTooComplexToInline(List<JExpression> bodyAsExpressionList,
+ boolean ignoringReturn) {
+ /*
+ * Limit inlined methods to multiexpressions of length 2 for now. This
+ * handles the simple { return JVariableRef; } or { expression; return
+ * something; } cases.
+ *
+ * TODO: add an expression complexity analyzer.
+ */
+ if (ignoringReturn) {
+ return bodyAsExpressionList.size() > 2;
+ }
+
+ if (bodyAsExpressionList.size() == 3 && bodyAsExpressionList.get(2).hasSideEffects()) {
return true;
}
+
+ // The expression is effectively of size 2, hence not too complex to inline.
+ return false;
}
/**
@@ -575,11 +594,14 @@
private JReferenceType merge(JReferenceType source, JReferenceType target) {
JReferenceType result;
- if (program.typeOracle.canTriviallyCast(source.getUnderlyingType(), target.getUnderlyingType())) {
+ if (program.typeOracle.canTriviallyCast(
+ source.getUnderlyingType(), target.getUnderlyingType())) {
result = source;
} else {
result = target;
}
return result;
}
+
+ private enum InlineResult { BLACKLIST, DO_NOT_BLACKLIST}
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java b/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
index 7ff1ad6..7ba0e69 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
@@ -37,7 +37,6 @@
import com.google.gwt.dev.jjs.ast.JMethodCall;
import com.google.gwt.dev.jjs.ast.JModVisitor;
import com.google.gwt.dev.jjs.ast.JNewInstance;
-import com.google.gwt.dev.jjs.ast.JNullLiteral;
import com.google.gwt.dev.jjs.ast.JNullType;
import com.google.gwt.dev.jjs.ast.JParameter;
import com.google.gwt.dev.jjs.ast.JParameterRef;
@@ -53,12 +52,13 @@
import com.google.gwt.dev.jjs.ast.JVariable;
import com.google.gwt.dev.jjs.ast.JVariableRef;
import com.google.gwt.dev.jjs.ast.JVisitor;
-import com.google.gwt.dev.jjs.ast.js.JMultiExpression;
import com.google.gwt.dev.jjs.ast.js.JsniFieldRef;
import com.google.gwt.dev.jjs.ast.js.JsniMethodRef;
import com.google.gwt.dev.util.log.speedtracer.CompilerEventType;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.Event;
+import com.google.gwt.thirdparty.guava.common.base.Predicate;
+import com.google.gwt.thirdparty.guava.common.collect.Lists;
import java.util.ArrayList;
import java.util.Collection;
@@ -196,8 +196,9 @@
* nodes onto sets of JReferenceType directly, which meant I had to rerun this
* visitor each time.
*/
- public class RecordVisitor extends JVisitor {
+ private class RecordVisitor extends JVisitor {
private JMethod currentMethod;
+ private Predicate<JField> canUninitializedValueBeObserved;
@Override
public void endVisit(JBinaryOperation x, Context ctx) {
@@ -231,9 +232,11 @@
@Override
public void endVisit(JField x, Context ctx) {
- if (x.getLiteralInitializer() != null) {
- // TODO: do I still need this?
- addAssignment(x, x.getLiteralInitializer());
+ if (x.hasInitializer() && canUninitializedValueBeObserved.apply(x)) {
+ addAssignment(x, x.getType().getDefaultValue());
+ }
+ if (!x.hasInitializer()) {
+ addAssignment(x, x.getType().getDefaultValue());
}
currentMethod = null;
}
@@ -330,6 +333,12 @@
return true;
}
+ public void record(JProgram program) {
+ canUninitializedValueBeObserved = ComputePotentiallyObservableUninitializedValues
+ .analyze(program);
+ accept(program);
+ }
+
private void addAssignment(JVariable target, JExpression rhs) {
add(target, rhs, assignments);
}
@@ -476,20 +485,22 @@
switch (analysisResult) {
case TRUE:
- // replace with a simple null test
- JNullLiteral nullLit = program.getLiteralNull();
- JBinaryOperation neq =
- new JBinaryOperation(x.getSourceInfo(), program.getTypePrimitiveBoolean(),
- JBinaryOperator.NEQ, x.getExpr(), nullLit);
- ctx.replaceMe(neq);
+ if (x.getExpr().getType().canBeNull()) {
+ // replace with a simple null test
+ JBinaryOperation neq =
+ new JBinaryOperation(x.getSourceInfo(), program.getTypePrimitiveBoolean(),
+ JBinaryOperator.NEQ, x.getExpr(), program.getLiteralNull());
+ ctx.replaceMe(neq);
+ } else {
+ ctx.replaceMe(
+ JjsUtils.createOptimizedMultiExpression(x.getExpr(),
+ program.getLiteralBoolean(true)));
+ }
break;
case FALSE:
// replace with a false literal
- JExpression result = program.getLiteralBoolean(false);
- if (x.getExpr().hasSideEffects()) {
- result = new JMultiExpression(x.getSourceInfo(), x.getExpr(), result);
- }
- ctx.replaceMe(result);
+ ctx.replaceMe(
+ JjsUtils.createOptimizedMultiExpression(x.getExpr(), program.getLiteralBoolean(false)));
break;
case UNKNOWN:
default:
@@ -718,19 +729,7 @@
}
// tighten based on assignment
- List<JReferenceType> typeList = new ArrayList<JReferenceType>();
-
- /*
- * For fields that are not compile time constants, add a null assignment, because the
- * field might be accessed before initialized. Technically even a field
- * with an initializer might be accessed before initialization, but
- * presumably that is not the programmer's intent, so the compiler cheats
- * and assumes the initial null will not be seen.
- */
- if (!cannotBeSeenUninitialized(x)) {
- typeList.add(typeNull);
- }
-
+ List<JReferenceType> typeList = Lists.newArrayList();
Collection<JExpression> myAssignments = assignments.get(x);
if (myAssignments != null) {
for (JExpression expr : myAssignments) {
@@ -868,7 +867,7 @@
private OptimizerStats execImpl() {
OptimizerStats stats = new OptimizerStats(NAME);
RecordVisitor recorder = new RecordVisitor();
- recorder.accept(program);
+ recorder.record(program);
/*
* We must iterate multiple times because each way point we tighten creates
diff --git a/dev/core/src/com/google/gwt/dev/util/collect/Stack.java b/dev/core/src/com/google/gwt/dev/util/collect/Stack.java
index e76e0bf..53077af 100644
--- a/dev/core/src/com/google/gwt/dev/util/collect/Stack.java
+++ b/dev/core/src/com/google/gwt/dev/util/collect/Stack.java
@@ -87,4 +87,11 @@
public void push(T value) {
elements.add(value);
}
+
+ /**
+ * Creates a new Stack for type {@code T}.
+ */
+ public static <T> Stack<T> create() {
+ return new Stack<T>();
+ }
}
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/ComputePotentiallyObservableUninitializedValuesTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/ComputePotentiallyObservableUninitializedValuesTest.java
new file mode 100644
index 0000000..ae6bf37
--- /dev/null
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/ComputePotentiallyObservableUninitializedValuesTest.java
@@ -0,0 +1,261 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.dev.jjs.impl;
+
+import com.google.gwt.dev.jjs.ast.JField;
+import com.google.gwt.dev.jjs.ast.JMethod;
+import com.google.gwt.dev.jjs.ast.JProgram;
+import com.google.gwt.thirdparty.guava.common.base.Predicate;
+import com.google.gwt.thirdparty.guava.common.collect.ImmutableList;
+
+/**
+ * Tests {@link ComputePotentiallyObservableUninitializedValues}.
+ */
+public class ComputePotentiallyObservableUninitializedValuesTest extends OptimizerTestBase {
+
+ private boolean runMethodInliner;
+ private boolean runSpecializer;
+
+ @Override
+ public void setUp() throws Exception {
+ runMethodInliner = false;
+ runSpecializer = false;
+ }
+
+ public void testSimpleClass() throws Exception {
+ addSnippetClassDecl(
+ "static class A { ",
+ " int i1 = 1;",
+ " Integer i2 = new Integer(1);",
+ " final int fi1 = 1;",
+ " final Integer fi2 = new Integer(1);",
+ "}");
+ JProgram program = compileSnippet("void", "return;");
+ assertAnalysisCorrect(program, ImmutableList.<String>of(),
+ ImmutableList
+ .of("EntryPoint$A.i1", "EntryPoint$A.i2", "EntryPoint$A.fi1", "EntryPoint$A.fi2"));
+ }
+
+ public void testOnlyFinalLiteralUnobservable() throws Exception {
+ addSnippetClassDecl(
+ "static class A { ",
+ " A() { m(); }",
+ " void m() { }",
+ "}");
+ addSnippetClassDecl(
+ "static class B extends A { ",
+ " int i1 = 1;",
+ " Integer i2 = new Integer(1);",
+ " final int fi1 = 1;",
+ " final Integer fi2 = new Integer(1);",
+ "}");
+ JProgram program = compileSnippet("void", "return;");
+ assertAnalysisCorrect(program,
+ ImmutableList.of("EntryPoint$B.i1", "EntryPoint$B.i2", "EntryPoint$B.fi2"),
+ ImmutableList.of("EntryPoint$B.fi1"));
+ }
+
+ public void testSafeCustomInitializer() throws Exception {
+ addSnippetClassDecl(
+ "static class A { ",
+ " { m(); }",
+ " static void m() { }",
+ "}");
+ addSnippetClassDecl(
+ "static class B extends A { ",
+ " int i1 = 1;",
+ " Integer i2 = new Integer(1);",
+ " final int fi1 = 1;",
+ " final Integer fi2 = new Integer(1);",
+ "}");
+ JProgram program = compileSnippet("void", "return;");
+ assertAnalysisCorrect(program, ImmutableList.<String>of(),
+ ImmutableList
+ .of("EntryPoint$B.fi1", "EntryPoint$B.i1", "EntryPoint$B.i2", "EntryPoint$B.fi2"));
+ MakeCallsStatic.exec(program, false);
+ assertAnalysisCorrect(program, ImmutableList.<String>of(),
+ ImmutableList
+ .of("EntryPoint$B.fi1", "EntryPoint$B.i1", "EntryPoint$B.i2", "EntryPoint$B.fi2"));
+ }
+
+ public void testSafeStaticCall() throws Exception {
+ addSnippetClassDecl(
+ "static class A { ",
+ " A() { m(new A(), 2); }",
+ " static void m(A a, int i) { }",
+ "}");
+ addSnippetClassDecl(
+ "static class B extends A { ",
+ " int i1 = 1;",
+ " Integer i2 = new Integer(1);",
+ " final int fi1 = 1;",
+ " final Integer fi2 = new Integer(1);",
+ "}");
+ JProgram program = compileSnippet("void", "return;");
+ assertAnalysisCorrect(program, ImmutableList.<String>of(),
+ ImmutableList
+ .of("EntryPoint$B.fi1", "EntryPoint$B.i1", "EntryPoint$B.i2", "EntryPoint$B.fi2"));
+ }
+
+ public void testSafePolyCall() throws Exception {
+ addSnippetClassDecl(
+ "static class A { ",
+ " A() { String s = new Integer(0).toString(); }",
+ "}");
+ addSnippetClassDecl(
+ "static class B extends A { ",
+ " int i1 = 1;",
+ " Integer i2 = new Integer(1);",
+ " final int fi1 = 1;",
+ " final Integer fi2 = new Integer(1);",
+ "}");
+ JProgram program = compileSnippet("void", "return;");
+ assertAnalysisCorrect(program, ImmutableList.<String>of(),
+ ImmutableList
+ .of("EntryPoint$B.fi1", "EntryPoint$B.i1", "EntryPoint$B.i2", "EntryPoint$B.fi2"));
+ }
+
+ public void testUnSafeCustomInitializer_polymorphicDispatch() throws Exception {
+ addSnippetClassDecl(
+ "static class A { ",
+ " void m() { }",
+ " { m(); }",
+ "}");
+ addSnippetClassDecl(
+ "static class B extends A { ",
+ " int i1 = 1;",
+ " Integer i2 = new Integer(1);",
+ " final int fi1 = 1;",
+ " final Integer fi2 = new Integer(1);",
+ "}");
+ JProgram program = compileSnippet("void", "return;");
+ assertAnalysisCorrect(program,
+ ImmutableList.of("EntryPoint$B.i1", "EntryPoint$B.i2", "EntryPoint$B.fi2"),
+ ImmutableList.of("EntryPoint$B.fi1"));
+ }
+
+ public void testUnSafeCustomInitializer_escapingThis() throws Exception {
+ addSnippetClassDecl(
+ "static class A { ",
+ " static void m(A a) { }",
+ " { m(this); }",
+ "}");
+ addSnippetClassDecl(
+ "static class B extends A { ",
+ " int i1 = 1;",
+ " Integer i2 = new Integer(1);",
+ " final int fi1 = 1;",
+ " final Integer fi2 = new Integer(1);",
+ "}");
+ JProgram program = compileSnippet("void", "return;");
+ assertAnalysisCorrect(program,
+ ImmutableList.of("EntryPoint$B.i1", "EntryPoint$B.i2", "EntryPoint$B.fi2"),
+ ImmutableList.of("EntryPoint$B.fi1"));
+ }
+
+ public void testUnSafeCustomInitializer_escapingThisThroughArrayInitializer() throws Exception {
+ addSnippetClassDecl(
+ "static class A { ",
+ " { A[] a = new A[] {this}; }",
+ "}");
+ addSnippetClassDecl(
+ "static class B extends A { ",
+ " int i1 = 1;",
+ " Integer i2 = new Integer(1);",
+ " final int fi1 = 1;",
+ " final Integer fi2 = new Integer(1);",
+ "}");
+ JProgram program = compileSnippet("void", "return;");
+ assertAnalysisCorrect(program,
+ ImmutableList.of("EntryPoint$B.i1", "EntryPoint$B.i2", "EntryPoint$B.fi2"),
+ ImmutableList.of("EntryPoint$B.fi1"));
+ }
+
+ public void testUnSafeCustomInitializer_escapingDeepReference() throws Exception {
+ addSnippetClassDecl(
+ "static class A { ",
+ " A() {String s = (new Integer(0).toString() + this.toString()).substring(0, 1); }",
+ "}");
+ addSnippetClassDecl(
+ "static class B extends A { ",
+ " int i1 = 1;",
+ " Integer i2 = new Integer(1);",
+ " final int fi1 = 1;",
+ " final Integer fi2 = new Integer(1);",
+ "}");
+ JProgram program = compileSnippet("void", "return;");
+ assertAnalysisCorrect(program,
+ ImmutableList.of("EntryPoint$B.i1", "EntryPoint$B.i2", "EntryPoint$B.fi2"),
+ ImmutableList.of("EntryPoint$B.fi1"));
+ MakeCallsStatic.exec(program, false); // required so that method is static
+ assertAnalysisCorrect(program,
+ ImmutableList.of("EntryPoint$B.i1", "EntryPoint$B.i2", "EntryPoint$B.fi2"),
+ ImmutableList.of("EntryPoint$B.fi1"));
+ }
+
+ @Override
+ protected boolean optimizeMethod(JProgram program, JMethod method) {
+
+ if (runMethodInliner) {
+ MethodInliner.exec(program);
+ }
+ if (runSpecializer) {
+ Finalizer.exec(program); // required so that method is marked final
+ MakeCallsStatic.exec(program, false); // required so that method is static
+ TypeTightener.exec(program); // required so that the parameter types are tightened
+ MethodCallSpecializer.exec(program);
+ }
+
+ OptimizerStats result = DeadCodeElimination.exec(program, method);
+ if (result.didChange()) {
+ // Make sure we converge in one pass.
+ //
+ // TODO(rluble): It does not appear to be true in general unless we iterate until a
+ // fixpoint in exec().
+ //
+ // Example:
+ //
+ // Constructor( ) { deadcode }
+ // m( new Constructor(); }
+ //
+ // If m is processed first, it will see the constructor as having side effects.
+ // Then the constructor will become empty enabling m() become empty in the next round.
+ //
+ assertFalse(DeadCodeElimination.exec(program, method).didChange());
+ }
+ return result.didChange();
+ }
+
+ private void assertAnalysisCorrect(JProgram program,
+ Iterable<String> fieldsThatCanBeObservedUninitialized,
+ Iterable<String> fieldsThatCannotBeObservedUninitialized) {
+ Predicate<JField> uninitializedValuesCanBeObserved =
+ ComputePotentiallyObservableUninitializedValues.analyze(program);
+
+ for (String fieldName : fieldsThatCanBeObservedUninitialized) {
+ JField field = findField(program, fieldName);
+ assertNotNull(field);
+ assertTrue("Field " + fieldName + " was erroneously determined to uninitialized unobservable",
+ uninitializedValuesCanBeObserved.apply(field));
+ }
+ for (String fieldName : fieldsThatCannotBeObservedUninitialized) {
+ JField field = findField(program, fieldName);
+ assertNotNull(field);
+ assertFalse("Field " + fieldName + " was erroneously determined to uninitialized observable",
+ uninitializedValuesCanBeObserved.apply(field));
+ }
+ }
+}
diff --git a/user/test/com/google/gwt/dev/jjs/test/FieldInitOrderChild.java b/user/test/com/google/gwt/dev/jjs/test/FieldInitOrderChild.java
index 56df745..8281cdf 100644
--- a/user/test/com/google/gwt/dev/jjs/test/FieldInitOrderChild.java
+++ b/user/test/com/google/gwt/dev/jjs/test/FieldInitOrderChild.java
@@ -56,19 +56,8 @@
// i3, i4 and i7 would be directly converted into strings hence show undefined instead of null.
// String operations should take care of corner cases where a string is null or undefined
// see issue 8257.
- seenValues.add("i1=" + i1 + ",i2=" + i2 + ",i3=" + undefinedToNull(i3) +
- ",i4=" + undefinedToNull(i4) + ",i5=" + i5 + ",i6=" + i6
- + ",i7=" + undefinedToNull(i7));
- }
-
- // Convert undefined to null this way {@code undefinedToNull(i)} instead of
- // {@code i == null ? "null" : i} to avoid the shortcomings of our nullness analysis in
- // {@link TypeTightener}, which will infer that i3 is never null.
- private String undefinedToNull(Object o) {
- String s = "" + o;
- if (s.equals("undefined")) {
- return "null";
- }
- return s;
+ seenValues.add("i1=" + i1 + ",i2=" + i2 + ",i3=" + (i3 == null ? null : i3) +
+ ",i4=" + (i4 == null ? null : i4) + ",i5=" + i5 + ",i6=" + i6
+ + ",i7=" + (i7 == null ? null : i7));
}
}
\ No newline at end of file
