8278373: JavacTrees.searchMethod finds incorrect match

[lahodaj]

Currently, when javac encounters a javadoc reference, like @see PrintStream#println(int), will first try to find a method println in PrintStream using subtyping on the argument types, which may find another overload of the method with an argument that is a subtype of int - like println(double). Consequently, the link in the javadoc may be to a wrong method.

In this patch, the proposal is to use the subtype search only as a backup option, using the existing check based on isSameType first, and only doing an inexact match using subtyping if the more exact match fails to find a method. This fallback should help possible existing broken references to still work as before, while the preferred use of the more exact match should select the correct method in usual correct cases.

This patch fixes some instances of incorrect references in the JDK's javadoc, a diff of the generated javadocs for the JDK mainline is here:
http://cr.openjdk.java.net/~jlahoda/8278373/JDK-8278373.diff

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8278373: JavacTrees.searchMethod finds incorrect match

Reviewers

• Vicente Romero (@vicente-romero-oracle - Reviewer) ⚠️ Review applies to 8c0ad7f
• Jonathan Gibbons (@jonathan-gibbons - Reviewer) ⚠️ Review applies to 8c0ad7f

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk18 pull/79/head:pull/79
$ git checkout pull/79

Update a local copy of the PR:
$ git checkout pull/79
$ git pull https://git.openjdk.java.net/jdk18 pull/79/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 79

View PR using the GUI difftool:
$ git pr show -t 79

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk18/pull/79.diff

[bridgekeeper]

👋 Welcome back jlahoda! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@lahodaj The following labels will be automatically applied to this pull request:

• compiler
• javadoc

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 01: Full - Incremental (0c88aa60)
• 00: Full (8c0ad7f0)

[vicente-romero-oracle]

will we need a release note for this bug? it could be that some user's APIs could be affected by this change

[vicente-romero-oracle]

looks sensible

[openjdk]

@lahodaj 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:

8278373: JavacTrees.searchMethod finds incorrect match

Reviewed-by: vromero, jjg

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 1 new commit pushed to the master branch:

• 9d43d25: 8278897: Alignment of heap segments is not enforced correctly

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[jonathan-gibbons]

Generally good; I think this is the right fix in the current situation and for potential back porting. It provides improved but compatible behavior.

The API diffs were an unexpected "bonus", if that is the right word. It's better to know about and fix issues than to not know about them. I've spot checked a few of them and all seem reasonable. The fix is especially obvious when the link and the clear-text are different and meant to be the same. The one thing I would suggest is that because a lot of the api diffs are related to "subtyping" between primitive types, we might want to add some test cases for that into the test as well. Here is a random example of the kind of diff I'm talking about:

- as if by calling <a href="../java.base/java/lang/Math.html#max(double,double)"><code>Math.max</code></a>.</div>
+ as if by calling <a href="../java.base/java/lang/Math.html#max(float,float)"><code>Math.max</code></a>.</div>

Further out (JDK 19?) I think we should investigate the need for the strict == false case and/or give warnings (in javadoc) when the lookup is not exact. We should fix any cases in the JDK docs where that is the case, and/or consider removing support for strict == false or else make it an opt-in behavior somehow.

[jonathan-gibbons]

[snip]

Further out (JDK 19?) I think we should investigate the need for the strict == false case and/or give warnings (in javadoc) when the lookup is not exact. We should fix any cases in the JDK docs where that is the case, and/or consider removing support for strict == false or else make it an opt-in behavior somehow.

Filed JDK-8279474

[lahodaj]

I've filled a release note here:
https://bugs.openjdk.java.net/browse/JDK-8279503

And updated the patch to include primitive types and methods with multiple parameters: 34e421b

Feedback is welcome!

[vicente-romero-oracle]

latest change looks good to me

[lahodaj]

/integrate

[openjdk]

Going to push as commit 642ab34.
Since your change was applied there have been 7 commits pushed to the master branch:

• d65c665: 8279527: Dereferencing segments backed by different scopes leads to pollution
• 967ef0c: 8278020: ~13% variation in Renaissance-Scrabble
• 7c792f2: 8279333: Some JFR tests do not accept 'GCLocker Initiated GC' as a valid GC Cause
• 564c8c6: 8279529: ProblemList java/nio/channels/DatagramChannel/ManySourcesAndTargets.java on macosx-aarch64
• 590fa9d: 8278612: [macos] test/jdk/java/awt/dnd/RemoveDropTargetCrashTest crashes with VoiceOver on macOS
• 5cd9515: 8279525: ProblemList java/awt/GraphicsDevice/CheckDisplayModes.java on macosx-aarch64
• 9d43d25: 8278897: Alignment of heap segments is not enforced correctly

Your commit was automatically rebased without conflicts.

[openjdk]

@lahodaj Pushed as commit 642ab34.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.