GROOVY-12488: SC: call the constructor of a constructor reference directly

COMPATIBILITY.md

@@ -809,6 +809,41 @@ of `CompilationUnit` that adds it as a phase operation, or that calls
 `PlaceholderVisitor.isPlaceholder`. Drop the phase operation. Where code needs
 to know whether a variable is `_`, check its name.
 
+### Groovy 7 — statically compiled constructor references call the constructor directly (GROOVY-12488)
+
+Under `@CompileStatic`, a constructor reference such as `X::new` created the
+object through the meta-class, which selected the constructor at run time from
+the types of the arguments. The constructor is now selected at compile time,
+among the constructors visible from the enclosing class, for the parameter
+types of the functional interface, as for `new X(args)` in the same class and
+as in Java, and it is called directly.
+
+**Who is affected.** Statically compiled constructor references whose result
+depended on that run-time selection:
+
+- The overload is chosen by the static parameter types of the functional
+  interface. `Function<CharSequence, X> f = X::new` calls `X(CharSequence)` even
+  when it is given a `String`. With `IL(Object)` and `IL(long)`,
+  `Function<Integer, IL>` calls `IL(Object)`, where it called `IL(long)`; with
+  `IL2(int)` and `IL2(Integer)`, `IntFunction<IL2>` calls `IL2(int)`, where it
+  called `IL2(Integer)`.
+- An `Object[]` given to `Function<Object, VO>` for `VO(Object...)` is passed as
+  one element, where its elements were passed.
+- `null` given to `Function<Integer, PI>` for `PI(int)` throws
+  `NullPointerException`, where it threw `GroovyRuntimeException`.
+- A constructor the enclosing class cannot call no longer wins over one it can.
+  With `private Foo(String)` and `public Foo(Object)`, `Function<String, Foo>`
+  in another class calls `Foo(Object)`, as `new Foo(s)` does.
+- A constructor added or overridden through the meta-class, for example with
+  `ExpandoMetaClass`, is not used, as for a statically compiled `new`.
+
+**What is unchanged.** The constructor is still selected at run time when no
+single constructor matches the parameter types (GROOVY-10971), for an enum, for
+a protected constructor of another package, and for a protected or
+package-private constructor of a class that a `GroovyClassLoader` defined in
+another compilation, such as a class of an earlier `GroovyShell` parse. Dynamic
+and `@TypeChecked` code select the constructor at run time as before.
+
 ## The binary-compatibility check
 
 The [`subprojects/binary-compatibility/`](subprojects/binary-compatibility)

src/main/java/groovy/lang/GroovyClassLoader.java

@@ -237,6 +237,7 @@ public Class defineClass(final ClassNode classNode, final String file, final Str
         }
 
         CompilationUnit unit = createCompilationUnit(config, codeSource);
