RFR: JDK-8191981: javac crash when detecting lambda candidates

[Maurizio Cimadamore]

On 28/11/17 14:16, Jan Lahoda wrote:
> On 28.11.2017 14:13, Maurizio Cimadamore wrote:
>> Your fix looks fine - unfortunately the
>> 'ArguymentAttr.withLocalCacheContext' was created for this very reason,
>> but I think that approach is not sufficient to fully disable the cache.
>> It will simply avoid cache changes to propagate out - while here you
>> want something which basically disables the cache entirely.
>
> I think withLocalCacheContext isolates well the speculative analyzer 
> attribution from the outter attribution (and vice versa). But I think 
> the issue here is that inside the speculative analyzer attribution two 
> distinct trees are using the same slot, which leads to the clash in 
> the cache and the exception.
Sure - and as I said the patch is fine.

I guess if the cache was something other than just a Map, we could set 
it to be some object which never cached anything - forcing recomputation 
all the time - which would probably be fine for analyzer and the other 
cases where we don't care about the cache (and don't want to pollute it 
also).

Maurizio
>
> Jan
>
>>
>> Maurizio
>>
>>
>> On 28/11/17 11:46, Jan Lahoda wrote:
>>> Hi,
>>>
>>> Consider this code:
>>> ---
>>> public class LambdaWithMethod {
>>>     public static void run(Runnable r) {
>>>         run(new Runnable() {
>>>             public void run() {
>>>                 put(get());
>>>             }
>>>         });
>>>     }
>>>     private static String get() { return null; }
>>>     private static void put(String i) {}
>>> }
>>> ---
>>>
>>> When detecting lambda candidates, this crashes javac (please see the
>>> bug for full exception):
>>> ---
>>> $ javac -XDfind=lambda /tmp/LambdaWithMethod.java
>>> [snip]
>>> java.lang.AssertionError: Analyzer error when processing: [snip]
>>> at jdk.compiler/com.sun.tools.javac.util.Assert.error(Assert.java:162)
>>> at
>>> jdk.compiler/com.sun.tools.javac.comp.Analyzer.doAnalysis(Analyzer.java:562) 
>>>
>>>
>>> [snip]
>>> ---
>>>
>>> The underlying exception is:
>>> ---
>>> java.lang.ClassCastException:
>>> jdk.compiler/com.sun.tools.javac.tree.JCTree$JCMethodInvocation cannot
>>> be cast to jdk.compiler/com.sun.tools.javac.tree.JCTree$JCLambda
>>>         at
>>> jdk.compiler/com.sun.tools.javac.comp.ArgumentAttr$ExplicitLambdaType.dup(ArgumentAttr.java:456) 
>>>
>>>
>>>         at
>>> jdk.compiler/com.sun.tools.javac.comp.ArgumentAttr.processArg(ArgumentAttr.java:237) 
>>>
>>>
>>> [snip]
>>> ---
>>>
>>> (Full exception in the bug.)
>>>
>>> The problem appears to be that when the trial lambda node (that
>>> replaces the anonymous class) is created, there is no explicit
>>> position set, and so the last position set is used. And the
>>> ArgumentAttr.argumentTypeCache uses the position as key, and so the
>>> value for "get()" is (appears to be) used for the lambda, which leads
>>> to the crash.
>>>
>>> Bug; https://bugs.openjdk.java.net/browse/JDK-8191981
>>> Proposed patch: http://cr.openjdk.java.net/~jlahoda/8191981/webrev.00
>>>
>>> How does this look?
>>>
>>> Thanks,
>>>    Jan
>>>
>>>
>>