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

[Srikanth]

Hello !

Thanks for the patch. I can help take it forward. (I implemented
<> with anonymous classes - I have been away on leave for some
time and expect to have some cycles beginning this week.)

Thanks!
Srikanth

On Saturday 05 September 2015 08:49 PM, bsrbnd wrote:
> 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
>>