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

[Maurizio Cimadamore]

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

>> I made several experiments and decided for the approach in the last commit pushed. We will need an additional field in class `MethodResolutionContext` basically pointing to the current `LookupHelper` which can now decide if a symbol is inherited at a given site or not. Semantically there are not changes for any other method the only semantic change will be when we are looking for the method in a method reference.
>
> 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.

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

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