RFR 8178150: Regression in logic for handling inference stuck constraints

[Maurizio Cimadamore]

Thanks - I'll address this (and maybe other comments). You are right 
that a simple for loop is probably more readable here (and better as it 
avoids stream creation).

I will upload an updated webrev once I accumulate enough comments :-)

Maurizio


On 19/10/17 12:04, Andrej Golovnin wrote:
> Hi Maurizio,
>
> in src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Infer.java
>
> 1768                         deps.stream().forEach(d ->
> d.closureInternal(closure));
>
> This can be replaced by a simple for-each loop. And if you really
> don't want to write a for-each loop, then you can can use the
> forEach-method defined in the Iterable interface:
>
> deps.forEach(d -> d.closureInternal(closure));
>
> and avoid creation of a stream.
>
> Best regards
> Andrej Golovnin
>
> On Thu, Oct 19, 2017 at 11:41 AM, Maurizio Cimadamore
> <maurizio.cimadamore at oracle.com> wrote:
>> Updated webrev
>>
>> http://cr.openjdk.java.net/~mcimadamore/8178150_v2/
>>
>> Maurizio
>>
>>
>> On 19/10/17 09:38, Maurizio Cimadamore wrote:
>>> I think you are right - this method was there when I first wrote the patch
>>> but then got removed. I resurrected it for the purpose of this patch, but it
>>> seems like it's not doing what we need.
>>>
>>> Maurizio
>>>
>>>
>>> On 19/10/17 00:57, Vicente Romero wrote:
>>>> Hi Maurizio,
>>>>
>>>> I'm not sure about the correctness or the objective of the closure()
>>>> method. The termination condition seems to be: stop as soon as you find a
>>>> cycle in the graph, at least this is my reading. But for some graphs this
>>>> could imply finding a subset of the closure not the whole of it. It seems to
>>>> me like implementing a graph traversal method should be what it is wanted
>>>> here. Unless there is something I'm missing.
>>>>
>>>> Also in the same method the javadoc could have stall comments as it says
>>>> that a kind of dependencies is given but there is no argument passed to the
>>>> method.
>>>>
>>>> Thanks,
>>>> Vicente
>>>>
>>>> On 10/18/2017 04:15 PM, Maurizio Cimadamore wrote:
>>>>> Hi,
>>>>> this issue has already been discussed in [1].
>>>>>
>>>>> The issue has to do with the fact that logic for picking a deferred node
>>>>> to unstick ignores the can-influence relationship (aka inference graph
>>>>> dependencies).
>>>>>
>>>>> The implemented code is not optimized (almost deliberately); I wanted
>>>>> the code to follow the spec more or less cleanly. While I'm open to small
>>>>> improvements, I would please ask to focus on correctness for the time being
>>>>> since (i) this code is not an hot execution path and (ii) other performance
>>>>> improvements in this area will follow, see [2].
>>>>>
>>>>> http://cr.openjdk.java.net/~mcimadamore/8178150/
>>>>>
>>>>> [1] -
>>>>> http://mail.openjdk.java.net/pipermail/compiler-dev/2017-October/011192.html
>>>>> [2] -
>>>>> http://mail.openjdk.java.net/pipermail/compiler-dev/2017-October/011126.html
>>>>>
>>>>> Cheers
>>>>> Maurizio
>>>>>