RFR: 8238735 NPE compiling lambda expression within conditional expression

[Adam Sotona]

Hi,
in this situation the second "recovery" round completely messes the results.
In the first run the result is correctly filled: result = that.type = types.createErrorType(pt()); and suppose to be returned,
however then another recovery is triggered and it  re-enters the whole visitLambda with totally messed initial values.
As a result the second evaluation returns Type.recoveryType constant, which I expect should be never returned and with no indication that the evaluation failed.
Type.recoveryType has missing .tsym field and that is causing NPE. 
My original idea was to simply avoid NPE by appropriate check, however I do not see any such similar code across javac.
I see many direct calls of Type.tsymin the code however no NPE protection anywhere. How it is guaranteed that instances of JCNoType (with null tsym) will never reach the code?

Then I tried to fix it by correcting the result of the recovery round, however that seems to be very complex as the initial values of the recovery may vary a lot and detect the "messed" situation and find what suppose to be returned in that case is very complex.

Then I realized that this is the situation where recovery should not happen as lambda without its FunctionDescriptor can hardly continue in compilation as it does not know anything about the expected content.

The two error messages I had to remove from the test after the fix didn't make any sense to me. From all aspect there are two errors in the two lambdas, however second round of recovery listed two more nonsense errors: "compiler.err.cant.resolve.location: kindname.class, NonExistentClass, , , (compiler.misc.location: kindname.class, TargetType43, null)"

BTW: Impact of the NPE is not just when used within conditional expression (as described in the 8238735), however it is silently killing javac with NPE anywhere you try to cast lambda expression that fails to lookup its FunctionDescriptor. For example just try to compile: class A {boolean b = (boolean)(() -> false);}

Thanks,
Adam

> On 20 May 2020, at 10:19, Adam Sotona <adam.sotona at oracle.com> wrote:
> 
> Hi,
> I would like to add results from Corpus compilation regression testing: the patch caused no regression in compilation of all Corpus sources :)
> 
> Thanks for review,
> Adam
> 
>> On 18 May 2020, at 17:47, Adam Sotona <adam.sotona at oracle.com> wrote:
>> 
>> Hi,
>> I would like to ask for review of the patch fixing NPE while compiling lambda expression within conditional expression.
>> The fix removes obsolete recovery cycle in com.sun.tools.javac.comp.Attr.visitLambda method when Types.FunctionDescriptorLookupError happens and it adds new test.
>> 
>> JBS:  https://bugs.openjdk.java.net/browse/JDK-8238735 
>> webrev: http://cr.openjdk.java.net/~asotona/8238735/webrev.00/
>> 
>> Mach 5 build with the patch passes all Tier1, Tier2 and Tier3 tests.
>> 
>> 
>> Thank you,
>> Adam
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20200520/c617e9f0/attachment.htm>