RFR: 8337980: Javac allows invocation of an inherited instance method from a static method [v3]

Jan Lahoda jlahoda at openjdk.org
Wed Oct 9 10:52:01 UTC 2024


On Tue, 8 Oct 2024 15:18:39 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

>> The method `Resolve.findFun()` performs the lookup for an unqualified method invocation like `foo()`. It does this by invoking `findMethod()` to find the method in each containing class scope, working from the inside out.
>> 
>> In this loop, as soon as it finds a matching method, but before returning, it performs the checks that prevent (a) invoking an instance method from a static context, and (b) invoking an instance method of the same class from an early-initialization context.
>> 
>> However, these checks don't account for the fact that in some cases `findMethod()` can return an `AmbiguityError` even though the code is perfectly valid. This is because `findMethod()` performs the "maximally specific" logic of JLS §15.12.2.5 ("Choosing the Most Specific Method"), but not the "preferred method" logic. Instead, in the case there are multiple "maximally specific" methods, they are all returned wrapped in an `AmbiguityError`; disambiguation is the caller's job.
>> 
>> When this happens, if the code is actually valid, there's no problem, but if the code has one of the errors (a) or (b), the error does not get detected, with resulting downstream chaos.
>> 
>> Here's an example of when `findMethod()` can return an `AmbiguityError` for perfectly valid code:
>> 
>> public class Bug {
>> 
>>     public interface A {
>>         int op();
>>     }
>> 
>>     public abstract static class B {
>>         public abstract int op();
>>     }
>> 
>>     public abstract static class C extends B implements A {
>> 
>>         public int test() {
>>             return op();   // findMethod("op") returns both A.op() and B.op() here
>>         }
>>     }
>> }
>> 
>> The fix in this PR is to add a few lines to `findFun()` to resolve the ambiguity (if possible) before performing checks (a) and (b). It's possible there is an opportunity for a more thorough refactoring here, but short of that, I tried to add a few helpful comments.
>
> Archie Cobbs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Refactor patch to resolve AmbiguityError's eagerly within findMethod().

Looks good to me.

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

Marked as reviewed by jlahoda (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20533#pullrequestreview-2356777862


More information about the compiler-dev mailing list