You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2022/01/31 18:00:19 UTC

[groovy] branch GROOVY_2_5_X updated (214ef11 -> cdea1fc)

This is an automated email from the ASF dual-hosted git repository.

emilles pushed a change to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.


    from 214ef11  bintray decommission
     new 70c74d9  GROOVY-5728: anon. inner can use private constructor when super is outer
     new e0fc7d0  GROOVY-6747: ensure enum ctor private and add bridge for const with body
     new 8f4baea  GROOVY-7686: AIC: place local variable references above super ctor call
     new b16f6cd  GROOVY-9524: enum const class extends enum class, so no this$0 for calls
     new cdea1fc  GROOVY-8104: fix this$0 for closure with anon. inner class of inner type

The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../groovy/classgen/EnumCompletionVisitor.java     | 97 +++++++++++++---------
 .../org/codehaus/groovy/classgen/EnumVisitor.java  | 15 +++-
 .../classgen/InnerClassCompletionVisitor.java      | 76 +++++++++++------
 .../groovy/classgen/InnerClassVisitor.java         | 29 +++++--
 .../org/codehaus/groovy/classgen/Verifier.java     | 36 +++++++-
 .../classgen/asm/sc/StaticInvocationWriter.java    |  3 +-
 .../TupleConstructorASTTransformation.java         | 14 +---
 src/test/gls/enums/EnumTest.groovy                 | 34 ++++++++
 src/test/gls/innerClass/InnerClassTest.groovy      | 72 ++++++++++++++--
 .../bugs/{Groovy9294.groovy => Groovy9524.groovy}  | 27 +++---
 .../stc/AnonymousInnerClassSTCTest.groovy          | 17 ++++
 11 files changed, 317 insertions(+), 103 deletions(-)
 copy src/test/groovy/bugs/{Groovy9294.groovy => Groovy9524.groovy} (67%)

[groovy] 01/05: GROOVY-5728: anon. inner can use private constructor when super is outer

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 70c74d9b948dbea9238809277b47a35699c3889a
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Mar 20 10:51:07 2021 -0500

    GROOVY-5728: anon. inner can use private constructor when super is outer
    
    Conflicts:
    	src/main/java/org/codehaus/groovy/classgen/InnerClassCompletionVisitor.java
---
 .../classgen/InnerClassCompletionVisitor.java      | 76 +++++++++++++++-------
 .../stc/AnonymousInnerClassSTCTest.groovy          | 17 +++++
 2 files changed, 69 insertions(+), 24 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/InnerClassCompletionVisitor.java b/src/main/java/org/codehaus/groovy/classgen/InnerClassCompletionVisitor.java
index 1e52435..445ed0a 100644
--- a/src/main/java/org/codehaus/groovy/classgen/InnerClassCompletionVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/InnerClassCompletionVisitor.java
@@ -42,7 +42,11 @@ import org.objectweb.asm.Opcodes;
 import java.util.List;
 
 import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedConstructor;
