Prevent JsInliner from triggering infinite expansion.
Now that JsStaticEval will turn "if (blah) {a()}" into "blah && a()" the functions can then be inlined by JsInliner. There was already code to blacklist self-recursive functions, but there was no code to detect a function that has become self-recursive during the inlining process.
Patch by: bobv
Review by: scottb
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@5585 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 f3e974c..1379f8a 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsInliner.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsInliner.java
@@ -728,11 +728,10 @@
private final JsProgram program;
/**
- * A map containing the next integer to try as an identifier suffix for
- * a given JsScope.
+ * A map containing the next integer to try as an identifier suffix for a
+ * given JsScope.
*/
- private IdentityHashMap<JsScope,HashMap<String,Integer>> startIdentForScope
- = new IdentityHashMap<JsScope, HashMap<String,Integer>>();
+ private IdentityHashMap<JsScope, HashMap<String, Integer>> startIdentForScope = new IdentityHashMap<JsScope, HashMap<String, Integer>>();
/**
* Not a stack because program fragments aren't nested.
@@ -851,6 +850,7 @@
if (functionStack.isEmpty()) {
return;
}
+ JsFunction callerFunction = functionStack.peek();
/*
* We only want to look at invocations of things that we statically know
@@ -869,6 +869,15 @@
return;
}
+ /*
+ * The current function has been mutated so as to be self-recursive. Ban
+ * it from any future inlining to prevent infinite expansion.
+ */
+ if (f == callerFunction) {
+ blacklist.add(f);
+ return;
+ }
+
List<JsStatement> statements;
if (f.getBody() != null) {
statements = new ArrayList<JsStatement>(f.getBody().getStatements());
@@ -954,7 +963,7 @@
}
// Confirm that the expression conforms to the desired heuristics
- if (!isInlinable(program, functionStack.peek(), x, f, op)) {
+ if (!isInlinable(program, callerFunction, f, x.getArguments(), op)) {
return;
}
@@ -970,13 +979,13 @@
*/
String ident;
String base = f.getName() + "_" + name.getIdent();
- JsScope scope = functionStack.peek().getScope();
- HashMap<String,Integer> startIdent = startIdentForScope.get(scope);
+ JsScope scope = callerFunction.getScope();
+ HashMap<String, Integer> startIdent = startIdentForScope.get(scope);
if (startIdent == null) {
- startIdent = new HashMap<String,Integer>();
+ startIdent = new HashMap<String, Integer>();
startIdentForScope.put(scope, startIdent);
}
-
+
Integer s = startIdent.get(base);
int suffix = (s == null) ? 0 : s.intValue();
do {
@@ -1004,8 +1013,7 @@
return;
}
- if (functionStack.peek() == programFunction
- && localVariableNames.size() > 0) {
+ if (callerFunction == programFunction && localVariableNames.size() > 0) {
// Don't add additional variables to the top-level program.
return;
}
@@ -1023,7 +1031,6 @@
* total number of passes required to finalize the AST.
*/
op = accept(op);
-
ctx.replaceMe(op);
}
@@ -1704,8 +1711,7 @@
* Determine if a statement can be inlined into a call site.
*/
private static boolean isInlinable(JsProgram program, JsFunction caller,
- JsInvocation invocation, JsFunction callee, JsNode<?> toInline) {
- List<JsExpression> arguments = invocation.getArguments();
+ JsFunction callee, List<JsExpression> arguments, JsNode<?> toInline) {
/*
* This will happen with varargs-style JavaScript functions that rely on the
diff --git a/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java b/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
new file mode 100644
index 0000000..bcbe315
--- /dev/null
+++ b/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2009 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.js;
+
+import com.google.gwt.dev.js.ast.JsContext;
+import com.google.gwt.dev.js.ast.JsExpression;
+import com.google.gwt.dev.js.ast.JsFunction;
+import com.google.gwt.dev.js.ast.JsModVisitor;
+import com.google.gwt.dev.js.ast.JsName;
+import com.google.gwt.dev.js.ast.JsProgram;
+
+/**
+ * Safety checks for JsInliner.
+ */
+public class JsInlinerTest extends OptimizerTestBase {
+
+ public void testSelfRecursion() throws Exception {
+ String input = "function a1() { return blah && b1() }"
+ + "function b1() { return bar && a1()}" + "function c() { a1() } c()";
+ input = optimize(input, JsSymbolResolver.class, FixStaticRefsVisitor.class,
+ JsInliner.class, JsUnusedFunctionRemover.class);
+
+ String expected = "function a1() { return blah && bar && a1() }"
+ + "function c() { a1() } c()";
+ expected = optimize(expected);
+ assertEquals(expected, input);
+ }
+
+ private static class FixStaticRefsVisitor extends JsModVisitor {
+ @Override
+ public void endVisit(JsFunction x, JsContext<JsExpression> ctx) {
+ JsName name = x.getName();
+ name.setStaticRef(x);
+ }
+
+ public static void exec(JsProgram program) {
+ (new FixStaticRefsVisitor()).accept(program);
+ }
+ }
+}
diff --git a/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java b/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java
index 7f7ba74..271f174 100644
--- a/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java
+++ b/dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java
@@ -15,19 +15,10 @@
*/
package com.google.gwt.dev.js;
-import com.google.gwt.dev.jjs.SourceOrigin;
-import com.google.gwt.dev.js.ast.JsProgram;
-import com.google.gwt.dev.js.ast.JsStatement;
-import com.google.gwt.dev.js.ast.JsVisitor;
-import com.google.gwt.dev.util.DefaultTextOutput;
-import com.google.gwt.dev.util.TextOutput;
-
-import junit.framework.TestCase;
-
-import java.io.StringReader;
-import java.util.List;
-
-public class JsStaticEvalTest extends TestCase {
+/**
+ * Tests the JsStaticEval optimizer.
+ */
+public class JsStaticEvalTest extends OptimizerTestBase {
public void testIfWithEmptyThen() throws Exception {
assertEquals("a();", optimize("if (a()) { }"));
@@ -81,18 +72,6 @@
}
private String optimize(String js) throws Exception {
- JsProgram program = new JsProgram();
- List<JsStatement> expected = JsParser.parse(SourceOrigin.UNKNOWN,
- program.getScope(), new StringReader(js));
- program.getGlobalBlock().getStatements().addAll(expected);
-
- // Run the static evaluation over this new program
- JsStaticEval.exec(program);
-
- TextOutput text = new DefaultTextOutput(true);
- JsVisitor generator = new JsSourceGenerationVisitor(text);
-
- generator.accept(program);
- return text.toString();
+ return optimize(js, JsStaticEval.class);
}
}
diff --git a/dev/core/test/com/google/gwt/dev/js/OptimizerTestBase.java b/dev/core/test/com/google/gwt/dev/js/OptimizerTestBase.java
new file mode 100644
index 0000000..e04b62a
--- /dev/null
+++ b/dev/core/test/com/google/gwt/dev/js/OptimizerTestBase.java
@@ -0,0 +1,62 @@
+/*
+ * Copyright 2009 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.js;
+
+import com.google.gwt.dev.jjs.SourceOrigin;
+import com.google.gwt.dev.js.ast.JsProgram;
+import com.google.gwt.dev.js.ast.JsStatement;
+import com.google.gwt.dev.js.ast.JsVisitor;
+import com.google.gwt.dev.util.DefaultTextOutput;
+import com.google.gwt.dev.util.TextOutput;
+
+import junit.framework.TestCase;
+
+import java.io.StringReader;
+import java.lang.reflect.Method;
+import java.util.List;
+
+/**
+ * A utility base type for writing tests for JS optimizers.
+ */
+public abstract class OptimizerTestBase extends TestCase {
+
+ /**
+ * Optimize a JS program.
+ *
+ * @param js the source program
+ * @param toExec a list of classes that implement
+ * <code>static void exec(JsProgram)</code>
+ * @return optimized JS
+ */
+ protected String optimize(String js, Class<?>... toExec) throws Exception {
+ JsProgram program = new JsProgram();
+ List<JsStatement> expected = JsParser.parse(SourceOrigin.UNKNOWN,
+ program.getScope(), new StringReader(js));
+
+ program.getGlobalBlock().getStatements().addAll(expected);
+
+ for (Class<?> clazz : toExec) {
+ Method m = clazz.getMethod("exec", JsProgram.class);
+ m.invoke(null, program);
+ }
+
+ TextOutput text = new DefaultTextOutput(true);
+ JsVisitor generator = new JsSourceGenerationVisitor(text);
+
+ generator.accept(program);
+ return text.toString();
+ }
+}
\ No newline at end of file
