RFR: JDK-8205418: Assorted improvements to source code model

[Maurizio Cimadamore]

<snip>
>
>>
>> * Resolve - not sure I get the bestCandidate part. I mean, it seems you
>> want to check that all candidates point to the same symbol. But you
>> don't seem to look as to whether the candidate was applicable or not
>> (and the phase at which it was not applicable is discarded too). Sure
>> that errCandidate() isn't what you want here? errCandidate picks the
>> last non-applicable candidate (which has the higher resolution phase).
>> Note that when an InapplicableSymbolError is issued, there should always
>> be just _one_ inapplicable method, otherwise you'd get an
>> InapplicableSymbol_s_Error. So I'm not sure why you have that logic to
>> check that all candidates are the same.
>
> InapplicableSymbol_s_Error extends from InapplicableSymbolError, 
> right? So there may be multiple inapplicable symbols, I think. The 
> intent is here to try to recover in case where the expected symbol is 
> obvious, but to not a pick a candidate if there's any ambiguity possible.
Right, they are one subclass of the other - so, maybe 
InapplicableSymbol_S_ could override 'bestCandidate' and return null - 
at that point,  in InapplicableSymbol I think it's safe to have 
bestCandidate delegate to errCandidate.
>> * T8071432.out - I'd say the fact that one error is missing here is step
>> backwards? Is this caused by the removal of logging in the recovery 
>> info?
>
> For default/simplified errors, there is only one error message that is 
> the same before and after this patch:
> ---
> T8071432.java:47: error: incompatible types: cannot infer 
> type-variable(s) U
>         System.out.println(c.stream().reduce(0.0,
>                                             ^
>     (argument mismatch; bad return type in lambda expression
>       int cannot be converted to Double)
>   where U,T are type-variables:
>     U extends Object declared in method <U>reduce(U,BiFunction<U,? 
> super T,U>,BinaryOperator<U>)
>     T extends Object declared in interface Stream
> Note: Some messages have been simplified; recompile with 
> -Xdiags:verbose to get full output
> 1 error
> ---

When messages are simplified, basically we turn errors into other errors 
- e.g. look at InapplicableSymbol_S_Error::getDiagnostic - there is some 
logic and then, if there's really onlt one relevant candidate, we go 
back to an InapplicableSymbol (without S) error, where 'errCandidate' is 
overridden to return the filtered symbol.

Another reason to rely more on errCandidate :-)

Maurizio


>
> But it is true that for -Xdiags:verbose, there were two errors, and 
> with this patch there is only one, and the error that is missing is 
> probably the more useful one. I'll investigate what are the options.
>
> Thanks for the comments!
>
> Jan
>
>>
>> * IgnoreLambdaBodyDuringResolutionTest2.out - this time the removal of
>> the diagnostic seems more benign
>>
>> As for whether this should go in 11 or 12, I guess I don't have a strong
>> opinion. Some of the things you touch are sensitive, as Vicente
>> mentioned, but at the same time I can't see obvious ways as to why they
>> should impact correctness - but they will almost surely have some
>> non-trivial consequences on compiler diagnostics - which might even
>> perceived as regressions (see above).
>>
>> Maurizio
>>
>>
>> On 21/06/18 12:43, Jan Lahoda wrote:
>>> Hi,
>>>
>>> I'd like to propose a few improvements to the source code model as
>>> seen through the com.sun.source APIs.
>>>
>>> The changes include better error recovery, a fix end positions, and a
>>> fix for Trees.getScope when used with a TreePath that point to a block
>>> of a lambda expression, including cases where there's an error in the
>>> source code.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8205418
>>> Webrev: http://cr.openjdk.java.net/~jlahoda/8205418/webrev.00/
>>>
>>> How does this look?
>>>
>>> Thanks,
>>>    Jan
>>