blackdrag commented on code in PR #2845:
URL: https://github.com/apache/groovy/pull/2845#discussion_r3889088589


##########
src/main/java/org/codehaus/groovy/ast/ClassNode.java:
##########
@@ -2034,6 +2053,28 @@ public List<ClassNode> getOuterClasses() {
         return result;
     }
 
+    /**
+     * For a JLS 4.5 rare type {@code Outer<T>.Inner}, the parameterized 
enclosing
+     * type. Distinct from {@link #getOuterClass()}, which is the enclosing 
class
+     * of a nested class <em>declaration</em>.
+     *
+     * @return the parameterized enclosing type, or {@code null} if this is 
not a rare type
+     * @since 6.0.0
+     */
+    public ClassNode getOuterClassType() {
+        return getNodeMetaData("outer.class");
+    }
+
+    /**
+     * Records the parameterized enclosing type of a JLS 4.5 rare type.
+     *
+     * @param outer the parameterized {@code Outer<T>} node; {@code null} 
clears it
+     * @since 6.0.0
+     */
+    public void setOuterClassType(final ClassNode outer) {
+        putNodeMetaData("outer.class", outer);

Review Comment:
   if you add a getter and setter for it, then we not a field? node meta data 
is for data the node does not know about and is "imposed" by outer processing. 
This change makes it known, thus nodemetadata is the wrong place.



##########
src/main/java/org/codehaus/groovy/classgen/asm/BytecodeHelper.java:
##########
@@ -466,12 +467,41 @@ private static void writeGenericsBoundType(StringBuilder 
ret, ClassNode printTyp
             ret.append(printType.getGenericsTypes()[0].getName());
             ret.append(";");
         } else {
-            ret.append(getTypeDescription(printType, false));
-            addSubTypes(ret, printType.getGenericsTypes(), "<", ">");
+            writeParameterizedClass(ret, printType);
             if (!isPrimitiveType(printType)) ret.append(";");
         }
     }
 
+    /**
+     * Writes a class type and its type arguments, using the JLS 4.5 nested 
form
+     * {@code LOuter&lt;...&gt;.Inner&lt;...&gt;} when an enclosing rare type 
is present.
+     */
+    private static void writeParameterizedClass(StringBuilder ret, ClassNode 
printType) {
+        ClassNode owner = printType.getOuterClassType();
+        if (owner != null) {
+            writeParameterizedClass(ret, owner);
+            ret.append('.');
+            ret.append(innerClassSimpleName(printType, owner));
+            addSubTypes(ret, printType.getGenericsTypes(), "<", ">");
+            return;
+        }
+        ret.append(getTypeDescription(printType, false));
+        addSubTypes(ret, printType.getGenericsTypes(), "<", ">");
+    }
+
+    private static String innerClassSimpleName(final ClassNode inner, final 
ClassNode owner) {
+        String innerName = inner.getName();
+        String ownerName = owner.getName();
+        if (innerName.startsWith(ownerName) && innerName.length() > 
ownerName.length()) {
+            char sep = innerName.charAt(ownerName.length());
+            if (sep == '.' || sep == '$') {
+                return innerName.substring(ownerName.length() + 
1).replace('$', '.');
+            }
+        }
+        int dot = Math.max(innerName.lastIndexOf('.'), 
innerName.lastIndexOf('$'));
+        return dot < 0 ? innerName : innerName.substring(dot + 1);

Review Comment:
   So the assumption seems to be that if the innerName does not start with 
ownerName, then it is a "plain" name. For example `Foo` as owner and `Foo$1` as 
inner.  
   1. why is it that one time we have innerName starting with owner and another 
time not? This feels like some kind of normalization is missing
   2. What if ower is `Foo` and inner is `Bar.X`? This will write `Foo.Bar.X` I 
assume, but is it legal? Considering that this means owner should have 
`Foo.Bar` since we do only then the recursive call, it seems that `Bar.X` is 
illegal for inner. But why can this not happen?



##########
src/main/java/org/codehaus/groovy/control/GenericsVisitor.java:
##########
@@ -159,11 +189,61 @@ public void visitDeclarationExpression(final 
DeclarationExpression expression) {
      */
     @Override
     public void visitArrayExpression(final ArrayExpression expression) {
+        ClassNode elementType = expression.getElementType();
+        if (!isReifiable(elementType)) {
+            addError("generic array creation", expression);
+        }
         checkGenericsUsage(expression.getType());
 
         super.visitArrayExpression(expression);
     }
 
+    /**
+     * JLS 15.8.2: a class literal may not name a type variable.
+     */
+    @Override
+    public void visitClassExpression(final ClassExpression expression) {
+        ClassNode type = expression.getType();
+        if (type.isGenericsPlaceHolder()) {
+            addError("Cannot select from a type parameter " + 
type.getUnresolvedName(), expression);
+        }
+        super.visitClassExpression(expression);
+    }
+
+    /**
+     * Groovy represents the type operand of {@code instanceof} as a
+     * {@link ClassExpression}, but it is not a class literal. Skip it so
+     * {@link InstanceOfVerifier} can diagnose JLS 15.20.2.
+     */
+    @Override
+    public void visitBinaryExpression(final BinaryExpression expression) {
+        if (expression.getOperation().isA(Types.INSTANCEOF_OPERATOR)
+                && expression.getRightExpression() instanceof ClassExpression) 
{

Review Comment:
   What happens if we do "x instanceof Map<String,Integer>"? Is this forbidden 
by the grammar?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to