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 { {