+        markDefinedInMemory(unit);
         var generatedClasses = new LinkedHashMap<ClassNode, ClassVisitor>();
         ClassCollector collector = createCollector(unit, classNode.getModule().getContext());
         try {
@@ -414,6 +415,7 @@ private Class<?> doParseClass(final GroovyCodeSource codeSource) {
         validate(codeSource);
         // Was neither already loaded nor compiling, so compile and add to cache.
         CompilationUnit unit = createCompilationUnit(config, codeSource.getCodeSource());
+        markDefinedInMemory(unit);
         if (recompile != null ? recompile : config.getRecompileGroovySource()) {
             unit.addFirstPhaseOperation(TimestampAdder.INSTANCE, CompilePhase.CLASS_GENERATION.getPhaseNumber());
         }
@@ -448,6 +450,22 @@ private Class<?> doParseClass(final GroovyCodeSource codeSource) {
         return answer;
     }
 
+    /**
+     * Records that the classes of the compilation unit are defined by an
+     * {@link InnerLoader} of their own, so that the compiler does not assume
+     * that a class of another compilation unit is in the same run-time package.
+     * <p>
+     * Every compilation of {@code parseClass} is marked, whatever its
+     * {@link ClassCollector} does with the classes. A subclass whose collector
+     * defines them with this class loader itself, rather than with an
+     * {@code InnerLoader}, therefore reaches the non-public members of classes
+     * of its other compilations through the meta-class where a direct access
+     * would also work; that is slower, but not wrong.
+     */
+    private static void markDefinedInMemory(final CompilationUnit unit) {
+        unit.getAST().putNodeMetaData(InnerLoader.class, Boolean.TRUE);
+    }
+
     private static void validate(final GroovyCodeSource codeSource) {
         if (codeSource.getFile() == null) {
             if (codeSource.getScriptText() == null) {

src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java

@@ -18,6 +18,7 @@
  */
 package org.codehaus.groovy.classgen;
 
+import groovy.lang.GroovyClassLoader;
 import groovy.lang.GroovyRuntimeException;
 import org.apache.groovy.ast.tools.ExpressionUtils;
 import org.apache.groovy.io.StringBuilderWriter;
@@ -1312,6 +1313,41 @@ public static boolean isFieldDirectlyAccessible(final FieldNode field, final Cla
         return field != null && isMemberDirectlyAccessible(field.getModifiers(), field.getDeclaringClass(), accessingClass);
     }
 
+    /**
+     * Determines if the bytecode of the accessing class can rely on being in
+     * the run-time package of the declaring class, which is the package name
+     * and the class loader. The JVM allows access to a protected or
+     * package-private member only within the same run-time package, apart from
+     * the subclass rule for a protected member.
+     * <p>
+     * A GroovyClassLoader defines the classes of each compilation, like a
+     * {@code GroovyShell} script, in memory with a class loader of their own,
+     * so a class compiled before, by another compilation, is in another
+     * run-time package although its package name is the same. Classes that
+     * are compiled to class files are expected to be loaded by one class
+     * loader with the classes they use from the same package, as javac
+     * expects.
+     * <p>
+     * {@code isPrimaryClassNode()} is used here as "defined by the class
+     * loader of the accessing class", since the classes of one compilation are
+     * defined together; it does not stand for "compiled from source".
+     *
+     * @param declaringClass the class declaring the member
+     * @param accessingClass the class attempting access
+     * @return {@code true} if both classes have the same package name and are
+     *         not known to be defined by different class loaders
+     */
+    public static boolean isInSameRuntimePackage(final ClassNode declaringClass, final ClassNode accessingClass) {
+        if (!Objects.equals(declaringClass.getPackageName(), accessingClass.getPackageName())) return false;
+        if (declaringClass.isPrimaryClassNode()) return true; // compiled and defined with the accessing class
+        CompileUnit unit = null;
+        for (ClassNode cn = accessingClass; unit == null && cn != null; cn = cn.getOuterClass()) {
+            unit = cn.getCompileUnit();
+        }
+        // see GroovyClassLoader: the classes of this compilation are defined by an InnerLoader of their own
+        return unit == null || !Boolean.TRUE.equals(unit.getNodeMetaData(GroovyClassLoader.InnerLoader.class));
+    }
+
     /**
      * Determines whether a member with the supplied modifiers is directly accessible to a class.
      *

src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java

@@ -176,6 +176,29 @@ public void writeInvokeConstructor(final ConstructorCallExpression call) {
         finnishConstructorCall(cn, ownerDescriptor, controller.getOperandStack().getStackLength() - before);
     }
 
+    /**
+     * Determines if the bytecode of the accessing class can call the constructor
+     * with {@code NEW} and {@code INVOKESPECIAL}.
+     * <p>
+     * A protected or package-private constructor needs the same run-time package
+     * (see {@link AsmClassGenerator#isInSameRuntimePackage}). Outside its package,
+     * a protected constructor can only be called by {@code super(...)} or an
+     * anonymous inner class.
+     * <p>
+     * Unlike {@link AsmClassGenerator#isMemberDirectlyAccessible}, there is no
+     * subclass rule: the verifier rejects {@code NEW} and {@code INVOKESPECIAL}
+     * of a protected constructor of another package in a subclass too, so a
+     * constructor reference to it creates the object through the meta-class.
+     */
+    static boolean isDirectlyAccessible(final ConstructorNode ctor, final ClassNode accessingClass) {
+        if (ctor.isPublic()) return true;
+        ClassNode declaringClass = ctor.getDeclaringClass();
+        if (ctor.isPrivate()) { // nestmates can call a private constructor
+            return AsmClassGenerator.isMemberDirectlyAccessible(ctor.getModifiers(), declaringClass, accessingClass);
+        }
+        return AsmClassGenerator.isInSameRuntimePackage(declaringClass, accessingClass);
+    }
+
     /** {@inheritDoc} */
     @Override
     public void writeSpecialConstructorCall(final ConstructorCallExpression call) {

src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java

@@ -21,6 +21,7 @@
 import org.codehaus.groovy.ast.ASTNode;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.ConstructorNode;
 import org.codehaus.groovy.ast.GenericsType;
 import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.ast.Parameter;
@@ -69,6 +70,7 @@
 import static org.codehaus.groovy.classgen.asm.sc.StaticTypesFunctionalInterfaceMetadataKey.METHOD_REFERENCE_DESERIALIZE_METHOD_NAME;
 import static org.codehaus.groovy.runtime.ArrayGroovyMethods.last;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.allParametersAndArgumentsMatch;
+import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.chooseBestMethod;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.filterMethodsByVisibility;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.findDGMMethodsForClassNode;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf;
@@ -460,26 +462,63 @@ private MethodNode addSyntheticMethodForConstructorReference(final String method
         ArgumentListExpression ctorArgs = new ArgumentListExpression(parametersWithExactType);
 
         Expression returnValue;
-        if (returnType.isArray()) {
+        boolean staticCompile = false;
+        if (returnType.isArray()) { // like new T[n]
             returnValue = new ArrayExpression(
                     returnType.getComponentType(),
                     null, ctorArgs.getExpressions());
+            staticCompile = true;
         } else {
             returnValue = ctorX(returnType, ctorArgs);
+            ConstructorNode ctor = findConstructorReferenceTarget(returnType, parametersWithExactType);
+            if (ctor != null) {
+                returnValue.putNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET, ctor);
+                staticCompile = true;
+            }
         }
 
         MethodNode delegateMethod = addSyntheticMethod(methodName, returnType, returnValue, parametersWithExactType, ClassNode.EMPTY_ARRAY);
-        // TODO: if StaticTypesMarker.DIRECT_METHOD_CALL_TARGET or
-        // OptimizingStatementWriter.StatementMeta.class metadatas
-        // can bet set for the ctorX above, then this can be TRUE:
-        delegateMethod.putNodeMetaData(StaticCompilationMetadataKeys.STATIC_COMPILE_NODE, Boolean.FALSE);
+        // without a target constructor, the constructor is selected at run time
+        delegateMethod.putNodeMetaData(StaticCompilationMetadataKeys.STATIC_COMPILE_NODE, staticCompile);
         return delegateMethod;
     }
 
+    /**
+     * Selects the constructor of {@code type} that accepts the parameter types
+     * of the functional interface, among the constructors visible from the
+     * enclosing class, as the type checker selects the constructor of a
+     * constructor call.
+     * <p>
+     * A returned constructor is called directly, like the constructor of a
+     * statically compiled {@code new}, so the meta-class does not see the call:
+     * a constructor added or overridden through the meta-class, for example
+     * with {@code ExpandoMetaClass}, is not used.
+     *
+     * @return the constructor, or {@code null} if no single constructor matches
+     *         or the matching constructor cannot be called directly
+     */
+    private ConstructorNode findConstructorReferenceTarget(final ClassNode type, final Parameter[] samParameters) {
+        // the type checker rejects an abstract class or interface, unless a type checking extension suppresses the error;
+        // an enum, which Java code cannot instantiate at all, is left to the meta-class
+        if (type.isInterface() || type.isAbstract() || type.isEnum()) return null;
+
+        ClassNode[] argumentTypes = Arrays.stream(samParameters).map(Parameter::getType).toArray(ClassNode[]::new);
+        // in a closure or lambda, visibility is checked for the enclosing class, as the type checker does
+        List<MethodNode> visible = filterMethodsByVisibility(new ArrayList<>(type.getDeclaredConstructors()), controller.getThisType());
+        List<MethodNode> ctors = chooseBestMethod(type, visible, argumentTypes);
+        if (ctors.size() == 1 && ctors.get(0) instanceof ConstructorNode ctor
+                && StaticInvocationWriter.isDirectlyAccessible(ctor, controller.getClassNode())) {
+            return ctor;
+        }
+        return null;
+    }
+
     private MethodNode addSyntheticMethod(final String methodName, final ClassNode returnType, final Expression returnValue, final Parameter[] parameters, final ClassNode[] exceptions) {
-        return controller.getClassNode().addSyntheticMethod(
+        ClassNode classNode = controller.getClassNode();
+        return classNode.addSyntheticMethod(
             methodName,
-            Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_FINAL,
+            // the JVM rejects ACC_FINAL on any method of an interface (JVMS 4.6), so no helper method is final there
+            Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | (classNode.isInterface() ? 0 : Opcodes.ACC_FINAL),
             returnType,
             parameters,
             exceptions,

src/spec/doc/core-operators.adoc

@@ -548,6 +548,13 @@ include::../test/OperatorsTest.groovy[tags=constructor_refs,indent=0]
 <1> class constructor reference
 <2> array constructor reference
 
+Under static compilation, a constructor reference calls the constructor that `new` would call with the parameter
+types of the functional interface, selected at compile time among the constructors visible from the enclosing class.
+It is selected at run time instead, through the meta-class, when no single constructor matches those types, for an
+enum, for a protected constructor of another package, and for a protected or package-private constructor of a class
+that a `GroovyClassLoader` defined in another compilation, such as a class of an earlier `GroovyShell` parse. In a
+GraalVM native image, these run-time cases still need reflection metadata for the constructors.
+
 == Regular expression operators
 
 === Pattern operator