RFR: 8325088: Overloads that differ in type parameters may be lost [v3]

[Pavel Rappo]

On Wed, 3 Apr 2024 23:29:44 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Pavel Rappo has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision:
>> 
>>  - Merge branch 'master' into 8325088
>>  - Remove registration phase
>>    
>>    Makes the code more robust and simple.
>>  - Merge branch 'master' into 8325088
>>  - Update copyright years
>>    
>>    Note: any commit hashes below might be outdated due to subsequent
>>    history rewriting (e.g. git rebase).
>>    
>>     - revert src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlIndexBuilder.java as spurious
>>     - revert test/langtools/jdk/javadoc/doclet/testMemberInheritance/TestMemberInheritance.java as spurious
>>     - revert test/langtools/jdk/javadoc/doclet/testMemberSummary/TestMemberSummary.java as spurious
>>     - revert test/langtools/jdk/javadoc/doclet/testNewLanguageFeatures/TestNewLanguageFeatures.java as spurious
>>     - revert test/langtools/jdk/javadoc/doclet/testOrdering/TestOrdering.java as spurious
>>     - revert test/langtools/jdk/javadoc/doclet/testOverriddenMethods/TestOverrideMethods.java as spurious
>>     - revert test/langtools/jdk/javadoc/doclet/testPrivateClasses/TestPrivateClasses.java as spurious
>>     - revert test/langtools/jdk/javadoc/doclet/testSeeTag/TestSeeTag.java as spurious
>>     - revert test/langtools/jdk/javadoc/doclet/testVisibleMembers/TestVisibleMembers.java as spurious
>>  - Use erased notation only when necessary
>>    
>>    Partially reverts 4f028269, which is the bulk of the previous solution,
>>    then adds a centralised ID registry for executable elements.
>>    
>>    The centralised registry is an alternative solution, which is more
>>    gentle and less disruptive to tests and composability (-link and
>>    -linkoffline).
>>  - Update copyright years
>>    
>>    Note: any commit hashes below might be outdated due to subsequent
>>    history rewriting (e.g. git rebase).
>>    
>>     + update src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlIndexBuilder.java due to 4f0282694fd
>>     + update test/langtools/jdk/javadoc/doclet/testMemberInheritance/TestMemberInheritance.java due to 4f0282694fd
>>     + update test/langtools/jdk/javadoc/doclet/testMemberSummary/TestMemberSummary.java due to 4f0282694fd
>>     + update test/langtools/jdk/javadoc/doclet/testNewLanguageFeatures/TestNewLanguageFeatures.java due to 4f0282694...
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlIds.java line 571:
> 
>> 569:         var list = Stream.concat(Stream.concat(ctors.stream(), methods.stream()), otherMethods.stream())
>> 570:                 .map(e1 -> (ExecutableElement) e1)
>> 571:                 .toList();
> 
> We can maybe do something like:
> 
> record ErasureCheck(ExecutableElement element, HtmlId erasure) {}
> 
> // ...
>     .map(e1 -> new ErasureCheck(e1, forErasure(e1)))
>     .collect(Collectors.groupingBy(check -> check.erasure == null));
> 
> // 1. Map all elements that can _only_ be addressed by the simple id
> for (var m : groups.get(true)) {...}

I addressed that in c7ea01c0cab. It looks leaner (`forErasure` is now called once), but is that as readable as you might have thought it would be?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18519#discussion_r1551585397