-import static org.codehaus.groovy.ast.ClassHelper.CLOSURE_TYPE;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.castX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorThisX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
 
 public class InnerClassCompletionVisitor extends InnerClassVisitorHelper implements Opcodes {
 
@@ -51,8 +55,8 @@ public class InnerClassCompletionVisitor extends InnerClassVisitorHelper impleme
     private FieldNode thisField = null;
 
     private static final String
-            CLOSURE_INTERNAL_NAME   = BytecodeHelper.getClassInternalName(CLOSURE_TYPE),
-            CLOSURE_DESCRIPTOR      = BytecodeHelper.getTypeDescription(CLOSURE_TYPE);
+            CLOSURE_INTERNAL_NAME = BytecodeHelper.getClassInternalName(ClassHelper.CLOSURE_TYPE),
+            CLOSURE_DESCRIPTOR    = BytecodeHelper.getTypeDescription(ClassHelper.CLOSURE_TYPE);
 
     public InnerClassCompletionVisitor(CompilationUnit cu, SourceUnit su) {
         sourceUnit = su;
@@ -65,7 +69,7 @@ public class InnerClassCompletionVisitor extends InnerClassVisitorHelper impleme
 
     @Override
     public void visitClass(ClassNode node) {
-        this.classNode = node;
+        classNode = node;
         thisField = null;
         InnerClassNode innerClass = null;
         if (!node.isEnum() && !node.isInterface() && node instanceof InnerClassNode) {
@@ -88,6 +92,30 @@ public class InnerClassCompletionVisitor extends InnerClassVisitorHelper impleme
     public void visitConstructor(ConstructorNode node) {
         addThisReference(node);
         super.visitConstructor(node);
+        // an anonymous inner class may use a private constructor (via a bridge) if its super class is also an outer class
+        if (((InnerClassNode) classNode).isAnonymous() && classNode.getOuterClasses().contains(classNode.getSuperClass())) {
+            ConstructorNode superCtor = classNode.getSuperClass().getDeclaredConstructor(node.getParameters());
+            if (superCtor != null && superCtor.isPrivate()) {
+                ClassNode superClass = classNode.getUnresolvedSuperClass();
+                makeBridgeConstructor(superClass, node.getParameters()); // GROOVY-5728
+                ConstructorCallExpression superCtorCall = getFirstIfSpecialConstructorCall(node.getCode());
+                ((TupleExpression) superCtorCall.getArguments()).addExpression(castX(superClass, nullX()));
+            }
+        }
+    }
+
+    private static void makeBridgeConstructor(ClassNode c, Parameter[] p) {
+        Parameter[] newP = new Parameter[p.length + 1];
+        for (int i = 0; i < p.length; i += 1) {
+            newP[i] = new Parameter(p[i].getType(), "p" + i);
+        }
+        newP[p.length] = new Parameter(c, "$anonymous");
+
+        if (c.getDeclaredConstructor(newP) == null) {
+            TupleExpression args = new TupleExpression();
+            for (int i = 0; i < p.length; i += 1) args.addExpression(varX(newP[i]));
+            addGeneratedConstructor(c, ACC_SYNTHETIC, newP, ClassNode.EMPTY_ARRAY, stmt(ctorThisX(args)));
+        }
     }
 
     private static String getTypeDescriptor(ClassNode node, boolean isStatic) {
@@ -160,7 +188,7 @@ public class InnerClassCompletionVisitor extends InnerClassVisitorHelper impleme
 
     private void getThis(MethodVisitor mv, String classInternalName, String outerClassDescriptor, String innerClassInternalName) {
         mv.visitVarInsn(ALOAD, 0);
-        if (thisField != null && CLOSURE_TYPE.equals(thisField.getType())) {
+        if (thisField != null && ClassHelper.CLOSURE_TYPE.equals(thisField.getType())) {
             mv.visitFieldInsn(GETFIELD, classInternalName, "this$0", CLOSURE_DESCRIPTOR);
             mv.visitMethodInsn(INVOKEVIRTUAL, CLOSURE_INTERNAL_NAME, "getThisObject", "()Ljava/lang/Object;", false);
             mv.visitTypeInsn(CHECKCAST, innerClassInternalName);
@@ -168,18 +196,17 @@ public class InnerClassCompletionVisitor extends InnerClassVisitorHelper impleme
             mv.visitFieldInsn(GETFIELD, classInternalName, "this$0", outerClassDescriptor);
         }
     }
-    
-    private void addDefaultMethods(InnerClassNode node) {
-        final boolean isStatic = isStatic(node);
 
+    private void addDefaultMethods(InnerClassNode node) {
+        boolean isStatic = isStatic(node);
         ClassNode outerClass = node.getOuterClass();
-        final String classInternalName = org.codehaus.groovy.classgen.asm.BytecodeHelper.getClassInternalName(node);
+        final int objectDistance = getObjectDistance(outerClass);
+        final String classInternalName = BytecodeHelper.getClassInternalName(node);
         final String outerClassInternalName = getInternalName(outerClass, isStatic);
         final String outerClassDescriptor = getTypeDescriptor(outerClass, isStatic);
-        final int objectDistance = getObjectDistance(outerClass);
 
         // add missing method dispatcher
-        Parameter[] parameters = new Parameter[]{
+        Parameter[] parameters = {
                 new Parameter(ClassHelper.STRING_TYPE, "name"),
                 new Parameter(ClassHelper.OBJECT_TYPE, "args")
         };
@@ -446,19 +473,20 @@ public class InnerClassCompletionVisitor extends InnerClassVisitorHelper impleme
         return namePrefix;
     }
 
-    private static ConstructorCallExpression getFirstIfSpecialConstructorCall(BlockStatement code) {
-        if (code == null) return null;
-
-        final List<Statement> statementList = code.getStatements();
-        if (statementList.isEmpty()) return null;
-
-        final Statement statement = statementList.get(0);
-        if (!(statement instanceof ExpressionStatement)) return null;
-
-        Expression expression = ((ExpressionStatement) statement).getExpression();
-        if (!(expression instanceof ConstructorCallExpression)) return null;
-        ConstructorCallExpression cce = (ConstructorCallExpression) expression;
-        if (cce.isSpecialCall()) return cce;
+    private static ConstructorCallExpression getFirstIfSpecialConstructorCall(Statement code) {
+        if (code != null && !code.isEmpty()) {
+            if (code instanceof BlockStatement) {
+                Statement stmt = ((BlockStatement) code).getStatements().get(0);
+                return getFirstIfSpecialConstructorCall(stmt); // blocks of blocks
+            }
+            if (code instanceof ExpressionStatement) {
+                Expression expr = ((ExpressionStatement) code).getExpression();
+                if (expr instanceof ConstructorCallExpression) {
+                    ConstructorCallExpression call = (ConstructorCallExpression) expr;
+                    if (call.isSpecialCall()) return call;
+                }
+            }
+        }
         return null;
     }
 }
diff --git a/src/test/groovy/transform/stc/AnonymousInnerClassSTCTest.groovy b/src/test/groovy/transform/stc/AnonymousInnerClassSTCTest.groovy
index eb52a3a..ab1bb2e 100644
--- a/src/test/groovy/transform/stc/AnonymousInnerClassSTCTest.groovy
+++ b/src/test/groovy/transform/stc/AnonymousInnerClassSTCTest.groovy
@@ -206,4 +206,21 @@ class AnonymousInnerClassSTCTest extends StaticTypeCheckingTestCase {
             }
         '''
     }
+
+    // GROOVY-5728
+    void testPrivateConstructorAndPublicStaticFactory() {
+        assertScript '''
+            abstract class A {
+                private A() { }
+                abstract answer()
+                static A create() {
+                    return new A() { // IllegalAccessError when A$1 calls private constructor
+                        def answer() { 42 }
+                    }
+                }
+            }
+
+            assert A.create().answer() == 42
+        '''
+    }
 }

[groovy] 05/05: GROOVY-8104: fix this$0 for closure with anon. inner class of inner type

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit cdea1fcf68fdebe3c5c12f82d79f7426f4107294
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Mar 21 13:21:33 2021 -0500

    GROOVY-8104: fix this$0 for closure with anon. inner class of inner type
    
    2_5_X backport
    
      class Outer {
        class Inner {
          // requires Outer reference
        }
        def x = { ->
          new Inner() { // this.this$0 returns closure; call getThisObject()
          }
        }
      }
    
    Conflicts:
    	src/test/gls/innerClass/InnerClassTest.groovy
---
 .../groovy/classgen/InnerClassVisitor.java         | 29 ++++++++++++------
 src/test/gls/innerClass/InnerClassTest.groovy      | 35 ++++++++++++++++++----
 2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/InnerClassVisitor.java b/src/main/java/org/codehaus/groovy/classgen/InnerClassVisitor.java
index c9cf8b3..635f236 100644
--- a/src/main/java/org/codehaus/groovy/classgen/InnerClassVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/InnerClassVisitor.java
@@ -33,7 +33,7 @@ import org.codehaus.groovy.ast.VariableScope;
 import org.codehaus.groovy.ast.expr.ClosureExpression;
 import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
 import org.codehaus.groovy.ast.expr.Expression;
-import org.codehaus.groovy.ast.expr.PropertyExpression;
+import org.codehaus.groovy.ast.expr.MethodCallExpression;
 import org.codehaus.groovy.ast.expr.TupleExpression;
 import org.codehaus.groovy.ast.expr.VariableExpression;
 import org.codehaus.groovy.ast.stmt.BlockStatement;
@@ -47,6 +47,11 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
+import static org.codehaus.groovy.ast.tools.GeneralUtils.attrX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
+
 public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcodes {
 
     private ClassNode classNode;
@@ -296,14 +301,20 @@ public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcode
         // if constructor call is not in outer class, don't pass 'this' implicitly. Return.
         if (parent == null) return;
 
-        //add this parameter to node
-        Expression argsExp = call.getArguments();
-        if (argsExp instanceof TupleExpression) {
-            TupleExpression argsListExp = (TupleExpression) argsExp;
-            Expression this0 = VariableExpression.THIS_EXPRESSION;
-            for (int i = 0; i != level; ++i)
-                this0 = new PropertyExpression(this0, "this$0");
-            argsListExp.getExpressions().add(0, this0);
+        Expression args = call.getArguments();
+        if (args instanceof TupleExpression) {
+            Expression this0 = varX("this"); // bypass closure
+            for (int i = 0; i != level; ++i) {
+                this0 = attrX(this0, constX("this$0"));
+                // GROOVY-8104: an anonymous inner class may still have closure nesting
+                if (i == 0 && classNode.getDeclaredField("this$0").getType().equals(ClassHelper.CLOSURE_TYPE)) {
+                    MethodCallExpression getThis = callX(this0, "getThisObject");
+                    getThis.setImplicitThis(false);
+                    this0 = getThis;
+                }
+            }
+
+            ((TupleExpression) args).getExpressions().add(0, this0);
         }
     }
 }
diff --git a/src/test/gls/innerClass/InnerClassTest.groovy b/src/test/gls/innerClass/InnerClassTest.groovy
index 73e7fb7..52a1c45 100644
--- a/src/test/gls/innerClass/InnerClassTest.groovy
+++ b/src/test/gls/innerClass/InnerClassTest.groovy
@@ -228,13 +228,38 @@ final class InnerClassTest {
         '''
     }
 
-    @Test @NotYetImplemented
-    void testNonStaticInnerClass2() {
-        shouldFail CompilationFailedException, '''
+    @Test // GROOVY-8104
+    void testNonStaticInnerClass5() {
+        assertScript '''
             class A {
-                class B {}
+                void foo() {
+                    C c = new C()
+                    ['1','2','3'].each { obj ->
+                        c.baz(obj, new I() {
+                            @Override
+                            void bar(Object o) {
+                                B b = new B() // Could not find matching constructor for: A$B(A$_foo_closure1)
+                            }
+                        })
+                    }
+                }
+
+                class B {
+                }
+            }
+
+            class C {
+                void baz(Object o, I i) {
+                    i.bar(o)
+                }
             }
-            def x = new A.B() // requires reference to A
+
+            interface I {
+                void bar(Object o)
+            }
+
+            A a = new A()
+            a.foo()
         '''
     }
 

[groovy] 02/05: GROOVY-6747: ensure enum ctor private and add bridge for const with body

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit e0fc7d0ad7951cdcf43d096de39b54134ee1a495
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Mar 18 18:58:18 2021 -0500

    GROOVY-6747: ensure enum ctor private and add bridge for const with body
    
    2_5_X backport
    
      enum E {
        X {
        },
        Y
      }
    
    E provides a private, implicit constructor (String,int) for Y and a
    package-private, synthetic constructor (String,int,E) for X (aka E$1).
    
    Conflicts:
    	src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
---
 .../groovy/classgen/EnumCompletionVisitor.java     | 97 +++++++++++++---------
 .../org/codehaus/groovy/classgen/EnumVisitor.java  | 15 +++-
 .../TupleConstructorASTTransformation.java         | 14 +---
 src/test/gls/enums/EnumTest.groovy                 | 34 ++++++++
 4 files changed, 109 insertions(+), 51 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java b/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
index 8ef3a04..c209a3f 100644
--- a/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
@@ -23,10 +23,10 @@ import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.CodeVisitorSupport;
 import org.codehaus.groovy.ast.ConstructorNode;
-import org.codehaus.groovy.ast.EnumConstantClassNode;
-import org.codehaus.groovy.ast.InnerClassNode;
 import org.codehaus.groovy.ast.Parameter;
 import org.codehaus.groovy.ast.expr.ArgumentListExpression;
+import org.codehaus.groovy.ast.expr.CastExpression;
+import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
 import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.ast.expr.TupleExpression;
@@ -41,7 +41,9 @@ import org.codehaus.groovy.transform.TupleConstructorASTTransformation;
 import java.util.ArrayList;
 import java.util.List;
 
-import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
+import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedConstructor;
+import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
+import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
 
 /**
  * Enums have a parent constructor with two arguments from java.lang.Enum.
@@ -49,57 +51,55 @@ import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
  * and performs the necessary super call.
  */
 public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
+
     private final SourceUnit sourceUnit;
 
     public EnumCompletionVisitor(CompilationUnit cu, SourceUnit su) {
         sourceUnit = su;
     }
 
-    public void visitClass(ClassNode node) {
-        if (!node.isEnum()) return;
-        completeEnum(node);
-    }
-
+    @Override
     protected SourceUnit getSourceUnit() {
         return sourceUnit;
     }
 
+    @Override
+    public void visitClass(ClassNode node) {
+        if (node.isEnum()) completeEnum(node);
+    }
+
     private void completeEnum(ClassNode enumClass) {
-        boolean isAic = isAnonymousInnerClass(enumClass);
-        if (enumClass.getDeclaredConstructors().isEmpty()) {
-            addImplicitConstructors(enumClass, isAic);
+        if (nonSyntheticConstructors(enumClass).isEmpty()) {
+            addImplicitConstructors(enumClass);
         }
 
-        for (ConstructorNode ctor : enumClass.getDeclaredConstructors()) {
-            transformConstructor(ctor, isAic);
+        for (ConstructorNode ctor : nonSyntheticConstructors(enumClass)) {
+            transformConstructor(ctor);
         }
     }
 
     /**
      * Add map and no-arg constructor or mirror those of the superclass (i.e. base enum).
      */
-    private static void addImplicitConstructors(ClassNode enumClass, boolean aic) {
-        if (aic) {
-            ClassNode sn = enumClass.getSuperClass();
-            List<ConstructorNode> sctors = new ArrayList<ConstructorNode>(sn.getDeclaredConstructors());
-            if (sctors.isEmpty()) {
-                addMapConstructors(enumClass);
-            } else {
-                for (ConstructorNode constructorNode : sctors) {
-                    ConstructorNode init = new ConstructorNode(ACC_PUBLIC, constructorNode.getParameters(), ClassNode.EMPTY_ARRAY, new BlockStatement());
-                    enumClass.addConstructor(init);
+    private static void addImplicitConstructors(ClassNode enumClass) {
+        if (EnumVisitor.isAnonymousInnerClass(enumClass)) {
+            List<ConstructorNode> superCtors = nonSyntheticConstructors(enumClass.getSuperClass());
+            if (!superCtors.isEmpty()) {
+                for (ConstructorNode ctor : superCtors) {
+                    addGeneratedConstructor(enumClass, ACC_PRIVATE, ctor.getParameters(), ClassNode.EMPTY_ARRAY, new BlockStatement());
                 }
+                return;
             }
-        } else {
-            addMapConstructors(enumClass);
         }
+        TupleConstructorASTTransformation.addSpecialMapConstructors(ACC_PRIVATE, enumClass, "One of the enum constants for enum " +
+                enumClass.getName() + " was initialized with null. Please use a non-null value or define your own constructor.", true);
     }
 
     /**
      * If constructor does not define a call to super, then transform constructor
      * to get String,int parameters at beginning and add call super(String,int).
      */
-    private void transformConstructor(ConstructorNode ctor, boolean isAic) {
+    private void transformConstructor(ConstructorNode ctor) {
         boolean chainedThisConstructorCall = false;
         ConstructorCallExpression cce = null;
         if (ctor.firstStatementIsSpecialConstructorCall()) {
@@ -127,13 +127,16 @@ public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
             argsExprs.add(1, intVariable);
         } else {
             // add a super call
-            List<Expression> args = new ArrayList<Expression>();
+            List<Expression> args = new ArrayList<>();
             args.add(stringVariable);
             args.add(intVariable);
-            if (isAic) {
+            if (EnumVisitor.isAnonymousInnerClass(ctor.getDeclaringClass())) {
                 for (Parameter parameter : oldP) {
-                    args.add(new VariableExpression(parameter.getName()));
+                    args.add(new VariableExpression(parameter));
                 }
+                ClassNode enumClass = ctor.getDeclaringClass().getSuperClass();
+                makeBridgeConstructor(enumClass, newP); // GROOVY-6747: bridge enum's private constructor
+                args.add(new CastExpression(enumClass.getPlainNodeReference(), ConstantExpression.NULL));
             }
             cce = new ConstructorCallExpression(ClassNode.SUPER, new ArgumentListExpression(args));
             BlockStatement code = new BlockStatement();
@@ -144,15 +147,11 @@ public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
         }
     }
 
-    private static void addMapConstructors(ClassNode enumClass) {
-        TupleConstructorASTTransformation.addSpecialMapConstructors(ACC_PUBLIC, enumClass, "One of the enum constants for enum " + enumClass.getName() +
-                " was initialized with null. Please use a non-null value or define your own constructor.", true);
-    }
-
     private String getUniqueVariableName(final String name, Statement code) {
         if (code == null) return name;
         final Object[] found = new Object[1];
         CodeVisitorSupport cv = new CodeVisitorSupport() {
+            @Override
             public void visitVariableExpression(VariableExpression expression) {
                 if (expression.getName().equals(name)) found[0] = Boolean.TRUE;
             }
@@ -162,9 +161,31 @@ public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
         return name;
     }
 
-    private static boolean isAnonymousInnerClass(ClassNode enumClass) {
-        if (!(enumClass instanceof EnumConstantClassNode)) return false;
-        InnerClassNode ic = (InnerClassNode) enumClass;
-        return ic.getVariableScope() == null;
+    /**
+     * Ensures the enum type {@code e} has an accessible constructor for its AIC
+     * constant class to call.  This constructor delegates to the enum's private
+     * constructor.
+     */
+    private static void makeBridgeConstructor(ClassNode e, Parameter[] p) {
+        Parameter[] newP = new Parameter[p.length + 1];
+        for (int i = 0; i < p.length; i += 1) {
+            newP[i] = new Parameter(p[i].getType(), "p" + i);
+        }
+        newP[p.length] = new Parameter(e.getPlainNodeReference(), "$anonymous");
+
+        if (e.getDeclaredConstructor(newP) == null) {
+            ArgumentListExpression args = new ArgumentListExpression();
+            for (int i = 0; i < p.length; i += 1) args.addExpression(new VariableExpression(newP[i]));
+            Statement thisCtorCall = new ExpressionStatement(new ConstructorCallExpression(ClassNode.THIS, args));
+            addGeneratedConstructor(e, ACC_SYNTHETIC, newP, ClassNode.EMPTY_ARRAY, thisCtorCall).setSynthetic(true);
+        }
+    }
+
+    private static List<ConstructorNode> nonSyntheticConstructors(ClassNode cn) {
+        List<ConstructorNode> ctors = new ArrayList<>(4);
+        for (ConstructorNode ctor : cn.getDeclaredConstructors()) {
+            if (!ctor.isSynthetic()) ctors.add(ctor);
+        }
+        return ctors;
     }
 }
diff --git a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java b/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
index 208a34e..cdf900b 100644
--- a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
@@ -22,6 +22,7 @@ import org.codehaus.groovy.ast.AnnotatedNode;
 import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.ConstructorNode;
 import org.codehaus.groovy.ast.EnumConstantClassNode;
 import org.codehaus.groovy.ast.FieldNode;
 import org.codehaus.groovy.ast.InnerClassNode;
@@ -98,6 +99,15 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
             values = new FieldNode("$VALUES", PRIVATE_FS | Opcodes.ACC_SYNTHETIC, enumRef.makeArray(), enumClass, null);
             values.setSynthetic(true);
 
+            for (ConstructorNode ctor : enumClass.getDeclaredConstructors()) {
+                if (ctor.isSyntheticPublic()) {
+                    ctor.setSyntheticPublic(false);
+                    ctor.setModifiers((ctor.getModifiers() | Opcodes.ACC_PRIVATE) & ~Opcodes.ACC_PUBLIC);
+                } else if (!ctor.isPrivate()) {
+                    addError(ctor, "Illegal modifier for the enum constructor; only private is permitted.");
+                }
+            }
+
             addMethods(enumClass, values);
             checkForAbstractMethods(enumClass);
 
@@ -296,7 +306,7 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
         // calling the constructor may require a table with MetaClass
         // selecting the constructor for each enum value. So instead we
         // use this method to have a central point for constructor selection
-        // and only one table. The whole construction is needed because 
+        // and only one table. The whole construction is needed because
         // Reflection forbids access to the enum constructor.
         // code:
         // def $INIT(Object[] para) {
@@ -437,10 +447,9 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
         );
     }
 
-    private static boolean isAnonymousInnerClass(ClassNode enumClass) {
+    static boolean isAnonymousInnerClass(ClassNode enumClass) {
         if (!(enumClass instanceof EnumConstantClassNode)) return false;
         InnerClassNode ic = (InnerClassNode) enumClass;
         return ic.getVariableScope() == null;
     }
-
 }
diff --git a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
index 1422fe7..a9271cf 100644
--- a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
@@ -58,7 +58,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
-import static org.apache.groovy.ast.tools.AnnotatedNodeUtils.markAsGenerated;
+import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedConstructor;
 import static org.apache.groovy.ast.tools.ClassNodeUtils.hasExplicitConstructor;
 import static org.apache.groovy.ast.tools.VisibilityUtils.getVisibility;
 import static org.codehaus.groovy.ast.ClassHelper.make;
@@ -260,9 +260,7 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
 
         boolean hasMapCons = AnnotatedNodeUtils.hasAnnotation(cNode, MapConstructorASTTransformation.MY_TYPE);
         int modifiers = getVisibility(anno, cNode, ConstructorNode.class, ACC_PUBLIC);
-        ConstructorNode consNode = new ConstructorNode(modifiers, params.toArray(Parameter.EMPTY_ARRAY), ClassNode.EMPTY_ARRAY, body);
-        markAsGenerated(cNode, consNode);
-        cNode.addConstructor(consNode);
+        addGeneratedConstructor(cNode, modifiers, params.toArray(Parameter.EMPTY_ARRAY), ClassNode.EMPTY_ARRAY, body);
 
         if (sourceUnit != null && !body.isEmpty()) {
             VariableScopeVisitor scopeVisitor = new VariableScopeVisitor(sourceUnit);
@@ -321,16 +319,12 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
         code.addStatement(ifElseS(equalsNullX(namedArgs),
                 illegalArgumentBlock(message),
                 processArgsBlock(cNode, namedArgs)));
-        ConstructorNode init = new ConstructorNode(modifiers, parameters, ClassNode.EMPTY_ARRAY, code);
-        markAsGenerated(cNode, init);
-        cNode.addConstructor(init);
+        addGeneratedConstructor(cNode, modifiers, parameters, ClassNode.EMPTY_ARRAY, code);
         // potentially add a no-arg constructor too
         if (addNoArg) {
             code = new BlockStatement();
             code.addStatement(stmt(ctorX(ClassNode.THIS, ctorX(LHMAP_TYPE))));
-            init = new ConstructorNode(modifiers, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, code);
-            markAsGenerated(cNode, init);
-            cNode.addConstructor(init);
+            addGeneratedConstructor(cNode, modifiers, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, code);
         }
     }
 
diff --git a/src/test/gls/enums/EnumTest.groovy b/src/test/gls/enums/EnumTest.groovy
index 0e72f0f..79da52f 100644
--- a/src/test/gls/enums/EnumTest.groovy
+++ b/src/test/gls/enums/EnumTest.groovy
@@ -424,6 +424,30 @@ class EnumTest extends CompilableTestSupport {
         """
     }
 
+    void testOverridingMethodsWithExplicitConstructor2() {
+        // GROOVY-6747
+        assertScript """
+            enum Codes {
+                YES('Y') {
+                    @Override String getCode() { /*string*/ }
+                },
+                NO('N') {
+                    @Override String getCode() { /*string*/ }
+                }
+
+                abstract String getCode()
+
+                private final String string
+
+                private Codes(String string) {
+                    this.string = string
+                }
+            }
+
+            assert Codes.YES.code == null // TODO: 'Y'
+        """
+    }
+
     void testAbstractMethodOverriding() {
         // GROOVY-4641
         assertScript """
@@ -600,6 +624,16 @@ class EnumTest extends CompilableTestSupport {
           '''
     }
 
+    void testEnumConstructorHasPrivateModifier() {
+        assertScript '''
+            enum Foo {
+                BAR, BAZ
+                Foo() {}
+            }
+            assert java.lang.reflect.Modifier.isPrivate(Foo.declaredConstructors[0].modifiers)
+        '''
+    }
+
     void testNestedEnumHasStaticModifier_GROOVY_8360() {
         assertScript '''
             class Foo {

[groovy] 03/05: GROOVY-7686: AIC: place local variable references above super ctor call

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 8f4baead5215c96b6cd08d4c218a11e6d9f0c849
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Mar 17 09:00:00 2021 -0500

    GROOVY-7686: AIC: place local variable references above super ctor call
    
    Conflicts:
    	src/main/java/org/codehaus/groovy/classgen/Verifier.java
    	src/test/gls/innerClass/InnerClassTest.groovy
---
 .../org/codehaus/groovy/classgen/Verifier.java     | 36 +++++++++++++++++++--
 src/test/gls/innerClass/InnerClassTest.groovy      | 37 ++++++++++++++++++++++
 2 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index 4a7e627..14e6e19 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -81,6 +81,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Map;
 import java.util.Set;
 
@@ -1050,8 +1051,6 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
             }
         }
 
-        statements.addAll(node.getObjectInitializerStatements());
-
         Statement code = constructorNode.getCode();
         BlockStatement block = new BlockStatement();
         List<Statement> otherStatements = block.getStatements();
@@ -1066,6 +1065,12 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                 // it is super(..) since this(..) is already covered
                 otherStatements.remove(0);
                 statements.add(0, firstStatement);
+                // GROOVY-7686: place local variable references above super ctor call
+                if (node instanceof InnerClassNode && ((InnerClassNode) node).isAnonymous()) {
+                    for (Statement stmt : extractVariableReferenceInitializers(statements)) {
+                        statements.add(0, stmt);
+                    }
+                }
             }
             Statement stmtThis$0 = getImplicitThis$0StmtIfInnerClass(otherStatements);
             if (stmtThis$0 != null) {
@@ -1073,8 +1078,12 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                 // that uses this$0, it needs to bubble up before the super call itself (GROOVY-4471)
                 statements.add(0, stmtThis$0);
             }
+            statements.addAll(node.getObjectInitializerStatements());
             statements.addAll(otherStatements);
+        } else {
+            statements.addAll(node.getObjectInitializerStatements());
         }
+
         BlockStatement newBlock = new BlockStatement(statements, block.getVariableScope());
         newBlock.setSourcePosition(block);
         constructorNode.setCode(newBlock);
@@ -1142,6 +1151,29 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         return null;
     }
 
+    private static List<Statement> extractVariableReferenceInitializers(List<Statement> statements) {
+        List<Statement> localVariableReferences = new ArrayList<>();
+        for (ListIterator<Statement> it = statements.listIterator(1); it.hasNext(); ) {
+            // the first statement is the super constructor call  ^
+
+            Statement stmt = it.next();
+            if (stmt instanceof ExpressionStatement
+                    && ((ExpressionStatement) stmt).getExpression() instanceof BinaryExpression) {
+                BinaryExpression expr = (BinaryExpression) ((ExpressionStatement) stmt).getExpression();
+
+                if (expr.getOperation().getType() == Types.ASSIGN
+                        && expr.getLeftExpression() instanceof FieldExpression
+                        && expr.getLeftExpression().getType().equals(ClassHelper.REFERENCE_TYPE)
+                        && (((FieldExpression) expr.getLeftExpression()).getField().getModifiers() & Opcodes.ACC_SYNTHETIC) != 0
+                        /* also could check if the right expression is a variable expression that references ctor parameter */) {
+                    localVariableReferences.add(stmt);
+                    it.remove();
+                }
+            }
+        }
+        return localVariableReferences;
+    }
+
     protected void addFieldInitialization(List list, List staticList, FieldNode fieldNode,
                                           boolean isEnumClassNode, List initStmtsAfterEnumValuesInit, Set explicitStaticPropsInEnum) {
         Expression expression = fieldNode.getInitialExpression();
diff --git a/src/test/gls/innerClass/InnerClassTest.groovy b/src/test/gls/innerClass/InnerClassTest.groovy
index 3a3a73c..73e7fb7 100644
--- a/src/test/gls/innerClass/InnerClassTest.groovy
+++ b/src/test/gls/innerClass/InnerClassTest.groovy
@@ -1001,6 +1001,43 @@ final class InnerClassTest {
         '''
     }
 
+    @Test // GROOVY-7686
+    void testReferencedVariableInAIC3() {
+        assertScript '''
+            abstract class A {
+                A() {
+                    m()
+                }
+                abstract void m();
+            }
+            void test() {
+                def v = false
+                def a = new A() {
+                    // run by super ctor
+                    @Override void m() {
+                        assert v != null
+                    }
+                }
+                v = true
+                a.m()
+            }
+            test()
+        '''
+    }
+
+    @Test // GROOVY-5754
+    void testResolveInnerOfSuperType() {
+        assertScript '''
+            interface I { class C { } }
+
+            class Outer implements I {
+                static class Inner extends C {}
+            }
+
+            print I.C
+        '''
+    }
+
     @Test // GROOVY-5989
     void testReferenceToOuterClassNestedInterface() {
         assertScript '''

[groovy] 04/05: GROOVY-9524: enum const class extends enum class, so no this$0 for calls

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit b16f6cd19987ba300247421dac0605c6f429696c
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Mar 17 18:01:56 2021 -0500

    GROOVY-9524: enum const class extends enum class, so no this$0 for calls
    
    Conflicts:
    	src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
---
 .../classgen/asm/sc/StaticInvocationWriter.java    |  3 +-
 src/test/groovy/bugs/Groovy9524.groovy             | 48 ++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
index b385ee4..93940a9 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
@@ -243,7 +243,8 @@ public class StaticInvocationWriter extends InvocationWriter {
             Expression fixedReceiver = receiver;
             if (implicitThis) {
                 if (!controller.isInClosure()) {
-                    fixedReceiver = new PropertyExpression(new ClassExpression(lookupClassNode), "this");
+                    if (!thisClass.isDerivedFrom(lookupClassNode))
+                        fixedReceiver = new PropertyExpression(new ClassExpression(lookupClassNode), "this");
                 } else if (thisClass != null) {
                     ClassNode current = thisClass.getOuterClass();
                     fixedReceiver = new VariableExpression("thisObject", current);
diff --git a/src/test/groovy/bugs/Groovy9524.groovy b/src/test/groovy/bugs/Groovy9524.groovy
new file mode 100644
index 0000000..e3dfa30
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy9524.groovy
@@ -0,0 +1,48 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.bugs
+
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+final class Groovy9524 {
+    @Test
+    void testEnumConstClassCallingPrivateMethod() {
+        assertScript '''
+            @groovy.transform.CompileStatic
+            class W {
+                enum X {
+                    Y {
+                        def z() {
+                            truncate('foo', 2)
+                        }
+                    }
+
+                    abstract def z()
+
+                    private String truncate(String input, int maxLength) {
+                        input.substring(0, maxLength)
+                    }
+                }
+            }
+            assert W.X.Y.z() == 'fo'
+        '''
+    }
+}