[PATCH] 8133135: NPE on anonymous class defined by qualified instance creation expression with diamond

[bsrbnd]

Just a little precision;
clazz is reassigned with clazzid1 in method Attr.visitNewClass() (on
line 1973) which implies that it differs from tree.clazz:

        if (tree.encl != null) {
            // We are seeing a qualified new, of the form
            //    <expr>.new C <...> (...) ...

            [...]

            clazz = clazzid1;
        }


It is then passed to Attr.visitAnonymousClassDefinition() and used to
perform type inference.
This is why recurcivity condition could be made on clazz.type instead
of tree.clazz.type as shown in the first patch.

But, I was also wondering if tree.clazz should really differs from
clazz as we assign clazz.type to tree.clazz.type in
Attr.visitNewClass() on line 2088 before calling
Attr.visitAnonymousClassDefinition()?

                if (!constructorType.isErroneous()) {
                    tree.clazz.type = clazz.type =
constructorType.getReturnType();
                    tree.constructorType =
types.createMethodTypeWithReturn(constructorType, syms.voidType);
                }

If it shouldn't, tree.clazz have to match clazz and then recurcivity
condition on tree.clazz.type could be right...
This leads me to give next a second patch (only in
Attr.visitNewClass()) with a slightly different meaning as explained
above:

diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
@@ -1970,7 +1970,7 @@
                               ((JCTypeApply) clazz).arguments);
             }

-            clazz = clazzid1;
+            tree.clazz = clazz = clazzid1;
         }

         // Attribute clazz expression and store

Regards,
bsrbnd

2015-08-31 16:32 GMT+02:00 Maurizio Cimadamore <maurizio.cimadamore at oracle.com>:
> Thanks, I'll take a look.
>
> Maurizio
>
>
> On 31/08/15 14:40, bsrbnd wrote:
>>
>> Hi,
>>
>> As explained in issue 8133135, the following code produces a null
>> pointer exception because the symbol of the anonymous class is not set
>> during the attribute phase:
>>
>> class List<T> {}
>>
>> class ClassOut {
>>      class ClassIn<T> { public ClassIn() {}}
>> }
>>
>> public class TestBug {
>>      public static <T> List<T> m(List<T> list, T item) {return list;}
>>
>>
>>      public static void main(String[] args) {
>>          m(new List<ClassOut.ClassIn<String>>(), new ClassOut().new
>> ClassIn<>(){});
>>      }
>> }
>>
>> Method Attr.visitAnonymousClassDefinition() doesn't perform well type
>> inference with diamond. Recurcivity condition on tree.clazz.type seems
>> to be wrong because it never changes... instead, it should be made on
>> clazz.type (or clazztype) to go deeper on the type resolution.
>>
>> The following patch shows a possible solution, I think (to be checked
>> by someone who knows the code better than me...):
>>
>> diff --git
>> a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>> @@ -2173,7 +2173,7 @@
>>               final boolean isDiamond = TreeInfo.isDiamond(tree);
>>               if (isDiamond
>>                       && ((tree.constructorType != null &&
>> inferenceContext.free(tree.constructorType))
>> -                    || (tree.clazz.type != null &&
>> inferenceContext.free(tree.clazz.type)))) {
>> +                    || (clazz.type != null &&
>> inferenceContext.free(clazz.type)))) {
>>                   final ResultInfo resultInfoForClassDefinition =
>> this.resultInfo;
>>
>> inferenceContext.addFreeTypeListener(List.of(tree.constructorType,
>> tree.clazz.type),
>>                           instantiatedContext -> {
>>
>> Regards,
>>
>> bsrbnd
>
>