RFR: JDK-8215482: check for cycles in type variables can provoke NPE
Maurizio Cimadamore
maurizio.cimadamore at oracle.com
Mon Jan 14 10:56:45 UTC 2019
Looks good!
Maurizio
On 11/01/2019 22:11, Vicente Romero wrote:
>
>
> 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/20190114/3d6343e8/attachment.html>
More information about the compiler-dev
mailing list