RFR: JDK-8215482: check for cycles in type variables can provoke NPE

[Vicente Romero]

On 1/11/19 2:13 PM, Maurizio Cimadamore wrote:
>
> As per my suggestion to split into multiple stages, it was mostly a 
> suggestion to take full advantage of the tiered architecture of 
> TypeEnter. You are basically caching some types in a 'todo later' 
> list, and then you are coming back at them - avoiding these kind of 
> queuing is what the (relatively) new TypeEnter code is for.
>
> If the cyclic check is moved to a later phase, then I think you just 
> need to fetch the type variables from the symbol/type/tree and check 
> them; the types will be already set, no need to stash them into a map. 
> At least in theory :-)
>

right thanks for the suggestion, that's a better option, please see:

http://cr.openjdk.java.net/~vromero/8215482/webrev.01/

Vicente
>
> Maurizio
>
> On 11/01/2019 18:05, Vicente Romero wrote:
>> I have realized that cloning the type variable is not necessary as 
>> the Pair<JCTypeParameter, TypeVar> element in the table will keep a 
>> reference to the type variable
>>
>> Vicente
>>
>>
>> On 1/11/19 12:29 PM, Vicente Romero wrote:
>>>
>>>
>>> On 1/11/19 12:21 PM, Maurizio Cimadamore wrote:
>>>>
>>>> It seems that another thing that your path is doing is storing all 
>>>> the type vars to check keyed by outermost class, so that only when 
>>>> you have finished entering the outermost symbol you actually start 
>>>> checking all pending typevars for cycles. I guess this delay is 
>>>> necessary otherwise you would hit a problem anyway when checking Bc 
>>>> for cyclicity?
>>>>
>>>
>>> correct
>>>
>>>> Have you consider moving the attribution and cyclicity check in 
>>>> different type enter phases? For instance, leave attribution in 
>>>> HeaderPhase, but move the cycle check in MembersPhase ?
>>>>
>>>
>>> I didn't try, but why would this be preferable? also as annotation 
>>> processing can nuke the type we would have to make sure that the 
>>> types are there before doing the cycle check which is something we 
>>> can guarantee now by keeping the check for cycles in the Header phase
>>>
>>>> Maurizio
>>>>
>>>
>>> Vicente
>>>
>>>> On 11/01/2019 16:47, Vicente Romero wrote:
>>>>> Please review the fix for [1] at [2]. The NPE showed up in code like:
>>>>>
>>>>> class Outer<A extends Outer.Inner, B> {
>>>>>     class Inner<Bc extends B> {}
>>>>> }
>>>>>
>>>>> here attribution of type variable `A` during type enter phase will 
>>>>> trigger attribution of class `Inner` while type variable `B` 
>>>>> hasn't been attributed yet and thus its bound is still set to 
>>>>> null. A similar problem arose a while ago see [3]. The issue 
>>>>> provoking the current bug is that checks for cycles in type 
>>>>> variables are done as soon as the type variable is attributed but 
>>>>> in cases like the one above we can't do that until the type 
>>>>> variable for the outer class has been attributed too.
>>>>>
>>>>> My first try was to create a fixup table that maps the outer type 
>>>>> to the list of type variables defined by it or its members that 
>>>>> happen to be types too, and once the compiler finish entering the 
>>>>> outer class it would be safe to check for cycles in all the 
>>>>> concerning type variables. I had a mild success here as there were 
>>>>> some trivial cases that were working before that started failing. 
>>>>> I realized that it was because the annotation processing phase was 
>>>>> setting all the types to null, no bueno. So I decided to clone the 
>>>>> type variables to be stored in the fixup table and do the cycle 
>>>>> check on the clones which is what the current patch is doing.
>>>>>
>>>>> Thanks,
>>>>> Vicente
>>>>>
>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8215482
>>>>> [2] http://cr.openjdk.java.net/~vromero/8215482/
>>>>> [3] https://bugs.openjdk.java.net/browse/JDK-6660289
>>>
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20190111/64c76f8a/attachment.html>