JSNI ::new() now targets constructors directly.
This patch removes the extra hop through a synthetic static 'new' function that is currently used for implementing JSNI ::new() invocations.
In the 99% case, the JsInvocation is replaced with a JsNew operation on the target constructor. In rare cases where the constructor is not immediately invoked, a tear off function is creation which performs the new op internally.
Review by: tobyr@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@8366 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JVisitor.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JVisitor.java
index 8ecc9a5..c31c2f2 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JVisitor.java
@@ -698,7 +698,7 @@
}
public boolean visit(JsniMethodRef x, Context ctx) {
- /* NOTE: Skip JMethodRef */
+ /* NOTE: Skip JMethodCall */
return visit((JExpression) x, ctx);
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/BuildTypeMap.java b/dev/core/src/com/google/gwt/dev/jjs/impl/BuildTypeMap.java
index f81072f..7182bf3 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/BuildTypeMap.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/BuildTypeMap.java
@@ -28,13 +28,10 @@
import com.google.gwt.dev.jjs.ast.JLocal;
import com.google.gwt.dev.jjs.ast.JMethod;
import com.google.gwt.dev.jjs.ast.JMethodBody;
-import com.google.gwt.dev.jjs.ast.JNewInstance;
import com.google.gwt.dev.jjs.ast.JParameter;
-import com.google.gwt.dev.jjs.ast.JParameterRef;
import com.google.gwt.dev.jjs.ast.JPrimitiveType;
import com.google.gwt.dev.jjs.ast.JProgram;
import com.google.gwt.dev.jjs.ast.JReferenceType;
-import com.google.gwt.dev.jjs.ast.JReturnStatement;
import com.google.gwt.dev.jjs.ast.JType;
import com.google.gwt.dev.jjs.ast.JField.Disposition;
import com.google.gwt.dev.jjs.ast.js.JsniMethodBody;
@@ -219,13 +216,6 @@
}
typeMap.put(b, newCtor);
-
- // Now let's implicitly create a static function called 'new' that will
- // allow construction from JSNI methods
- if (!enclosingType.isAbstract()) {
- createSyntheticConstructor(newCtor);
- }
-
return true;
} catch (Throwable e) {
throw translateException(ctorDecl, e);
@@ -388,63 +378,6 @@
return param;
}
- /**
- * Create a method that invokes the specified constructor. This is done as
- * an aid to JSNI users to be able to invoke a Java constructor via a method
- * named ::new.
- *
- * @param constructor the constructor to invoke
- * @param staticClass indicates if the class being constructed is static
- * @param enclosingType the type that encloses the type that is to be
- * constructed. This may be <code>null</code> if the class is a
- * top-level type.
- */
- private JMethod createSyntheticConstructor(JConstructor constructor) {
- JClassType type = constructor.getEnclosingType();
-
- // Define the method
- JMethod synthetic = program.createMethod(type.getSourceInfo().makeChild(
- BuildDeclMapVisitor.class, "Synthetic constructor"), "new", type,
- program.getNonNullType(type), false, true, true, false, false);
- synthetic.setSynthetic();
-
- synthetic.addThrownExceptions(constructor.getThrownExceptions());
-
- // new Foo() : Create the instance
- JNewInstance newInstance = new JNewInstance(
- type.getSourceInfo().makeChild(BuildDeclMapVisitor.class,
- "new instance"), constructor, type);
-
- /*
- * In one pass, add the parameters to the synthetic constructor and
- * arguments to the method call.
- */
- for (JParameter param : constructor.getParams()) {
- JParameter syntheticParam = JProgram.createParameter(
- synthetic.getSourceInfo().makeChild(BuildDeclMapVisitor.class,
- "Argument " + param.getName()), param.getName(),
- param.getType(), true, false, synthetic);
- newInstance.addArg(new JParameterRef(
- syntheticParam.getSourceInfo().makeChild(BuildDeclMapVisitor.class,
- "reference"), syntheticParam));
- }
-
- // Lock the method.
- synthetic.freezeParamTypes();
-
- // return new Foo() : The only statement in the function
- JReturnStatement ret = new JReturnStatement(
- synthetic.getSourceInfo().makeChild(BuildDeclMapVisitor.class,
- "Return statement"), newInstance);
-
- // Add the return statement to the method body
- JMethodBody body = (JMethodBody) synthetic.getBody();
- body.getBlock().addStmt(ret);
-
- // Done
- return synthetic;
- }
-
private JMethodBody findEnclosingMethod(BlockScope scope) {
JMethod method;
MethodScope methodScope = scope.methodScope();
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
index 408e2e0..c54842e 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
@@ -23,6 +23,7 @@
import com.google.gwt.dev.jjs.ast.JCastOperation;
import com.google.gwt.dev.jjs.ast.JClassLiteral;
import com.google.gwt.dev.jjs.ast.JClassType;
+import com.google.gwt.dev.jjs.ast.JConstructor;
import com.google.gwt.dev.jjs.ast.JDeclarationStatement;
import com.google.gwt.dev.jjs.ast.JDeclaredType;
import com.google.gwt.dev.jjs.ast.JExpression;
@@ -367,7 +368,7 @@
@Override
public boolean visit(JNewInstance x, Context ctx) {
// rescue and instantiate the target class!
- rescue(x.getClassType(), true, true);
+ rescueAndInstantiate(x.getClassType());
return super.visit(x, ctx);
}
@@ -420,7 +421,12 @@
*/
rescue(param);
}
- // JsniMethodRef rescues as JMethodCall
+ // JsniMethodRef rescues as a JMethodCall
+ if (x.getTarget() instanceof JConstructor) {
+ // But if a constructor is targeted, there is an implicit 'new' op.
+ JConstructor ctor = (JConstructor) x.getTarget();
+ rescueAndInstantiate(ctor.getEnclosingType());
+ }
return visit((JMethodCall) x, ctx);
}
@@ -604,6 +610,10 @@
}
}
+ private void rescueAndInstantiate(JClassType type) {
+ rescue(type, true, true);
+ }
+
/**
* Handle special rescues needed implicitly to support concat.
*/
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
index 1075083..11970fc 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
@@ -2953,11 +2953,11 @@
+ jsoImplType.getName()
+ "'. Use a stronger type in the JSNI "
+ "identifier or a Java trampoline method.");
- } else if (method.isStatic() && nameRef.getQualifier() != null) {
+ } else if (!method.needsVtable() && nameRef.getQualifier() != null) {
JsniCollector.reportJsniError(info, methodDecl,
"Cannot make a qualified reference to the static method "
+ method.getName());
- } else if (!method.isStatic() && nameRef.getQualifier() == null) {
+ } else if (method.needsVtable() && nameRef.getQualifier() == null) {
JsniCollector.reportJsniError(info, methodDecl,
"Cannot make an unqualified reference to the instance method "
+ method.getName());
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 1c5e94c..c5c4242 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
@@ -1323,15 +1323,41 @@
@Override
public boolean visit(JsniMethodBody x, Context ctx) {
- JsFunction jsFunc = x.getFunc();
+ final JsFunction jsFunc = x.getFunc();
// replace all JSNI idents with a real JsName now that we know it
new JsModVisitor() {
+ /**
+ * Marks a ctor that is a direct child of an invocation. Instead of
+ * replacing the ctor with a tear-off, we replace the invocation with a
+ * new operation.
+ */
+ private JsNameRef dontReplaceCtor;
+
+ public void endVisit(JsInvocation x, JsContext<JsExpression> ctx) {
+ // Replace invocation to ctor with a new op.
+ if (x.getQualifier() instanceof JsNameRef) {
+ JsNameRef ref = (JsNameRef) x.getQualifier();
+ String ident = ref.getIdent();
+ if (isJsniIdent(ident)) {
+ HasEnclosingType node = program.jsniMap.get(ident);
+ assert node instanceof JConstructor;
+ assert ref.getQualifier() == null;
+ JsName jsName = names.get(node);
+ assert (jsName != null);
+ ref.resolve(jsName);
+ JsNew jsNew = new JsNew(x.getSourceInfo(), ref);
+ jsNew.getArguments().addAll(x.getArguments());
+ ctx.replaceMe(jsNew);
+ }
+ }
+ }
+
@Override
public void endVisit(JsNameRef x, JsContext<JsExpression> ctx) {
String ident = x.getIdent();
- if (ident.charAt(0) == '@') {
+ if (isJsniIdent(ident)) {
HasEnclosingType node = program.jsniMap.get(ident);
assert (node != null);
if (node instanceof JField) {
@@ -1346,6 +1372,31 @@
JsExpression commaExpr = createCommaExpression(clinitCall, x);
ctx.replaceMe(commaExpr);
}
+ } else if (node instanceof JConstructor) {
+ if (x == dontReplaceCtor) {
+ // Do nothing, parent will handle.
+ } else {
+ // Replace with a local closure function.
+ // function(a,b,c){return new Obj(a,b,c);}
+ JConstructor ctor = (JConstructor) node;
+ JsName jsName = names.get(ctor);
+ assert (jsName != null);
+ x.resolve(jsName);
+ SourceInfo info = x.getSourceInfo();
+ JsFunction closureFunc = new JsFunction(info, jsFunc.getScope());
+ for (JParameter p : ctor.getParams()) {
+ JsName name = closureFunc.getScope().declareName(p.getName());
+ closureFunc.getParameters().add(new JsParameter(info, name));
+ }
+ JsNew jsNew = new JsNew(info, x);
+ for (JsParameter p : closureFunc.getParameters()) {
+ jsNew.getArguments().add(p.getName().makeRef(info));
+ }
+ JsBlock block = new JsBlock(info);
+ block.getStatements().add(new JsReturn(info, jsNew));
+ closureFunc.setBody(block);
+ ctx.replaceMe(closureFunc);
+ }
} else {
JMethod method = (JMethod) node;
if (x.getQualifier() == null) {
@@ -1364,6 +1415,17 @@
}
}
}
+
+ public boolean visit(JsInvocation x, JsContext<JsExpression> ctx) {
+ if (x.getQualifier() instanceof JsNameRef) {
+ dontReplaceCtor = (JsNameRef) x.getQualifier();
+ }
+ return true;
+ }
+
+ private boolean isJsniIdent(String ident) {
+ return ident.charAt(0) == '@';
+ }
}.accept(jsFunc);
push(jsFunc);
@@ -1900,6 +1962,9 @@
@Override
public void endVisit(JsniMethodRef x, Context ctx) {
+ if (x.getTarget() instanceof JConstructor) {
+ liveCtors.add((JConstructor) x.getTarget());
+ }
endVisit((JMethodCall) x, ctx);
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/JsniRefLookup.java b/dev/core/src/com/google/gwt/dev/jjs/impl/JsniRefLookup.java
index 527c37d..7ea3d1d 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/JsniRefLookup.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/JsniRefLookup.java
@@ -19,9 +19,11 @@
import com.google.gwt.dev.jjs.ast.JArrayType;
import com.google.gwt.dev.jjs.ast.JClassLiteral;
import com.google.gwt.dev.jjs.ast.JClassType;
+import com.google.gwt.dev.jjs.ast.JConstructor;
import com.google.gwt.dev.jjs.ast.JDeclaredType;
import com.google.gwt.dev.jjs.ast.JField;
import com.google.gwt.dev.jjs.ast.JMethod;
+import com.google.gwt.dev.jjs.ast.JParameter;
import com.google.gwt.dev.jjs.ast.JPrimitiveType;
import com.google.gwt.dev.jjs.ast.JProgram;
import com.google.gwt.dev.jjs.ast.JType;
@@ -103,7 +105,7 @@
return null;
} else {
// look for a method
- LinkedHashMap<String, LinkedHashMap<String, HasEnclosingType>> matchesBySig = new LinkedHashMap<String, LinkedHashMap<String, HasEnclosingType>>();
+ LinkedHashMap<String, LinkedHashMap<String, JMethod>> matchesBySig = new LinkedHashMap<String, LinkedHashMap<String, JMethod>>();
String methodName = ref.memberName();
String jsniSig = ref.memberSignature();
if (type == null) {
@@ -111,9 +113,9 @@
return program.getNullMethod();
}
} else {
- findMostDerivedMembers(matchesBySig, (JDeclaredType) type,
- ref.memberName(), true);
- LinkedHashMap<String, HasEnclosingType> matches = matchesBySig.get(jsniSig);
+ findMostDerivedMembers(matchesBySig, (JDeclaredType) type, methodName,
+ true);
+ LinkedHashMap<String, JMethod> matches = matchesBySig.get(jsniSig);
if (matches != null && matches.size() == 1) {
/*
* Backward compatibility: allow accessing bridge methods with full
@@ -165,11 +167,11 @@
* @param fullSig The fully qualified signature for that member
*/
private static void addMember(
- LinkedHashMap<String, LinkedHashMap<String, HasEnclosingType>> matchesBySig,
- HasEnclosingType member, String refSig, String fullSig) {
- LinkedHashMap<String, HasEnclosingType> matchesByFullSig = matchesBySig.get(refSig);
+ LinkedHashMap<String, LinkedHashMap<String, JMethod>> matchesBySig,
+ JMethod member, String refSig, String fullSig) {
+ LinkedHashMap<String, JMethod> matchesByFullSig = matchesBySig.get(refSig);
if (matchesByFullSig == null) {
- matchesByFullSig = new LinkedHashMap<String, HasEnclosingType>();
+ matchesByFullSig = new LinkedHashMap<String, JMethod>();
matchesBySig.put(refSig, matchesByFullSig);
}
matchesByFullSig.put(fullSig, member);
@@ -182,7 +184,7 @@
* methods.
*/
private static void findMostDerivedMembers(
- LinkedHashMap<String, LinkedHashMap<String, HasEnclosingType>> matchesBySig,
+ LinkedHashMap<String, LinkedHashMap<String, JMethod>> matchesBySig,
JDeclaredType targetType, String memberName,
boolean addConstructorsAndPrivates) {
/*
@@ -202,74 +204,41 @@
// Get the methods on this class/interface.
for (JMethod method : targetType.getMethods()) {
- if (method.getName().equals(memberName)) {
+ if (method instanceof JConstructor && JsniRef.NEW.equals(memberName)) {
if (!addConstructorsAndPrivates) {
- if (method.getName().equals(JsniRef.NEW)) {
- continue;
- }
- if (method.isPrivate()) {
- continue;
- }
+ continue;
}
- String fullSig = getJsniSignature(method, false);
- String wildcardSig = getJsniSignature(method, true);
+ StringBuilder sb = new StringBuilder();
+ sb.append(JsniRef.NEW);
+ sb.append("(");
+ for (JParameter param : method.getParams()) {
+ sb.append(param.getType().getJsniSignatureName());
+ }
+ sb.append(")");
+ String fullSig = sb.toString();
+ String wildcardSig = JsniRef.NEW + "(" + JsniRef.WILDCARD_PARAM_LIST + ")";
+ addMember(matchesBySig, method, fullSig, fullSig);
+ addMember(matchesBySig, method, wildcardSig, fullSig);
+ } else if (method.getName().equals(memberName)) {
+ if (!addConstructorsAndPrivates && method.isPrivate()) {
+ continue;
+ }
+ String fullSig = JProgram.getJsniSig(method, false);
+ String wildcardSig = method.getName() + "("
+ + JsniRef.WILDCARD_PARAM_LIST + ")";
addMember(matchesBySig, method, fullSig, fullSig);
addMember(matchesBySig, method, wildcardSig, fullSig);
}
}
-
- // Get the fields on this class/interface.
- for (JField field : targetType.getFields()) {
- if (field.getName().equals(memberName)) {
- addMember(matchesBySig, field, field.getName(), field.getName());
- }
- }
- }
-
- /**
- * Return the JSNI signature for a member. Leave off the return type for a
- * method signature, so as to match what a user would type in as a JsniRef.
- */
- private static String getJsniSignature(HasEnclosingType member,
- boolean wildcardParams) {
- if (member instanceof JField) {
- return ((JField) member).getName();
- }
- JMethod method = (JMethod) member;
-
- if (wildcardParams) {
- return method.getName() + "(" + JsniRef.WILDCARD_PARAM_LIST + ")";
- } else {
- return JProgram.getJsniSig(method, false);
- }
- }
-
- private static boolean isNewMethod(HasEnclosingType member) {
- if (member instanceof JMethod) {
- JMethod method = (JMethod) member;
- return method.getName().equals(JsniRef.NEW);
- }
-
- assert member instanceof JField;
- return false;
- }
-
- private static boolean isSynthetic(HasEnclosingType member) {
- if (member instanceof JMethod) {
- return ((JMethod) member).isSynthetic();
- }
-
- assert member instanceof JField;
- return false;
}
private static void removeSyntheticMembers(
- LinkedHashMap<String, LinkedHashMap<String, HasEnclosingType>> matchesBySig) {
- for (LinkedHashMap<String, HasEnclosingType> matchesByFullSig : matchesBySig.values()) {
+ LinkedHashMap<String, LinkedHashMap<String, JMethod>> matchesBySig) {
+ for (LinkedHashMap<String, JMethod> matchesByFullSig : matchesBySig.values()) {
Set<String> toRemove = new LinkedHashSet<String>();
for (String fullSig : matchesByFullSig.keySet()) {
- HasEnclosingType member = matchesByFullSig.get(fullSig);
- if (isSynthetic(member) && !isNewMethod(member)) {
+ JMethod member = matchesByFullSig.get(fullSig);
+ if (member.isSynthetic()) {
toRemove.add(fullSig);
}
}
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/JsniRefLookupTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/JsniRefLookupTest.java
index ed562ff..f6b24dc 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/JsniRefLookupTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/JsniRefLookupTest.java
@@ -233,7 +233,7 @@
JMethod res = (JMethod) lookup("test.Foo::new()", errors);
errors.assertNoError();
assertEquals("test.Foo", res.getEnclosingType().getName());
- assertEquals("new", res.getName());
+ assertEquals("Foo", res.getName());
}
{
@@ -292,7 +292,7 @@
JMethod res = (JMethod) lookup("test.Foo::new()", errors);
errors.assertNoError();
assertEquals("test.Foo", res.getEnclosingType().getName());
- assertEquals("new", res.getName());
+ assertEquals("Foo", res.getName());
assertEquals(0, res.getParams().size());
}
{
@@ -300,7 +300,7 @@
JMethod res = (JMethod) lookup("test.Foo::new(I)", errors);
errors.assertNoError();
assertEquals("test.Foo", res.getEnclosingType().getName());
- assertEquals("new", res.getName());
+ assertEquals("Foo", res.getName());
assertEquals(1, res.getParams().size());
assertSame(JPrimitiveType.INT, res.getParams().get(0).getType());
}
@@ -315,7 +315,7 @@
JMethod res = (JMethod) lookup("test.Bar::new()", errors);
errors.assertNoError();
assertEquals("test.Bar", res.getEnclosingType().getName());
- assertEquals("new", res.getName());
+ assertEquals("Bar", res.getName());
assertEquals(0, res.getParams().size());
}
{
@@ -323,7 +323,7 @@
JMethod res = (JMethod) lookup("test.Bar::new(*)", errors);
errors.assertNoError();
assertEquals("test.Bar", res.getEnclosingType().getName());
- assertEquals("new", res.getName());
+ assertEquals("Bar", res.getName());
assertEquals(0, res.getParams().size());
}
{