[ 
https://issues.apache.org/jira/browse/GROOVY-12319?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18109636#comment-18109636
 ] 

ASF GitHub Bot commented on GROOVY-12319:
-----------------------------------------

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?





> Java compatibility: remaining generic type syntax
> -------------------------------------------------
>
>                 Key: GROOVY-12319
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12319
>             Project: Groovy
>          Issue Type: Improvement
>            Reporter: Daniel Sun
>            Priority: Major
>
> Groovy already accepts most Java generics. Three Java forms still fail: two 
> in the parser, one in {{GenericsVisitor}}. Each has a local workaround.
> Method type arguments such as {{Helper.<String>identity( x )}} already work 
> and are not part of this request.
> h3. 1. Diamond {{<>}} on an anonymous class
> Java 9+ (JEP 213, JLS 15.9.5) allows diamond when creating an anonymous 
> class, if the target type supplies the type arguments:
> {code:java}
> Processor<String> p = new Processor<>() {
>     public String process(String val) { return val.toUpperCase(); }
> };
> {code}
> This should work in assignment, as a method argument, and under 
> {{@TypeChecked}} / {{@CompileStatic}}, for both interfaces and abstract 
> classes.
> *Actual:* Groovy rejects it with {{Cannot use diamond <> with anonymous inner 
> classes}}.
> *Workaround:* write the type arguments explicitly:
> {code:java}
> Processor<String> p = new Processor<String>() {
>     public String process(String val) { return val.toUpperCase(); }
> };
> {code}
> GROOVY-6730 and GROOVY-7159 were false-positive STC errors when diamond was 
> _not_ used. They did not add this form.
> h3. 2. Qualified parameterized inner types ("rare" types)
> Java allows an inner type to keep the enclosing type's arguments (JLS 4.5):
> {code:java}
> class Outer<T> {
>     class Inner<U> {}
> }
> Outer<String>.Inner<Integer> x = new Outer<String>().new Inner<Integer>("v", 
> 1);
> {code}
> The same qualification appears as a field or method type, nested in another 
> type argument, as a superclass when the member is a non-static class, and in 
> a qualified instance creation inside the outer class:
> {code:java}
> List<Outer<String>.Inner<Integer>> list;
> class Sub extends Outer<String>.Inner {
>     Sub(Outer<String> o) { o.super(); }
> }
> // inside Outer:
> new Outer<T>.Inner<U>(...)
> {code}
> Selecting a static member type from a parameterization is a compile-time 
> error (JLS 4.5.2 / 6.5.5). Nested interfaces and enums are implicitly static 
> (JLS 9.5), so a nested interface must be selected from the raw enclosing name:
> {code:java}
> class Impl implements Outer.Inner<Integer> { ... }           // legal
> class Bad  implements Outer<String>.Inner<Integer> { ... }   // error
> {code}
> *Actual:* Groovy fails to parse {{Outer<String>.Inner}} ({{Unexpected 
> input}}).
> *Workaround:* a factory that returns {{Inner}} without naming 
> {{Outer<T>.Inner}}.
> h3. 3. Explicit type arguments on constructors, {{this()}} and {{super()}}
> Java allows constructor type arguments independently of the class type 
> arguments (JLS 15.9 / 8.8.7.1):
> {code:java}
> class Box {
>     <T> Box(T t) {}
>     Box() { <String>this("x"); }
> }
> class Derived extends Box {
>     Derived() { <String>super("y"); }
> }
> class Outer {
>     class Inner {
>         <T> Inner(T t) {}
>     }
> }
> new <String>Box("x");
> new Outer().new <String>Inner("z");
> {code}
> *Actual:* Groovy fails at {{new <}} with {{Unexpected input: '<'}}.
> *Workaround:* inference, e.g. {{new Box("x")}}.
> Constructor type arguments are already tracked by GROOVY-10501. The 
> {{this()}} / {{super()}} / inner-{{new}} forms are the same JLS production 
> and should be handled together.
> h3. Expected
> All three forms compile in dynamic Groovy and under {{@TypeChecked}} / 
> {{@CompileStatic}}, matching javac on well-formed programs.
> The well-formedness rules are javac's (checked against javac 25). Parsing a 
> rare type does not make every use of it legal.
> These remain legal. {{Inner}} is a non-static member of {{Outer}}; 
> {{Outer<?>}} is reifiable (JLS 4.7), so a non-static member type of that 
> enclosing type is reifiable (JLS 15.10.1):
> {code:java}
> class Outer<T> {
>     class Inner {}
>     class InnerG<U> {}
>     interface Iface<U> { U id(U u); }
> }
> Outer<?>.Inner[] a = new Outer<?>.Inner[0];
> Outer<?>.InnerG<?>[] b = new Outer<?>.InnerG<?>[0];
> Outer<?>.Inner field;
> Outer<String>.Inner concrete;
> class Impl implements Outer.Iface<Integer> {
>     public Integer id(Integer u) { return u; }
> }
> {code}
> These remain compile errors:
> {code:java}
> new Object<>() {}                          // diamond on a non-generic type
> class C extends ArrayList<> {}             // diamond on a class declaration
> List<> list;                               // diamond on a field
> new <String>Box<>("x")                     // diamond combined with 
> constructor type arguments
> Outer<String, Integer>.Inner x;            // wrong arity
> x instanceof Outer<String>.Inner           // parameterized type is not 
> reifiable
> new Outer<String>.Inner[1]                 // generic array creation (JLS 
> 15.10.1)
> new Outer<?>.Nested[0]                     // static member from a 
> parameterized type (JLS 6.5.5)
> Outer<?>.Nested z;                         // same rule as a type name
> new Outer<?>.InnerG[0]                     // raw generic member of a 
> parameterized enclosing type
> class Bad implements Outer<String>.Iface<Integer> {} // nested interface is 
> implicitly static (JLS 9.5)
> new java.util.Map<?,?>.Entry[0]            // Map.Entry is a nested interface
> new Outer<?>().new Inner()                 // constructor type argument may 
> not be a wildcard
> {code}
> The {{Nested}} cases assume {{static class Nested}} inside {{Outer}}. The 
> {{InnerG}} array error is the raw-member form; the legal counterpart is {{new 
> Outer<?>.InnerG<?>[0]}} above.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to