RFR: 8318160: javac does not reject private method reference with type-variable receiver [v7]

Vicente Romero vromero at openjdk.org
Wed Oct 18 18:45:19 UTC 2023

On Wed, 18 Oct 2023 11:00:32 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> I looked at the Resolve code some more, and also looked at the fix which likely caused this: 
>> https://bugs.openjdk.org/browse/JDK-8073842
>> So, `Resolve::findMethod` basically must NOT be called on a `site` Type that is a type variable. The original method resolution code enforces this in Attr::selectSym:
>> https://github.com/openjdk/jdk/blob/6fc35142315f1616fa35e415005c9483939c6920/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java#L4575
>> Note that here:
>> 1. If we see that the qualifier is a type variable, we recurse on the upper bound
>> 2. If we find that we ended up accessing a symbol that was private, replace the symbol with an access error
>> 3. call `accessBase` on the final symbol
>> I think a similar fix should be possible for method references. That is, as we do now, normalize the `site` using `skipTypeVars`, but also save the original `site` somewhere (as we already do). Note that the LookupHelper class has an `access` method, which is used to validate the result of the lookup (e.g. typically call `accessBase` on it). Effectively, this is a post-resolution step, pretty much like the one in `Attr`, so we can add whatever logic we want there (I think replicating the code in `Attr`  - see steps above - would be the best).
>> So, I don't think we need to add the new `isInherited` method - just overriding `access` to do the same thing we do in `Attr::selectSym` should be enough (I hope)
> In fact... looking more - I see that `ReferenceLookupHelper` already overrides `access`. But this class doesn't have `originalSite` so you can't add the new access check here. So, `MethodReferenceLookupHelper` would have to override, add the new access check and call the super method.

I gave it another try and updated the code. One of my experiments was on this line but not exactly what you are proposing and I was not very happy with the end result. I think now it should be good, thanks


PR Review Comment: https://git.openjdk.org/jdk/pull/16210#discussion_r1364357155

More information about the compiler-dev mailing list