[PATCH] 8133135: NPE on anonymous class defined by qualified instance creation expression with diamond
Srikanth
srikanth.adayapalam at oracle.com
Mon Oct 5 03:09:40 UTC 2015
On Monday 07 September 2015 04:22 PM, Srikanth wrote:
> 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.)
Hello, sorry for the delay, I am finally back online full time to look into
this issue.
Unfortunately, the proposed patch fails some of the diamond tests so
cannot be accepted as is - I'll investigate a fix and take it forward.
Thanks!
Srikanth
>
> 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
>>>
>
More information about the compiler-dev
mailing list