RFR: 8296171: Compiler incorrectly rejects code with variadic method references

[Maurizio Cimadamore]

On Thu, 10 Nov 2022 22:02:04 GMT, Vicente Romero <vromero at openjdk.org> wrote:

> Please review this PR that is syncing javac with the spec. Javac is rejecting this code:
> 
> 
> import java.util.function.*;
> interface Intf {
>     Object apply(String... args);
> }
>                 
> public class Test {
>     public static Object foo(Object o) { return "bar"; }
>     public final Object foo(Object... o) { return "foo"; }
>                 
>     public void test() {
>         Intf f = this::foo;
>     }
> }
> 
> 
> javac indicates that the method reference is invalid when according to the spec the second `foo` method should be selected by the compiler. The related section of the spec can be found at section `15.13.1 Compile-Time Declaration of a Method Reference`: 
> 
> 
>   – For all other forms of method reference expression, one search for a most
>   specific applicable method is performed. The search is as specified in
>   §15.12.2.2 through §15.12.2.5, with the clarifications below.
>   The method reference is treated as if it were an invocation with argument
>   expressions of types P 1 , ..., P n ; the type arguments, if any, are given by the
>   method reference expression.
>   If the search results in an error as specified in §15.12.2.2 through §15.12.2.5,
>   or if the most specific applicable method is static , there is no compile-time
>   declaration.
>   Otherwise, the compile-time declaration is the most specific applicable
>   method.
> 
> 
> 
> Actually the method search is correct but later on javac is seeing that both methods are applicable and given that they have different staticness, it is making wrong assumptions down the road.
> 
> TIA

Looks good - watch out for whitespaces

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java line 3114:

> 3112:         Symbol boundSym = lookupMethod(boundEnv, env.tree.pos(),
> 3113:                 site.tsym, boundSearchResolveContext, boundLookupHelper);
> 3114:         boolean isStaticSelector = TreeInfo.isStaticSelector(referenceTree.expr, names);

This is a good minimal fix I believe.
Moving forward (e.g. maybe in a separate PR) I wonder if it would be useful to make this code less general. E.g. the method reference lookup code comes from a time where we always did two lookups - but the JL:S has later been rectified, so that now we only do two lookups for unbound method references (e.g. those with static qualifier). I think that the current code is not the best in terms of achieving good JLS parity w/ a good readability, because it's trying to do too much. But of course that's an issue that was there before your fix.

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

Marked as reviewed by mcimadamore (Reviewer).

PR: https://git.openjdk.org/jdk/pull/11093