[jdk16] RFR: 8231461: static/instance overload leads to 'unexpected static method found in unbound lookup' when resolving method reference

[Maurizio Cimadamore]

On Tue, 12 Jan 2021 10:24:50 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> This patch fixes the following issue, for code like:
>> import java.util.function.*;
>>  
>> public class Test {
>>     public String foo(Object o) { return "foo"; }
>>     public static String foo(String o) { return "bar"; }
>> 
>>     public void test() {
>>         Function<String, String> f = Test::foo;
>>     }
>> }
>> javac is issuing an invalid method reference error even though the static method is applicable. This is due to a regression on the implementation of the following assertion on section 15.13.1 of the JLS 14:
>> 
>> If the first search produces a most specific method that is static , and the set
>> of applicable methods produced by the second search contains no non- static
>> methods, then the compile-time declaration is the most specified method of
>> the first search.
>> 
>> Apart from fixing the mentioned regression, this patch also adds a debug feature for the compiler to print out the findings during the search for an applicable method reference. The information is printed out if a debug flag is passed to the compiler.
>> 
>> TIA
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java line 3327:
> 
>> 3325:         @Override
>> 3326:         ReferenceLookupResult unboundResult(ReferenceLookupResult boundRes, ReferenceLookupResult unboundRes) {
>> 3327:             if (boundRes.isSuccess() && boundRes.sym.isStatic() &&
> 
> For consistency here I'd prefer to see `boundRes.hasKind(StaticKind.STATIC)` (which is set if the underlying symbol is static). E.g. I think the fix here is really adding the `isSuccess` part, not changing the static check.

That said - I'm having issue understanding how this has been fixed: if the fix search succeeds and the second search fails to produce a non-static method, then the first search should be preferred. It seems to me that the old code already covers that? Note that `isSuccess` is true whenever the kind != UNDEFINED, so if we check that kind should be STATIC, we already check that is a successful lookup and that the result is static. In other words, I don't see how the new code is different from the old?

-------------

PR: https://git.openjdk.java.net/jdk16/pull/107