Fixes a problem where inlining an JS method with no return statement fails to evaluate to "undefined".
Review by: bobv (postmortem)
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@1558 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/js/JsInliner.java b/dev/core/src/com/google/gwt/dev/js/JsInliner.java
index 801a964..0f94ec2 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsInliner.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsInliner.java
@@ -577,8 +577,8 @@
* configured to collect idents from qualified xor unqualified JsNameRefs.
*/
private static class IdentCollector extends JsVisitor {
- private final Set<String> idents = new HashSet<String>();
private final boolean collectQualified;
+ private final Set<String> idents = new HashSet<String>();
public IdentCollector(boolean collectQualified) {
this.collectQualified = collectQualified;
@@ -610,8 +610,8 @@
* statements if the context of the invocation would allow this.
*/
private static class InliningVisitor extends JsModVisitor {
- private final Stack<JsFunction> functionStack = new Stack<JsFunction>();
private final Set<JsFunction> blacklist = new HashSet<JsFunction>();
+ private final Stack<JsFunction> functionStack = new Stack<JsFunction>();
private final JsProgram program;
public InliningVisitor(JsProgram program) {
@@ -737,6 +737,7 @@
List<JsExpression> hoisted = new ArrayList<JsExpression>(
statements.size());
+ boolean sawReturnStatement = false;
for (JsStatement statement : statements) {
/*
* Create replacement expressions to use in place of the original
@@ -753,41 +754,37 @@
hoisted.add(h);
- if (hoistedExpressionTerminal(statement)) {
+ if (isReturnStatement(statement)) {
+ sawReturnStatement = true;
break;
}
}
/*
- * Determine the expressions that will be used to construct the
- * replacement expression.
+ * If the inlined method has no return statement, synthesize an undefined
+ * reference. It will be reclaimed if the method call is from a
+ * JsExprStmt.
*/
- JsExpression op;
+ if (!sawReturnStatement) {
+ hoisted.add(program.getUndefinedLiteral());
+ }
- if (hoisted.size() == 0) {
- /*
- * There are no expressions evaluated in the target function, so we'll
- * try to replace the invocation with nothing. The null literal may be
- * reclaimed during the JsExprStmt cleanup pass.
- */
- op = program.getNullLiteral();
+ assert (hoisted.size() > 0);
- } else {
- /*
- * Build up the new comma expression from right-to-left; building the
- * rightmost comma expressions first. Bootstrapping with i.previous()
- * ensures that this logic will function correctly in the case of a
- * single expression.
- */
- ListIterator<JsExpression> i = hoisted.listIterator(hoisted.size());
- op = i.previous();
- while (i.hasPrevious()) {
- JsBinaryOperation outerOp = new JsBinaryOperation(
- JsBinaryOperator.COMMA);
- outerOp.setArg1(i.previous());
- outerOp.setArg2(op);
- op = outerOp;
- }
+ /*
+ * Build up the new comma expression from right-to-left; building the
+ * rightmost comma expressions first. Bootstrapping with i.previous()
+ * ensures that this logic will function correctly in the case of a single
+ * expression.
+ */
+ ListIterator<JsExpression> i = hoisted.listIterator(hoisted.size());
+ JsExpression op = i.previous();
+ while (i.hasPrevious()) {
+ JsBinaryOperation outerOp = new JsBinaryOperation(
+ JsBinaryOperator.COMMA);
+ outerOp.setArg1(i.previous());
+ outerOp.setArg2(op);
+ op = outerOp;
}
// Confirm that the expression conforms to the desired heuristics
@@ -890,8 +887,8 @@
* Generally speaking, we disallow the use of parameters as lvalues.
*/
private static class ParameterUsageVisitor extends JsVisitor {
- private final Set<JsName> parameterNames;
private boolean lvalue = false;
+ private final Set<JsName> parameterNames;
public ParameterUsageVisitor(Set<JsName> parameterNames) {
this.parameterNames = parameterNames;
@@ -1008,8 +1005,8 @@
* the lifetime of the program.
*/
private static class RedefinedFunctionCollector extends JsVisitor {
- private final Set<JsFunction> redefined = new HashSet<JsFunction>();
private final Map<JsName, JsFunction> nameMap = new IdentityHashMap<JsName, JsFunction>();
+ private final Set<JsFunction> redefined = new HashSet<JsFunction>();
/**
* Look for assignments to JsNames whose static references are JsFunctions.
@@ -1157,8 +1154,8 @@
* identifiers to resolve to different values.
*/
private static class StableNameChecker extends JsVisitor {
- private final JsScope callerScope;
private final JsScope calleeScope;
+ private final JsScope callerScope;
private final Collection<JsName> parameterNames;
private boolean stable = true;
@@ -1205,15 +1202,6 @@
}
/**
- * When attempting to inline an invocation, this constant determines the
- * maximum allowable ratio of potential inlined complexity to initial
- * complexity. This acts as a brake on very large expansions from bloating the
- * the generated output. Increasing this number will allow larger sections of
- * code to be inlined, but at a cost of larger JS output.
- */
- private static final int MAX_COMPLEXITY_INCREASE = 10;
-
- /**
* A List of expression types that are known to never be affected by
* side-effects. Used by {@link #alwaysFlexible(JsExpression)}.
*/
@@ -1223,6 +1211,15 @@
JsThisRef.class});
/**
+ * When attempting to inline an invocation, this constant determines the
+ * maximum allowable ratio of potential inlined complexity to initial
+ * complexity. This acts as a brake on very large expansions from bloating the
+ * the generated output. Increasing this number will allow larger sections of
+ * code to be inlined, but at a cost of larger JS output.
+ */
+ private static final int MAX_COMPLEXITY_INCREASE = 5;
+
+ /**
* Static entry point used by JavaToJavaScriptCompiler.
*/
public static boolean exec(JsProgram program) {
@@ -1328,13 +1325,10 @@
if (statement instanceof JsExprStmt) {
JsExprStmt exprStmt = (JsExprStmt) statement;
expression = exprStmt.getExpression();
-
} else if (statement instanceof JsReturn) {
JsReturn ret = (JsReturn) statement;
expression = ret.getExpr();
-
} else {
- // TODO If additional statements are supported, update hETerminal()
return null;
}
@@ -1342,16 +1336,6 @@
}
/**
- * This is used in combination with {@link #hoistedExpression(JsStatement)} to
- * indicate if a given statement would terminate the list of hoisted
- * expressions.
- */
- private static boolean hoistedExpressionTerminal(JsStatement statement) {
- // TODO keep this in synch with hoistedExpression
- return statement instanceof JsReturn;
- }
-
- /**
* Given a JsInvocation, determine if it is invoking a JsFunction that is
* specified to be executed only once during the program's lifetime.
*/
@@ -1475,6 +1459,15 @@
}
/**
+ * This is used in combination with {@link #hoistedExpression(JsStatement)} to
+ * indicate if a given statement would terminate the list of hoisted
+ * expressions.
+ */
+ private static boolean isReturnStatement(JsStatement statement) {
+ return statement instanceof JsReturn;
+ }
+
+ /**
* Utility class.
*/
private JsInliner() {
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsProgram.java b/dev/core/src/com/google/gwt/dev/js/ast/JsProgram.java
index a8ca1bf..9c222b1 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsProgram.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsProgram.java
@@ -139,6 +139,10 @@
return trueLiteral;
}
+ public JsNameRef getUndefinedLiteral() {
+ return rootScope.findExistingName("undefined").makeRef();
+ }
+
public void traverse(JsVisitor v, JsContext<JsProgram> ctx) {
if (v.visit(this, ctx)) {
v.accept(globalBlock);
