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());
     }
     {