Optimizes LongFromJSNIChecker's execution time significantly. An initial sloppy pass of FindJSNIRefs quickly checks for problems; only if problems are found is a strict parse done.
Patch by: spoon
Review by: me
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@2611 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jdt/FindJsniRefVisitor.java b/dev/core/src/com/google/gwt/dev/jdt/FindJsniRefVisitor.java
index 908d25a..e9a1e5b 100644
--- a/dev/core/src/com/google/gwt/dev/jdt/FindJsniRefVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/jdt/FindJsniRefVisitor.java
@@ -24,6 +24,7 @@
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.js.rhino.TokenStream;
import com.google.gwt.dev.util.Jsni;
import org.eclipse.jdt.internal.compiler.ASTVisitor;
@@ -34,21 +35,23 @@
import java.io.IOException;
import java.io.StringReader;
import java.util.Collections;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
/**
* Walks the AST to find references to Java identifiers from within JSNI blocks.
- * By default, it only records the class names referenced. If
- * {@link #justRecordClasses(boolean)} is called with an argument of
- * <code>false</code>, then full references to fields or methods are
- * recorded.
+ * By default, it does a full JavaScript parse to accurately find JSNI
+ * references. If {@link #beSloppy()} is called, then it will run much more
+ * quickly but it will return a superset of the actual JSNI references.
*/
public class FindJsniRefVisitor extends ASTVisitor {
- private final Set<String> jsniRefs = new HashSet<String>();
- private final JsParser jsParser = new JsParser();
- private final JsProgram jsProgram = new JsProgram();
+ private final Set<String> jsniRefs = new LinkedHashSet<String>();
+ private boolean sloppy = false;
+
+ public void beSloppy() {
+ sloppy = true;
+ }
public Set<String> getJsniRefs() {
return Collections.unmodifiableSet(jsniRefs);
@@ -60,19 +63,28 @@
}
// Handle JSNI block
- char[] source = methodDeclaration.compilationResult().getCompilationUnit().getContents();
- String jsniCode = String.valueOf(source, methodDeclaration.bodyStart,
- methodDeclaration.bodyEnd - methodDeclaration.bodyStart + 1);
- int startPos = jsniCode.indexOf(Jsni.JSNI_BLOCK_START);
- int endPos = jsniCode.lastIndexOf(Jsni.JSNI_BLOCK_END);
- if (startPos < 0 || endPos < 0) {
- return false; // ignore the error
+ String jsniCode = getJSNICode(methodDeclaration);
+ if (jsniCode == null) {
+ return false;
+ }
+ if (jsniCode.indexOf('@') < 0) {
+ // short cut: if there are no at signs, there are no JSNI refs
+ return false;
}
- startPos += Jsni.JSNI_BLOCK_START.length() - 1; // move up to open brace
- endPos += 1; // move past close brace
+ if (sloppy) {
+ findJsniRefsSloppily(jsniCode);
+ } else {
+ findJsniRefsAccurately(methodDeclaration, jsniCode);
+ }
- jsniCode = jsniCode.substring(startPos, endPos);
+ return false;
+ }
+
+ private void findJsniRefsAccurately(MethodDeclaration methodDeclaration,
+ String jsniCode) throws InternalCompilerException {
+ JsParser jsParser = new JsParser();
+ JsProgram jsProgram = new JsProgram();
String syntheticFnHeader = "function(";
boolean first = true;
@@ -91,7 +103,7 @@
StringReader sr = new StringReader(syntheticFnHeader + '\n' + jsniCode);
try {
// start at -1 to avoid counting our synthetic header
- List<JsStatement> result = jsParser.parse(jsProgram.getScope(), sr, -1);
+ List<JsStatement> result = jsParser.parse(jsProgram.getScope(), sr, -1);
new JsVisitor() {
public void endVisit(JsNameRef x, JsContext<JsExpression> ctx) {
String ident = x.getIdent();
@@ -106,8 +118,50 @@
} catch (JsParserException e) {
// ignore, we only care about finding valid references
}
+ }
- return false;
+ private void findJsniRefsSloppily(String jsniCode) {
+ StringReader reader = new StringReader(jsniCode);
+ int idx = 0;
+ while (true) {
+ idx = jsniCode.indexOf('@', idx);
+ if (idx < 0) {
+ break;
+ }
+ try {
+ // use Rhino to read a JavaScript identifier
+ reader.reset();
+ reader.skip(idx);
+ TokenStream tokStr = new TokenStream(reader, "(memory)", 0);
+ if (tokStr.getToken() != TokenStream.NAME) {
+ // scottb: Why do we break here?
+ break;
+ }
+ String jsniRefString = tokStr.getString();
+ assert (jsniRefString.charAt(0) == '@');
+ jsniRefs.add(jsniRefString.substring(1));
+ idx += jsniRefString.length();
+ } catch (IOException e) {
+ throw new InternalCompilerException("Unexpected IO error while reading a JS identifier", e);
+ }
+ }
+ }
+
+ private String getJSNICode(MethodDeclaration methodDeclaration) {
+ char[] source = methodDeclaration.compilationResult().getCompilationUnit().getContents();
+ String jsniCode = String.valueOf(source, methodDeclaration.bodyStart,
+ methodDeclaration.bodyEnd - methodDeclaration.bodyStart + 1);
+ int startPos = jsniCode.indexOf(Jsni.JSNI_BLOCK_START);
+ int endPos = jsniCode.lastIndexOf(Jsni.JSNI_BLOCK_END);
+ if (startPos < 0 || endPos < 0) {
+ return null; // ignore the error
+ }
+
+ startPos += Jsni.JSNI_BLOCK_START.length() - 1; // move up to open brace
+ endPos += 1; // move past close brace
+
+ jsniCode = jsniCode.substring(startPos, endPos);
+ return jsniCode;
}
}
diff --git a/dev/core/src/com/google/gwt/dev/jdt/LongFromJSNIChecker.java b/dev/core/src/com/google/gwt/dev/jdt/LongFromJSNIChecker.java
index 0ad43b3..3e2f259 100644
--- a/dev/core/src/com/google/gwt/dev/jdt/LongFromJSNIChecker.java
+++ b/dev/core/src/com/google/gwt/dev/jdt/LongFromJSNIChecker.java
@@ -38,6 +38,9 @@
import org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.TypeIds;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.Map;
import java.util.Set;
/**
@@ -78,27 +81,27 @@
}
}
- private void checkFieldRef(MethodDeclaration meth, JsniRef jsniRef) {
+ private void checkFieldRef(MethodDeclaration meth, JsniRef jsniRef, Set<String> errors) {
assert jsniRef.isField();
FieldBinding target = getField(jsniRef);
if (target == null) {
return;
}
if (containsLong(target.type)) {
- longAccessError(meth, "Referencing field '" + jsniRef.className() + "."
+ errors.add("Referencing field '" + jsniRef.className() + "."
+ jsniRef.memberName() + "': type '" + typeString(target.type)
+ "' is not safe to access in JSNI code");
}
}
- private void checkMethodRef(MethodDeclaration meth, JsniRef jsniRef) {
+ private void checkMethodRef(MethodDeclaration meth, JsniRef jsniRef, Set<String> errors) {
assert jsniRef.isMethod();
MethodBinding target = getMethod(jsniRef);
if (target == null) {
return;
}
if (containsLong(target.returnType)) {
- longAccessError(meth, "Referencing method '" + jsniRef.className()
+ errors.add("Referencing method '" + jsniRef.className()
+ "." + jsniRef.memberName() + "': return type '"
+ typeString(target.returnType)
+ "' is not safe to access in JSNI code");
@@ -110,7 +113,7 @@
++i;
if (containsLong(paramType)) {
// It would be nice to print the parameter name, but how to find it?
- longAccessError(meth, "Parameter " + i + " of method '"
+ errors.add("Parameter " + i + " of method '"
+ jsniRef.className() + "." + jsniRef.memberName()
+ "': type '" + typeString(paramType)
+ "' may not be passed out of JSNI code");
@@ -120,17 +123,38 @@
}
private void checkRefs(MethodDeclaration meth, ClassScope scope) {
- FindJsniRefVisitor jsniRefsVisitor = new FindJsniRefVisitor();
- meth.traverse(jsniRefsVisitor, scope);
- Set<String> jsniRefs = jsniRefsVisitor.getJsniRefs();
+ Map<String, Set<String>> errors = new LinkedHashMap<String, Set<String>>();
- for (String jsniRefString : jsniRefs) {
+ // first do a sloppy parse, for speed
+
+ FindJsniRefVisitor sloppyRefsVisitor = new FindJsniRefVisitor();
+ sloppyRefsVisitor.beSloppy();
+ meth.traverse(sloppyRefsVisitor, scope);
+
+ for (String jsniRefString : sloppyRefsVisitor.getJsniRefs()) {
JsniRef jsniRef = JsniRef.parse(jsniRefString);
if (jsniRef != null) {
+ Set<String> refErrors = new LinkedHashSet<String>();
if (jsniRef.isMethod()) {
- checkMethodRef(meth, jsniRef);
+ checkMethodRef(meth, jsniRef, refErrors);
} else {
- checkFieldRef(meth, jsniRef);
+ checkFieldRef(meth, jsniRef, refErrors);
+ }
+ if (!refErrors.isEmpty()) {
+ errors.put(jsniRefString, refErrors);
+ }
+ }
+ }
+
+ if (!errors.isEmpty()) {
+ // do a strict parse to find out which JSNI refs are real
+ FindJsniRefVisitor jsniRefsVisitor = new FindJsniRefVisitor();
+ meth.traverse(jsniRefsVisitor, scope);
+ for (String jsniRefString : jsniRefsVisitor.getJsniRefs()) {
+ if (errors.containsKey(jsniRefString)) {
+ for (String err : errors.get(jsniRefString)) {
+ longAccessError(meth, err);
+ }
}
}
}
diff --git a/dev/core/test/com/google/gwt/dev/jdt/LongFromJSNITest.java b/dev/core/test/com/google/gwt/dev/jdt/LongFromJSNITest.java
index d11efd4..2412ad2 100644
--- a/dev/core/test/com/google/gwt/dev/jdt/LongFromJSNITest.java
+++ b/dev/core/test/com/google/gwt/dev/jdt/LongFromJSNITest.java
@@ -204,6 +204,19 @@
shouldGenerateNoError(code);
}
}
+
+ public void testRefInString() throws UnableToCompleteException {
+ {
+ StringBuffer code = new StringBuffer();
+ code.append("import com.google.gwt.core.client.UnsafeNativeLong;");
+ code.append("class Buggy {\n");
+ code.append(" void print(long x) { }\n");
+ code.append(" native void jsniMeth() /*-{ 'this.@Buggy::print(J)(0)'; }-*/;\n");
+ code.append("}\n");
+
+ shouldGenerateNoError(code);
+ }
+ }
public void testViolator() throws UnableToCompleteException {
{
