8059632: Method reference compilation uses incorrect qualifying type #10809
Conversation
|
👋 Welcome back sadayapalam! A progress list of the required criteria for merging this PR into |
| // would refer to the type of the receiver and not the type of the declaring interface, | ||
| // per JLS 13.1 (see "the qualifying type of the method invocation"). | ||
|
|
||
| Class.forName("MethodRefQualifyingTypeTest$MethodInvoker").getMethod("invoke").invoke(null); |
There was a problem hiding this comment.
This test is actually derived from a "failing" JCK test: lang/LMBD/lmbd171/lmbd17101m51/lmbd17101m51.html.lmbd17101m51 that incorrectly asserts that there be a linkage error
|
@sadayapalam The following label will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
mcimadamore
left a comment
mcimadamore
left a comment
There was a problem hiding this comment.
Looks very good to me. Nice unification of the code, the spec is quite clear on this (binary form of method references and method invocation should be the same).
| return sym; | ||
| // array clone can be qualified by the array type in later targets | ||
| Symbol qualifier; | ||
| if ((qualifier = qualifiedSymbolCache.get(site)) == null) { |
There was a problem hiding this comment.
I know this code has been moved here from Gen - I'd be very very curious to understand if the caching mechanism actually does anything meaningful performance-wise. Seems just a way to add more state.
There was a problem hiding this comment.
Needs a CSR, given the potential change to the set of programs that might run correctly.
|
@sadayapalam This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
|
/csr needed |
|
@sadayapalam has indicated that a compatibility and specification (CSR) request is needed for this pull request. @sadayapalam please create a CSR request for issue JDK-8059632 with the correct fix version. This pull request cannot be integrated until the CSR request is approved. |
| public static void invoke() { | ||
| MyFunctionalInterface instance = null; | ||
| MethodSupplierImpl ms = new MethodSupplierImpl() { | ||
| public int m(int a){ |
There was a problem hiding this comment.
| public int m(int a){ | |
| public int m(int a) { |
openjdk
bot
added
ready
|
/integrate |
|
Going to push as commit 544e317. |
openjdk
bot
removed
ready
|
@sadayapalam Pushed as commit 544e317. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
3 participants
Align with JLS 13.1 (the qualifying class or interface of the method invocation) by
referring to the type of the receiver rather than the type of the declaring
class/interface in the bootstrap attribute
Progress
Issues
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10809/head:pull/10809$ git checkout pull/10809Update a local copy of the PR:
$ git checkout pull/10809$ git pull https://git.openjdk.org/jdk pull/10809/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 10809View PR using the GUI difftool:
$ git pr show -t 10809Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10809.diff