Re: RFR: 8338411: Implement JEP 486: Permanently Disable the Security Manager
On Mon, 14 Oct 2024 13:52:24 GMT, Sean Mullan wrote: > This is the implementation of JEP 486: Permanently Disable the Security > Manager. See [JEP 486](https://openjdk.org/jeps/486) for more details. The > [CSR](https://bugs.openjdk.org/browse/JDK-8338412) describes in detail the > main changes in the JEP and also includes an apidiff of the specification > changes. > > NOTE: the majority (~95%) of the changes in this PR are test updates > (removal/modifications) and API specification changes, the latter mostly to > remove `@throws SecurityException`. The remaining changes are primarily the > removal of the `SecurityManager`, `Policy`, `AccessController` and other > Security Manager API implementations. There is very little new code. > > The code changes can be broken down into roughly the following categories: > > 1. Degrading the behavior of Security Manager APIs to either throw Exceptions > by default or provide an execution environment that disallows access to all > resources by default. > 2. Changing hundreds of methods and constructors to no longer throw a > `SecurityException` if a Security Manager was enabled. They will operate as > they did in JDK 23 with no Security Manager enabled. > 3. Changing the `java` command to exit with a fatal error if a Security > Manager is enabled. > 4. Removing the hotspot native code for the privileged stack walk and the > inherited access control context. The remaining hotspot code and tests > related to the Security Manager will be removed immediately after integration > - see [JDK-8341916](https://bugs.openjdk.org/browse/JDK-8341916). > 5. Removing or modifying hundreds of tests. Many tests that tested Security > Manager behavior are no longer relevant and thus have been removed or > modified. > > There are a handful of Security Manager related tests that are failing and > are at the end of the `test/jdk/ProblemList.txt`, > `test/langtools/ProblemList.txt` and `test/hotspot/jtreg/ProblemList.txt` > files - these will be removed or separate bugs will be filed before > integrating this PR. > > Inside the JDK, we have retained calls to > `SecurityManager::getSecurityManager` and `AccessController::doPrivileged` > for now, as these methods have been degraded to behave the same as they did > in JDK 23 with no Security Manager enabled. After we integrate this JEP, > those calls will be removed in each area (client-libs, core-libs, security, > etc). > > I don't expect each reviewer to review all the code changes in this JEP. > Rather, I advise that you only focus on the changes for the area > (client-libs, core-libs, net, security, etc) that you are most f... @seanjmullan I think you can use many lines of command in one github comment, like - PR Comment: https://git.openjdk.org/jdk/pull/21498#issuecomment-2411850563
Re: RFR: 8340203: Link color is hard to distinguish from text color in API documentation
On Tue, 8 Oct 2024 14:28:44 GMT, Hannes Wallnöfer wrote: > Please review a CSS-only change to make it easier to recognize links in API > documentation with the default stylesheet. > > This subtle change makes the link color a bit lighter and more saturated and > normal text a bit darker. The new link color is `#437291` which is the same > color used for links on [openjdk.org](https://openjdk.org/) and other OpenJDK > sites. > > The table of contents and breadcrumb-navigation uses a slightly darker color > closer to the original one, which works better with the gray background and > the large amount of links. > > The change also introduces CSS variables (aka custom properties) for the TOC > colors which have been missing, and removes some redundant CSS rules for the > TOC header. > > I've uploaded API docs for `java.base` to compare the changes: > > New colors: > https://cr.openjdk.org/~hannesw/8340203/api.00/java.base/java/lang/Thread.html > Old colors: > https://download.java.net/java/early_access/jdk24/docs/api/java.base/java/lang/Thread.html The contrast indeed has improved, especially for inlined non-code fonts. - PR Review: https://git.openjdk.org/jdk/pull/21409#pullrequestreview-2354743752
Re: RFR: 8325090: javadoc fails when -subpackages option is used with non-modular -source [v2]
On Thu, 26 Sep 2024 09:48:52 GMT, Hannes Wallnöfer wrote: >> Please review a simple fix to prevent `java.lang.AssertionError` to be >> thrown when the `javadoc` `-subpackages` option is used with a `-source` >> release that doesn't support modules. > > Hannes Wallnöfer has updated the pull request incrementally with one > additional commit since the last revision: > > Align indentation Looks reasonable to me. Looking at javac, it seems source version is the only way to toggle module feature to reproduce this. - Marked as reviewed by liach (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/21201#pullrequestreview-2331331766
Integrated: 8339115: Rename TypeKind enum constants to follow code style
On Tue, 27 Aug 2024 22:15:17 GMT, Chen Liang wrote: > `TypeKind` enum constants are named in wrong code style; correct them before > finalization. > > Also improved `TypeKind` specification to talk about not mentioned > `returnType`, `void`, and subword types being erased to int (and how). See > the associated CSR too. > > See the HTML output for the changed `JSpec` taglet's `6.5.newarray` > rendering: > https://cr.openjdk.org/~liach/8339115-typekind/java.base/java/lang/classfile/TypeKind.html > > Note: when reviewing, please use > https://cr.openjdk.org/~iris/se/23/spec/draft/java-se-23-draft-spec-37/ > instead of JVMS 22 for reference; JVMS 23 has fixed a few ambiguities about > subword types being absent from JVM's stack and locals. This pull request has now been integrated. Changeset: 25e03b52 Author:Chen Liang URL: https://git.openjdk.org/jdk/commit/25e03b52094f46f89f2fe8f20e7e5622928add5f Stats: 753 lines in 38 files changed: 116 ins; 29 del; 608 mod 8339115: Rename TypeKind enum constants to follow code style Reviewed-by: asotona - PR: https://git.openjdk.org/jdk/pull/20737
Re: RFR: 8339115: Rename TypeKind enum constants to follow code style [v4]
On Thu, 29 Aug 2024 22:33:34 GMT, Chen Liang wrote: >> `TypeKind` enum constants are named in wrong code style; correct them before >> finalization. >> >> Also improved `TypeKind` specification to talk about not mentioned >> `returnType`, `void`, and subword types being erased to int (and how). See >> the associated CSR too. >> >> See the HTML output for the changed `JSpec` taglet's `6.5.newarray` >> rendering: >> https://cr.openjdk.org/~liach/8339115-typekind/java.base/java/lang/classfile/TypeKind.html >> >> Note: when reviewing, please use >> https://cr.openjdk.org/~iris/se/23/spec/draft/java-se-23-draft-spec-37/ >> instead of JVMS 22 for reference; JVMS 23 has fixed a few ambiguities about >> subword types being absent from JVM's stack and locals. > > Chen Liang 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 six additional commits since > the last revision: > > - Reorder float and long to match instruction order, fix compilation > - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typekind > - space > - More fixes, reorder constants > - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typekind > - 8339115: Rename TypeKind enum constants to follow code style Requesting a review because I reordered `LONG` and `FLOAT`. They are now in the order of the instructions. - PR Comment: https://git.openjdk.org/jdk/pull/20737#issuecomment-2321289110
Re: RFR: 8339115: Rename TypeKind enum constants to follow code style [v4]
> `TypeKind` enum constants are named in wrong code style; correct them before > finalization. > > Also improved `TypeKind` specification to talk about not mentioned > `returnType`, `void`, and subword types being erased to int (and how). See > the associated CSR too. > > See the HTML output for the changed `JSpec` taglet's `6.5.newarray` > rendering: > https://cr.openjdk.org/~liach/8339115-typekind/java.base/java/lang/classfile/TypeKind.html > > Note: when reviewing, please use > https://cr.openjdk.org/~iris/se/23/spec/draft/java-se-23-draft-spec-37/ > instead of JVMS 22 for reference; JVMS 23 has fixed a few ambiguities about > subword types being absent from JVM's stack and locals. Chen Liang 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 six additional commits since the last revision: - Reorder float and long to match instruction order, fix compilation - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typekind - space - More fixes, reorder constants - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typekind - 8339115: Rename TypeKind enum constants to follow code style - Changes: - all: https://git.openjdk.org/jdk/pull/20737/files - new: https://git.openjdk.org/jdk/pull/20737/files/41f81fe8..3ab23576 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20737&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20737&range=02-03 Stats: 2553 lines in 73 files changed: 2112 ins; 146 del; 295 mod Patch: https://git.openjdk.org/jdk/pull/20737.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20737/head:pull/20737 PR: https://git.openjdk.org/jdk/pull/20737
Re: RFR: 8339115: Rename TypeKind enum constants to follow code style [v3]
On Thu, 29 Aug 2024 07:07:13 GMT, Adam Sotona wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> space
>
> src/java.base/share/classes/java/lang/classfile/CodeBuilder.java line 564:
>
>> 562: case INT -> {
>> 563: switch (computationalFrom) {
>> 564: case FLOAT -> f2i();
>
> Actual implementation shows complete conversion table as nested switches.
> What is the advantage of the proposed change?
Smaller switches reduce code size. A single switch case in the range adds 4
bytes to the size. This may be further optimized later.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20737#discussion_r1736034410
Re: RFR: 8339115: Rename TypeKind enum constants to follow code style [v3]
> `TypeKind` enum constants are named in wrong code style; correct them before > finalization. > > Also improved `TypeKind` specification to talk about not mentioned > `returnType`, `void`, and subword types being erased to int (and how). See > the associated CSR too. > > See the HTML output for the changed `JSpec` taglet's `6.5.newarray` > rendering: > https://cr.openjdk.org/~liach/8339115-typekind/java.base/java/lang/classfile/TypeKind.html > > Note: when reviewing, please use > https://cr.openjdk.org/~iris/se/23/spec/draft/java-se-23-draft-spec-37/ > instead of JVMS 22 for reference; JVMS 23 has fixed a few ambiguities about > subword types being absent from JVM's stack and locals. Chen Liang has updated the pull request incrementally with one additional commit since the last revision: space - Changes: - all: https://git.openjdk.org/jdk/pull/20737/files - new: https://git.openjdk.org/jdk/pull/20737/files/b7c499fa..41f81fe8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20737&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20737&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/20737.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20737/head:pull/20737 PR: https://git.openjdk.org/jdk/pull/20737
Re: RFR: 8339115: Rename TypeKind enum constants to follow code style [v2]
> `TypeKind` enum constants are named in wrong code style; correct them before > finalization. > > Also improved `TypeKind` specification to talk about not mentioned > `returnType`, `void`, and subword types being erased to int (and how). See > the associated CSR too. > > See the HTML output for the changed `JSpec` taglet's `6.5.newarray` > rendering: > https://cr.openjdk.org/~liach/8339115-typekind/java.base/java/lang/classfile/TypeKind.html > > Note: when reviewing, please use > https://cr.openjdk.org/~iris/se/23/spec/draft/java-se-23-draft-spec-37/ > instead of JVMS 22 for reference; JVMS 23 has fixed a few ambiguities about > subword types being absent from JVM's stack and locals. Chen Liang 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 three additional commits since the last revision: - More fixes, reorder constants - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typekind - 8339115: Rename TypeKind enum constants to follow code style - Changes: - all: https://git.openjdk.org/jdk/pull/20737/files - new: https://git.openjdk.org/jdk/pull/20737/files/f686716f..b7c499fa Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20737&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20737&range=00-01 Stats: 3432 lines in 65 files changed: 2324 ins; 537 del; 571 mod Patch: https://git.openjdk.org/jdk/pull/20737.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20737/head:pull/20737 PR: https://git.openjdk.org/jdk/pull/20737
RFR: 8339115: Rename TypeKind enum constants to follow code style
`TypeKind` enum constants are named in wrong code style; correct them before finalization. Also improved `TypeKind` specification to talk about not mentioned `returnType`, `void`, and subword types being erased to int (and how). See the associated CSR too. - Commit messages: - 8339115: Rename TypeKind enum constants to follow code style Changes: https://git.openjdk.org/jdk/pull/20737/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20737&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8339115 Stats: 664 lines in 31 files changed: 70 ins; 14 del; 580 mod Patch: https://git.openjdk.org/jdk/pull/20737.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20737/head:pull/20737 PR: https://git.openjdk.org/jdk/pull/20737
Integrated: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation
On Fri, 19 Jul 2024 01:55:57 GMT, Chen Liang wrote: > `TypeAnnotation` is not an annotation, as it should not be used in places > like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an > annotation at a given location instead of to be an annotation. > > Depends on #20205. This pull request has now been integrated. Changeset: 961e944f Author:Chen Liang URL: https://git.openjdk.org/jdk/commit/961e944fa731dc84be2764c01e4b326187474605 Stats: 374 lines in 20 files changed: 131 ins; 114 del; 129 mod 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation Co-authored-by: Alex Buckley Reviewed-by: asotona - PR: https://git.openjdk.org/jdk/pull/20247
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v11]
On Fri, 16 Aug 2024 15:28:03 GMT, Chen Liang wrote: >> `TypeAnnotation` is not an annotation, as it should not be used in places >> like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an >> annotation at a given location instead of to be an annotation. >> >> Depends on #20205. > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 20 commits: > > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/typeanno-model > - Missed JLS prefix from web review > - More bad terms and redundancy > - Fix terminology > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/typeanno-model > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/typeanno-model > - Stage > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/typeanno-model > - remove compile, use element-value, break long sentences > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/typeanno-model > - ... and 10 more: https://git.openjdk.org/jdk/compare/07352c67...802775ee Thanks for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/20247#issuecomment-2293747699
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v10]
On Tue, 13 Aug 2024 19:56:22 GMT, Chen Liang wrote: >> `TypeAnnotation` is not an annotation, as it should not be used in places >> like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an >> annotation at a given location instead of to be an annotation. >> >> Depends on #20205. > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > Missed JLS prefix from web review Swallowed the 2 conflicts from #20513: The tags are already correct in this patch. - PR Comment: https://git.openjdk.org/jdk/pull/20247#issuecomment-2293712126
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v11]
> `TypeAnnotation` is not an annotation, as it should not be used in places > like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an > annotation at a given location instead of to be an annotation. > > Depends on #20205. Chen Liang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 20 commits: - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - Missed JLS prefix from web review - More bad terms and redundancy - Fix terminology - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - Stage - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - remove compile, use element-value, break long sentences - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - ... and 10 more: https://git.openjdk.org/jdk/compare/07352c67...802775ee - Changes: https://git.openjdk.org/jdk/pull/20247/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=10 Stats: 374 lines in 20 files changed: 131 ins; 114 del; 129 mod Patch: https://git.openjdk.org/jdk/pull/20247.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20247/head:pull/20247 PR: https://git.openjdk.org/jdk/pull/20247
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v10]
> `TypeAnnotation` is not an annotation, as it should not be used in places > like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an > annotation at a given location instead of to be an annotation. > > Depends on #20205. Chen Liang has updated the pull request incrementally with one additional commit since the last revision: Missed JLS prefix from web review - Changes: - all: https://git.openjdk.org/jdk/pull/20247/files - new: https://git.openjdk.org/jdk/pull/20247/files/c66c7d25..8af07782 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=09 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=08-09 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/20247.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20247/head:pull/20247 PR: https://git.openjdk.org/jdk/pull/20247
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v9]
> `TypeAnnotation` is not an annotation, as it should not be used in places > like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an > annotation at a given location instead of to be an annotation. > > Depends on #20205. Chen Liang 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 18 additional commits since the last revision: - More bad terms and redundancy - Fix terminology - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - Stage - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - remove compile, use element-value, break long sentences - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - Improve docs for repeating, default, and value name - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - ... and 8 more: https://git.openjdk.org/jdk/compare/f09e48cb...c66c7d25 - Changes: - all: https://git.openjdk.org/jdk/pull/20247/files - new: https://git.openjdk.org/jdk/pull/20247/files/3a91a3a5..c66c7d25 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=08 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=07-08 Stats: 15357 lines in 539 files changed: 6184 ins; 6850 del; 2323 mod Patch: https://git.openjdk.org/jdk/pull/20247.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20247/head:pull/20247 PR: https://git.openjdk.org/jdk/pull/20247
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v8]
One specification question for Alex: we call an annotation interface's element
"element". What about the element-value pair that binds a value to an interface
element in an annotation? Can we call this element-value pair an "element", or
is there any other more proper name that is still concise? For context:
> [This annotation] is an array element of an array-valued element named {@code
> value} in a container annotation
From: core-libs-dev on behalf of Adam Sotona
Sent: Friday, August 2, 2024 8:55 AM
To: compiler-...@openjdk.org ;
core-libs-...@openjdk.org ; javadoc-dev@openjdk.org
Subject: Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an
Annotation [v8]
On Fri, 2 Aug 2024 00:04:48 GMT, Chen Liang wrote:
>> `TypeAnnotation` is not an annotation, as it should not be used in places
>> like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an
>> annotation at a given location instead of to be an annotation.
>>
>> Depends on #20205.
>
> Chen Liang 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 12 additional commits since
> the last revision:
>
> - remove compile, use element-value, break long sentences
> - Merge branch 'master' of https://github.com/openjdk/jdk into
> fix/typeanno-model
> - Improve docs for repeating, default, and value name
> - Merge branch 'master' of https://github.com/openjdk/jdk into
> fix/typeanno-model
> - Merge branch 'master' of https://github.com/openjdk/jdk into
> fix/typeanno-model
> - More refinements from alex
> - Artifact -> construct
> - More about Annotation, add equals note
> - Further refine wording
> - Refine the spec of TypeAnnotation per Alex feedback
> - ... and 2 more: https://git.openjdk.org/jdk/compare/f3707e9b...3a91a3a5
Marked as reviewed by asotona (Reviewer).
-
PR Review: https://git.openjdk.org/jdk/pull/20247#pullrequestreview-2215642863
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v7]
On Thu, 1 Aug 2024 20:02:54 GMT, Chen Liang wrote:
>> `TypeAnnotation` is not an annotation, as it should not be used in places
>> like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an
>> annotation at a given location instead of to be an annotation.
>>
>> Depends on #20205.
>
> Chen Liang 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 10 additional commits since
> the last revision:
>
> - Improve docs for repeating, default, and value name
> - Merge branch 'master' of https://github.com/openjdk/jdk into
> fix/typeanno-model
> - Merge branch 'master' of https://github.com/openjdk/jdk into
> fix/typeanno-model
> - More refinements from alex
> - Artifact -> construct
> - More about Annotation, add equals note
> - Further refine wording
> - Refine the spec of TypeAnnotation per Alex feedback
> - SealedGraph now redundant
> - 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation
I have implemented the suggestions (compiled to -> represented by, key-value ->
element value) and tried to simplify the wording. Please review the current
version:
*
* In an annotation in Java source code, elements in the annotation interface
* with default values may not be represented unless an explicit value is
provided.
* ({@jls 9.6.2}) The default value is derived from the {@link
AnnotationDefaultAttribute
* AnnotationDefault} attribute on the method representing the annotation
* interface element, in the class file representing the annotation interface.
*
* Multiple annotations of the same interface A in Java source code
* ({@jls 9.7.5}) are represented by the {@linkplain
AnnotationValue.OfAnnotation
* annotation-valued} array elements of the {@linkplain AnnotationValue.OfArray
* array-valued} element named {@code value} of a container annotation, whose
* interface is the containing annotation interface of A. ({@jls 9.6.3})
// location in class files to source code construct/type section
* If this annotation is of a {@linkplain ##repeatable repeatable} annotation
* interface A, is an array element of an array-valued element named
* {@code value} in a container annotation, and the interface of the container
* annotation is the containing annotation interface AC of A,
* this annotation represents a base annotation of type A, which applies
* to the same source code construct or type as the container annotation of
* type AC.
// On AnnotationElement::name
* @apiNote
* In Java source code, by convention, the name of the sole element in a
* single-element annotation interface is {@code value}. ({@jls 9.6.1})
* A single-element annotation ({@jls 9.7.3}) declares the element value
* for the {@code value} element. The single element of a containing
* annotation interface that holds {@linkplain Annotation##repeatable
* multiple} base annotations is also named {@code value}. ({@jls 9.6.3})
-
PR Comment: https://git.openjdk.org/jdk/pull/20247#issuecomment-2264242499
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v8]
> `TypeAnnotation` is not an annotation, as it should not be used in places > like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an > annotation at a given location instead of to be an annotation. > > Depends on #20205. Chen Liang 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 12 additional commits since the last revision: - remove compile, use element-value, break long sentences - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - Improve docs for repeating, default, and value name - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - More refinements from alex - Artifact -> construct - More about Annotation, add equals note - Further refine wording - Refine the spec of TypeAnnotation per Alex feedback - ... and 2 more: https://git.openjdk.org/jdk/compare/cfd1bb21...3a91a3a5 - Changes: - all: https://git.openjdk.org/jdk/pull/20247/files - new: https://git.openjdk.org/jdk/pull/20247/files/d45c35fc..3a91a3a5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=07 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=06-07 Stats: 531 lines in 22 files changed: 322 ins; 11 del; 198 mod Patch: https://git.openjdk.org/jdk/pull/20247.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20247/head:pull/20247 PR: https://git.openjdk.org/jdk/pull/20247
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v6]
On Wed, 24 Jul 2024 13:00:47 GMT, Chen Liang wrote:
>> `TypeAnnotation` is not an annotation, as it should not be used in places
>> like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an
>> annotation at a given location instead of to be an annotation.
>>
>> Depends on #20205.
>
> Chen Liang has updated the pull request incrementally with three additional
> commits since the last revision:
>
> - More refinements from alex
> - Artifact -> construct
> - More about Annotation, add equals note
I have added a few more paragraphs in response to the request for comments on
default values and repeatable annotations. Below are the excerpts, for
`Annotation` and `AnnotationElement::name`.
* In an annotation in Java source code, elements in the annotation interface
* with default values may not be compiled into element-value pairs unless an
* explicit value is provided. ({@jls 9.6.2}) The default value is derived from
* the {@link AnnotationDefaultAttribute AnnotationDefault} attribute on the
* method representing the annotation interface element in the class file
* representing the annotation interface.
*
* Multiple annotations of the same interface A in Java source code
* ({@jls 9.7.5}) are represented by the {@linkplain
AnnotationValue.OfAnnotation
* annotation-valued} array elements of the {@linkplain AnnotationValue.OfArray
* array-valued} element named {@code value} of a container annotation of the
* containing annotation interface of A. ({@jls 9.6.3})
*
* A single-element annotation ({@jls 9.7.3}) in Java source code is
* compiled to an {@link Annotation} with exactly one {@linkplain
* Annotation#elements key-value pair} where the element has the name
* {@code value}.
*
* Multiple annotations of the same interface A in Java source code
* is compiled to an implicitly declared container annotation ({@jls 9.7.5})
* with exactly one {@linkplain Annotation#elements key-value pair} where
* the element has the name {@code value} and the type of the element is the
* {@linkplain AnnotationValue.OfArray array} whose component type is the
* {@linkplain AnnotationValue.OfAnnotation annotation interface A}.
-
PR Comment: https://git.openjdk.org/jdk/pull/20247#issuecomment-2263874434
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v7]
> `TypeAnnotation` is not an annotation, as it should not be used in places > like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an > annotation at a given location instead of to be an annotation. > > Depends on #20205. Chen Liang 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 10 additional commits since the last revision: - Improve docs for repeating, default, and value name - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - Merge branch 'master' of https://github.com/openjdk/jdk into fix/typeanno-model - More refinements from alex - Artifact -> construct - More about Annotation, add equals note - Further refine wording - Refine the spec of TypeAnnotation per Alex feedback - SealedGraph now redundant - 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation - Changes: - all: https://git.openjdk.org/jdk/pull/20247/files - new: https://git.openjdk.org/jdk/pull/20247/files/cf16e7ee..d45c35fc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=06 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=05-06 Stats: 13943 lines in 436 files changed: 7864 ins; 4437 del; 1642 mod Patch: https://git.openjdk.org/jdk/pull/20247.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20247/head:pull/20247 PR: https://git.openjdk.org/jdk/pull/20247
Re: RFR: 6381729: Javadoc for generic constructor doesn't document type parameter
On Mon, 29 Jul 2024 19:14:40 GMT, Archie Cobbs wrote: >> The standard Javadoc doclet has a long-standing bug (since 2006, old enough >> to vote :) whereby constructor type parameters are simply omitted from the >> output. This results in confusing documentation that look like this: >> >> public MyClass(T obj) - Description of this constructor... >> >> with no explanation of what type `T` is. >> >> The fix itself is straightforward (a one liner), but it requires a bit of >> prerequisite refactoring: >> >> 1. The method `Signatures.appendTypeParameters()` would throw an NPE if >> given a constructor, because the `returnType` is null. This is easy to work >> around with a simple null check. >> >> 2. The code for generating the HTML for the type parameters was embedded in >> a method `AbstractMemberWriter.addModifiersAndType()` which assumed a normal >> method with a return type. In order to make this bit resuable for >> constructors too, it has been extracted out into a new method >> `AbstractExecutableMemberWriter.addTypeParameters()`. >> >> 3. `ConstructorWriter` has an optimization in which constructor modifiers >> are omitted if all constructors are `public`, a common case. This >> optimization is disabled using a field named `foundNonPubConstructor`. A >> side effect of this optimization is that the constructor summary table has >> only two columns instead of three (the first being unnecessary). However, >> type parameters should appear in the same column as modifiers, so this logic >> was generalized to check for both (a) all constructors `public`, and (b) no >> constructors with type parameters. The field `foundNonPubConstructor` has >> been renamed to `showConstructorModifiers`; this clarifies `ClassUseWriter` >> using it for that purpose when generating the use index. >> >> The above steps have been broken out into separate commits. > > Thanks very much for the reviews. On a side note, @archiecobbs you can add your openjdk email acobbs at openjdk to your github account for github to correctly attribute your commits. (this address delegates your jbs email) - PR Comment: https://git.openjdk.org/jdk/pull/20044#issuecomment-2256724979
Re: RFR: 6381729: Javadoc for generic constructor doesn't document type parameter
On Thu, 4 Jul 2024 22:32:19 GMT, Archie Cobbs wrote: > The standard Javadoc doclet has a long-standing bug (since 2006, old enough > to vote :) whereby constructor type parameters are simply omitted from the > output. This results in confusing documentation that look like this: > > public MyClass(T obj) - Description of this constructor... > > with no explanation of what type `T` is. > > The fix itself is straightforward (a one liner), but it requires a bit of > prerequisite refactoring: > > 1. The method `Signatures.appendTypeParameters()` would throw an NPE if given > a constructor, because the `returnType` is null. This is easy to work around > with a simple null check. > > 2. The code for generating the HTML for the type parameters was embedded in a > method `AbstractMemberWriter.addModifiersAndType()` which assumed a normal > method with a return type. In order to make this bit resuable for > constructors too, it has been extracted out into a new method > `AbstractExecutableMemberWriter.addTypeParameters()`. > > 3. `ConstructorWriter` has an optimization in which constructor modifiers are > omitted if all constructors are `public`, a common case. This optimization is > disabled using a field named `foundNonPubConstructor`. A side effect of this > optimization is that the constructor summary table has only two columns > instead of three (the first being unnecessary). However, type parameters > should appear in the same column as modifiers, so this logic was generalized > to check for both (a) all constructors `public`, and (b) no constructors with > type parameters. The field `foundNonPubConstructor` has been renamed to > `showConstructorModifiers`; this clarifies `ClassUseWriter` using it for that > purpose when generating the use index. > > The above steps have been broken out into separate commits. Marked as reviewed by liach (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/20044#pullrequestreview-2205700997
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v6]
On Wed, 24 Jul 2024 19:11:18 GMT, Andrey Turbanov wrote:
>> Chen Liang has updated the pull request incrementally with three additional
>> commits since the last revision:
>>
>> - More refinements from alex
>> - Artifact -> construct
>> - More about Annotation, add equals note
>
> test/langtools/lib/annotations/annotations/classfile/ClassfileInspector.java
> line 1200:
>
>> 1198: case RuntimeVisibleTypeAnnotationsAttribute rvattr -> {
>> 1199: if (expected.matchVisibility(true)) {
>> 1200: for(var anno : rvattr.annotations()) {
>
> Suggestion:
>
> for (var anno : rvattr.annotations()) {
There are 43 `for(` and 2 `for (` in this file, so fixing a single occurrence
isn't too helpful; for consistency I think you can create a dedicated RFE for
running IDE formatters over all jdk/classfile tests. I will review it if you
submit such a patch.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20247#discussion_r1690320937
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v6]
> `TypeAnnotation` is not an annotation, as it should not be used in places > like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an > annotation at a given location instead of to be an annotation. > > Depends on #20205. Chen Liang has updated the pull request incrementally with three additional commits since the last revision: - More refinements from alex - Artifact -> construct - More about Annotation, add equals note - Changes: - all: https://git.openjdk.org/jdk/pull/20247/files - new: https://git.openjdk.org/jdk/pull/20247/files/beb92159..cf16e7ee Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=04-05 Stats: 73 lines in 4 files changed: 52 ins; 0 del; 21 mod Patch: https://git.openjdk.org/jdk/pull/20247.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20247/head:pull/20247 PR: https://git.openjdk.org/jdk/pull/20247
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v5]
> `TypeAnnotation` is not an annotation, as it should not be used in places > like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an > annotation at a given location instead of to be an annotation. > > Depends on #20205. Chen Liang has updated the pull request incrementally with one additional commit since the last revision: Further refine wording - Changes: - all: https://git.openjdk.org/jdk/pull/20247/files - new: https://git.openjdk.org/jdk/pull/20247/files/416890a7..beb92159 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=03-04 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/20247.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20247/head:pull/20247 PR: https://git.openjdk.org/jdk/pull/20247
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v4]
> `TypeAnnotation` is not an annotation, as it should not be used in places > like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an > annotation at a given location instead of to be an annotation. > > Depends on #20205. Chen Liang has updated the pull request incrementally with one additional commit since the last revision: Refine the spec of TypeAnnotation per Alex feedback - Changes: - all: https://git.openjdk.org/jdk/pull/20247/files - new: https://git.openjdk.org/jdk/pull/20247/files/6e31c489..416890a7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=02-03 Stats: 5 lines in 1 file changed: 1 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/20247.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20247/head:pull/20247 PR: https://git.openjdk.org/jdk/pull/20247
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v2]
On Mon, 22 Jul 2024 16:42:06 GMT, Chen Liang wrote: >> `TypeAnnotation` is not an annotation, as it should not be used in places >> like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an >> annotation at a given location instead of to be an annotation. >> >> Depends on #20205. > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. Unfortunately had to force push: the old commit histories now cause conflicts, so I had squashed them into one and applied onto the latest master. - PR Comment: https://git.openjdk.org/jdk/pull/20247#issuecomment-2245592977
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v3]
> `TypeAnnotation` is not an annotation, as it should not be used in places > like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an > annotation at a given location instead of to be an annotation. > > Depends on #20205. Chen Liang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits: - SealedGraph now redundant - 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation - Changes: https://git.openjdk.org/jdk/pull/20247/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=02 Stats: 245 lines in 20 files changed: 81 ins; 115 del; 49 mod Patch: https://git.openjdk.org/jdk/pull/20247.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20247/head:pull/20247 PR: https://git.openjdk.org/jdk/pull/20247
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v2]
> `TypeAnnotation` is not an annotation, as it should not be used in places > like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an > annotation at a given location instead of to be an annotation. > > Depends on #20205. Chen Liang 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 three additional commits since the last revision: - Update to the specification per Alex feedback - Merge branch 'fix/cf-apis' of https://github.com/liachmodded/jdk into fix/typeanno-model - 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation - Changes: - all: https://git.openjdk.org/jdk/pull/20247/files - new: https://git.openjdk.org/jdk/pull/20247/files/752451a7..877b9384 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=00-01 Stats: 605 lines in 40 files changed: 515 ins; 31 del; 59 mod Patch: https://git.openjdk.org/jdk/pull/20247.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20247/head:pull/20247 PR: https://git.openjdk.org/jdk/pull/20247
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation
On Fri, 19 Jul 2024 13:30:44 GMT, Adam Sotona wrote:
>> `TypeAnnotation` is not an annotation, as it should not be used in places
>> like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an
>> annotation at a given location instead of to be an annotation.
>>
>> Depends on #20205.
>
> src/java.base/share/classes/jdk/internal/classfile/impl/AnnotationReader.java
> line 302:
>
>> 300: }
>> 301:
>> 302: public static void writeTypeAnnotation(BufWriterImpl buf,
>> TypeAnnotation ta) {
>
> Is there any reason to move writeTypeAnnotation from UnboundAttribute?
This is for consistency with reading annotations: they are now defined in the
same file.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20247#discussion_r1684409764
RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation
`TypeAnnotation` is not an annotation, as it should not be used in places like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an annotation at a given location instead of to be an annotation. Depends on #20205. - Depends on: https://git.openjdk.org/jdk/pull/20205 Commit messages: - 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation Changes: https://git.openjdk.org/jdk/pull/20247/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20247&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8336754 Stats: 218 lines in 18 files changed: 60 ins; 116 del; 42 mod Patch: https://git.openjdk.org/jdk/pull/20247.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20247/head:pull/20247 PR: https://git.openjdk.org/jdk/pull/20247
[jdk23] Integrated: 8336259: Wrong link to stylesheet.css in JavaDoc API documentation
On Fri, 12 Jul 2024 23:38:31 GMT, Chen Liang wrote: > Please review the backport of #20145 onto jdk23, fixing 2 unnecessary and > erroneous links in the doc files. This pull request has now been integrated. Changeset: 908d1e92 Author: Chen Liang URL: https://git.openjdk.org/jdk/commit/908d1e92fcc4a23a6634ab2c20109df83238110b Stats: 4 lines in 2 files changed: 0 ins; 2 del; 2 mod 8336259: Wrong link to stylesheet.css in JavaDoc API documentation Reviewed-by: iris, prappo Backport-of: 5bc86f332986e3fffc1363f569029bb73a706064 - PR: https://git.openjdk.org/jdk/pull/20166
Re: [jdk23] RFR: 8336259: Wrong link to stylesheet.css in JavaDoc API documentation
On Fri, 12 Jul 2024 23:38:31 GMT, Chen Liang wrote: > Please review the backport of #20145 onto jdk23, fixing 2 unnecessary and > erroneous links in the doc files. Thanks for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/20166#issuecomment-2228806864
[jdk23] Integrated: 8325369: @sealedGraph: Bad link to image for tag on nested classes
On Thu, 11 Jul 2024 16:19:49 GMT, Chen Liang wrote: > Please review this build-only change that fixes the link to the SVG sealed > graph by the `@sealedGraph` javadoc taglet. This affects JDK 23 ea > documentation (as shown in #20122) and thus is backported. This pull request has now been integrated. Changeset: fd386068 Author:Chen Liang URL: https://git.openjdk.org/jdk/commit/fd3860685df3c19b5d2fcc457d397084bc875fad Stats: 11 lines in 1 file changed: 2 ins; 2 del; 7 mod 8325369: @sealedGraph: Bad link to image for tag on nested classes Reviewed-by: prappo Backport-of: d06d79c80980644df511cded0eb8bc0309d878d3 - PR: https://git.openjdk.org/jdk/pull/20141
Re: [jdk23] RFR: 8325369: @sealedGraph: Bad link to image for tag on nested classes
On Thu, 11 Jul 2024 16:19:49 GMT, Chen Liang wrote: > Please review this build-only change that fixes the link to the SVG sealed > graph by the `@sealedGraph` javadoc taglet. This affects JDK 23 ea > documentation (as shown in #20122) and thus is backported. Thanks for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/20141#issuecomment-2228806463
[jdk23] Integrated: 8318106: Generated HTML for snippet does not always contain an id
On Fri, 12 Jul 2024 23:43:28 GMT, Chen Liang wrote: > Please review the backport of #20136, which improves the accessibility of > snippets by generating an id for them if one is not specified. This pull request has now been integrated. Changeset: cfbfe4a4 Author: Chen Liang URL: https://git.openjdk.org/jdk/commit/cfbfe4a472062e1f07817134a05d10ead769b7cb Stats: 65 lines in 7 files changed: 46 ins; 0 del; 19 mod 8318106: Generated HTML for snippet does not always contain an id Reviewed-by: prappo Backport-of: 4166e5345283d118d76b20de579d73bd55436ea6 - PR: https://git.openjdk.org/jdk/pull/20167
[jdk23] Integrated: 8336036: Synthetic documentation for a record's equals is incorrect for floating-point types
On Thu, 11 Jul 2024 21:10:43 GMT, Chen Liang wrote: > Please review this backport of #20129 to JDK 23, fixing the documentation > Javadoc generates for the default object method implementations in user > records about the comparison behavior for `float` and `double` values. This pull request has now been integrated. Changeset: d1373a2f Author:Chen Liang URL: https://git.openjdk.org/jdk/commit/d1373a2fd6186cf6f2a55f6e49a4fce90857cb8e Stats: 13 lines in 2 files changed: 6 ins; 0 del; 7 mod 8336036: Synthetic documentation for a record's equals is incorrect for floating-point types Reviewed-by: prappo Backport-of: b3ef2a600cfec31723dc78fe552e9cf9976b0337 - PR: https://git.openjdk.org/jdk/pull/20147
[jdk23] RFR: 8318106: Generated HTML for snippet does not always contain an id
Please review the backport of #20136, which improves the accessibility of snippets by generating an id for them if one is not specified. - Commit messages: - Backport 4166e5345283d118d76b20de579d73bd55436ea6 Changes: https://git.openjdk.org/jdk/pull/20167/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20167&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8318106 Stats: 65 lines in 7 files changed: 46 ins; 0 del; 19 mod Patch: https://git.openjdk.org/jdk/pull/20167.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20167/head:pull/20167 PR: https://git.openjdk.org/jdk/pull/20167
[jdk23] RFR: 8336259: Wrong link to stylesheet.css in JavaDoc API documentation
Please review the backport of #20145 onto jdk23, fixing 2 unnecessary and erroneous links in the doc files. - Commit messages: - Backport 5bc86f332986e3fffc1363f569029bb73a706064 Changes: https://git.openjdk.org/jdk/pull/20166/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20166&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8336259 Stats: 4 lines in 2 files changed: 0 ins; 2 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/20166.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20166/head:pull/20166 PR: https://git.openjdk.org/jdk/pull/20166
Re: RFR: 8336259: Wrong link to stylesheet.css in JavaDoc API documentation
On Thu, 11 Jul 2024 20:55:29 GMT, Nizar Benalla wrote: > Can I please get a review for this small change, the relative link to the > stylesheet isn't needed as it wasn't used anyway in the generated HTML. The > correct link to the stylesheet is already in the generated HTML. > > This is the difference between the generated docs before and after this > change, by running `diff -r` on the docs before and after the change. > > > GeneratedDocs % diff -r 8336259-stylesheet old-docs > diff -r 8336259-stylesheet/api/java.base/java/lang/doc-files/ValueBased.html > old-docs/api/java.base/java/lang/doc-files/ValueBased.html > 13a14 >> > title="Style"> > diff -r > 8336259-stylesheet/api/java.base/java/lang/doc-files/threadPrimitiveDeprecation.html > old-docs/api/java.base/java/lang/doc-files/threadPrimitiveDeprecation.html > 13a14 >> > title="Style"> > > > Here is the link for the generated docs after the change, no visual change is > seen but the erronous link is gone. > Value based classes > [before](https://docs.oracle.com/en%2Fjava%2Fjavase%2F22%2Fdocs%2Fapi%2F%2F/java.base/java/lang/doc-files/ValueBased.html) > - > [after](https://cr.openjdk.org/~nbenalla/GeneratedDocs/8336259-stylesheet/api/java.base/java/lang/doc-files/ValueBased.html) > > Java Thread Primitive Deprecation > [before](https://docs.oracle.com/en%2Fjava%2Fjavase%2F22%2Fdocs%2Fapi%2F%2F/java.base/java/lang/doc-files/threadPrimitiveDeprecation.html) > - > [after](https://cr.openjdk.org/~nbenalla/GeneratedDocs/8336259-stylesheet/api/java.base/java/lang/doc-files/threadPrimitiveDeprecation.html) Thanks nizar! - PR Comment: https://git.openjdk.org/jdk/pull/20145#issuecomment-2226399579
Re: RFR: 8336259: Wrong link to stylesheet.css in JavaDoc API documentation
On Thu, 11 Jul 2024 20:55:29 GMT, Nizar Benalla wrote: > Can I please get a review for this small change, the relative link to the > stylesheet isn't needed as it wasn't used anyway in the generated HTML. The > correct link to the stylesheet is already in the generated HTML. > > This is the difference between the generated docs before and after this > change, by running `diff -r` on the docs before and after the change. > > > GeneratedDocs % diff -r 8336259-stylesheet old-docs > diff -r 8336259-stylesheet/api/java.base/java/lang/doc-files/ValueBased.html > old-docs/api/java.base/java/lang/doc-files/ValueBased.html > 13a14 >> > title="Style"> > diff -r > 8336259-stylesheet/api/java.base/java/lang/doc-files/threadPrimitiveDeprecation.html > old-docs/api/java.base/java/lang/doc-files/threadPrimitiveDeprecation.html > 13a14 >> > title="Style"> > > > Here is the link for the generated docs after the change, no visual change is > seen but the erronous link is gone. > Value based classes > [before](https://docs.oracle.com/en%2Fjava%2Fjavase%2F22%2Fdocs%2Fapi%2F%2F/java.base/java/lang/doc-files/ValueBased.html) > - > [after](https://cr.openjdk.org/~nbenalla/GeneratedDocs/8336259-stylesheet/api/java.base/java/lang/doc-files/ValueBased.html) > > Java Thread Primitive Deprecation > [before](https://docs.oracle.com/en%2Fjava%2Fjavase%2F22%2Fdocs%2Fapi%2F%2F/java.base/java/lang/doc-files/threadPrimitiveDeprecation.html) > - > [after](https://cr.openjdk.org/~nbenalla/GeneratedDocs/8336259-stylesheet/api/java.base/java/lang/doc-files/threadPrimitiveDeprecation.html) Marked as reviewed by liach (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/20145#pullrequestreview-2173846026
[jdk23] RFR: 8336036: Synthetic documentation for a record's equals is incorrect for floating-point types
Please review this backport of #20129 to JDK 23, fixing the documentation Javadoc generates for the default object method implementations in user records about the comparison behavior for `float` and `double` values. - Commit messages: - Backport b3ef2a600cfec31723dc78fe552e9cf9976b0337 Changes: https://git.openjdk.org/jdk/pull/20147/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20147&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8336036 Stats: 13 lines in 2 files changed: 6 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/20147.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20147/head:pull/20147 PR: https://git.openjdk.org/jdk/pull/20147
Integrated: 8336036: Synthetic documentation for a record's equals is incorrect for floating-point types
On Wed, 10 Jul 2024 23:24:43 GMT, Chen Liang wrote: > Fixes to Javadoc's default documentation on record classes, that all > primitives are compared as if with their wrapper classes' `compare` method by > default. This pull request has now been integrated. Changeset: b3ef2a60 Author:Chen Liang URL: https://git.openjdk.org/jdk/commit/b3ef2a600cfec31723dc78fe552e9cf9976b0337 Stats: 13 lines in 2 files changed: 6 ins; 0 del; 7 mod 8336036: Synthetic documentation for a record's equals is incorrect for floating-point types Reviewed-by: prappo - PR: https://git.openjdk.org/jdk/pull/20129
Re: RFR: 8335623: Clean up HtmlTag.HtmlTag and make the ARIA role attribute global
On Wed, 3 Jul 2024 14:17:14 GMT, Nizar Benalla wrote: > Can I please get a review for this small change? It removes some lines that > are no longer necessary and sets the ARIA role as global. > Thanks in advance. Thank you for your consistent improvement fixes! - PR Comment: https://git.openjdk.org/jdk/pull/20007#issuecomment-2223907851
[jdk23] RFR: 8325369: @sealedGraph: Bad link to image for tag on nested classes
Please review this build-only change that fixes the link to the SVG sealed graph by the `@sealedGraph` javadoc taglet. This affects JDK 23 ea documentation (as shown in #20122) and thus is backported. - Commit messages: - Backport d06d79c80980644df511cded0eb8bc0309d878d3 Changes: https://git.openjdk.org/jdk/pull/20141/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20141&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8325369 Stats: 11 lines in 1 file changed: 2 ins; 2 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/20141.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20141/head:pull/20141 PR: https://git.openjdk.org/jdk/pull/20141
Integrated: 8325369: @sealedGraph: Bad link to image for tag on nested classes
On Wed, 10 Jul 2024 18:57:22 GMT, Chen Liang wrote: > Fixes the link to the generated sealed graph svg generated by the sealed > graph taglet. > > Affected interfaces includes > [`ClassFile.Option`](https://cr.openjdk.org/~nbenalla/GeneratedDocs/8325369-nested-sealed-svg/api/java.base/java/lang/classfile/ClassFile.Option.html) > > ([before](https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/classfile/ClassFile.Option.html)), > > [`AnnotationValue.OfConstant`](https://cr.openjdk.org/~nbenalla/GeneratedDocs/8325369-nested-sealed-svg/api/java.base/java/lang/classfile/AnnotationValue.OfConstant.html) > > ([before](https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/classfile/AnnotationValue.OfConstant.html)), > a few [nested > interfaces](https://cr.openjdk.org/~nbenalla/GeneratedDocs/8325369-nested-sealed-svg/api/java.base/java/lang/classfile/Signature.html#nested-class-summary) > in `Signature` > ([original](https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/classfile/Signature.html#nested-class-summary)), > and [`TypeAnnotaton.TargetInfo`](https://cr.openjdk.org/~nbena lla/GeneratedDocs/8325369-nested-sealed-svg/api/java.base/java/lang/classfile/TypeAnnotation.TargetInfo.html) ([before](https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/classfile/TypeAnnotation.TargetInfo.html)). > > Courtesy to @nizarbenalla for hosting the updated doc rendering. This pull request has now been integrated. Changeset: d06d79c8 Author:Chen Liang URL: https://git.openjdk.org/jdk/commit/d06d79c80980644df511cded0eb8bc0309d878d3 Stats: 11 lines in 1 file changed: 2 ins; 2 del; 7 mod 8325369: @sealedGraph: Bad link to image for tag on nested classes Reviewed-by: jjg - PR: https://git.openjdk.org/jdk/pull/20122
Re: RFR: 8325369: @sealedGraph: Bad link to image for tag on nested classes
On Wed, 10 Jul 2024 21:47:52 GMT, Jonathan Gibbons wrote:
>> Fixes the link to the generated sealed graph svg generated by the sealed
>> graph taglet.
>>
>> Affected interfaces includes
>> [`ClassFile.Option`](https://cr.openjdk.org/~nbenalla/GeneratedDocs/8325369-nested-sealed-svg/api/java.base/java/lang/classfile/ClassFile.Option.html)
>>
>> ([before](https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/classfile/ClassFile.Option.html)),
>>
>> [`AnnotationValue.OfConstant`](https://cr.openjdk.org/~nbenalla/GeneratedDocs/8325369-nested-sealed-svg/api/java.base/java/lang/classfile/AnnotationValue.OfConstant.html)
>>
>> ([before](https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/classfile/AnnotationValue.OfConstant.html)),
>> a few [nested
>> interfaces](https://cr.openjdk.org/~nbenalla/GeneratedDocs/8325369-nested-sealed-svg/api/java.base/java/lang/classfile/Signature.html#nested-class-summary)
>> in `Signature`
>> ([original](https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/classfile/Signature.html#nested-class-summary)),
>> and [`TypeAnnotaton.TargetInfo`](https://cr.openjdk.org/~nben
alla/GeneratedDocs/8325369-nested-sealed-svg/api/java.base/java/lang/classfile/TypeAnnotation.TargetInfo.html)
([before](https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/classfile/TypeAnnotation.TargetInfo.html)).
>>
>> Courtesy to @nizarbenalla for hosting the updated doc rendering.
>
> make/jdk/src/classes/build/tools/taglet/SealedGraph.java line 324:
>
>> 322: while (element.getNestingKind() == NestingKind.MEMBER) {
>> 323: element = (TypeElement) element.getEnclosingElement();
>> 324: result = element.getSimpleName().toString() + '.' + result;
>
> This would be slightly simpler as `element.getSimpleName() + "." + result`
I am inclined to keep to code as-is, as I just moved this static method to the
top-level class without extra changes so the top-level class can access it
without weird qualifications.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20122#discussion_r1674188742
Re: RFR: 8318106: Generated HTML for snippet does not always contain an id [v2]
On Thu, 11 Jul 2024 15:08:34 GMT, Pavel Rappo wrote:
>> Please review this jdk24 bug, which we intend to backport to jdk23 before
>> RDP2, which is a week away. This is a simple fix, but it disturbs multiple
>> tests.
>>
>> Here's a tip for reviewing. Sometimes a snippet id in a test ends up with
>> `()1`, other times with `()2`. This is because the same snippet may appear
>> more than once on the same page, and we are always looking for its last
>> appearance. For example, if a snippet resides in the first sentence of a
>> method documentation, that snippet will appear in both the "Method Summary"
>> and "Method Details" sections. Otherwise, it will only appear in "Method
>> Details". Appearances are numbered starting from 1 and increment by 1 from
>> top ("Method Summary") to bottom ("Method Details").
>
> Pavel Rappo has refreshed the contents of this pull request, and previous
> commits have been removed. The incremental views will show differences
> compared to the previous content of the PR. The pull request contains two new
> commits since the last revision:
>
> - Remove extra hyphen as suggested
> - Initial commit
Marked as reviewed by liach (Committer).
-
PR Review: https://git.openjdk.org/jdk/pull/20136#pullrequestreview-2172186395
Re: RFR: 8318106: Generated HTML for snippet does not always contain an id
On Thu, 11 Jul 2024 14:07:39 GMT, Pavel Rappo wrote:
> Please review this jdk24 bug, which we intend to backport to jdk23 before
> RDP2, which is a week away. This is a simple fix, but it disturbs multiple
> tests.
>
> Here's a tip for reviewing. Sometimes a snippet id in a test ends up with
> `()1`, other times with `()2`. This is because the same snippet may appear
> more than once on the same page, and we are always looking for its last
> appearance. For example, if a snippet resides in the first sentence of a
> method documentation, that snippet will appear in both the "Method Summary"
> and "Method Details" sections. Otherwise, it will only appear in "Method
> Details". Appearances are numbered starting from 1 and increment by 1 from
> top ("Method Summary") to bottom ("Method Details").
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlIds.java
line 619:
> 617: } else {
> 618: // utmost unexpected, but this case shouldn't fail
> 619: id += "unknown-element-";
Feels weird that unknown element has a `-` between name and count but the
others don't.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20136#discussion_r1674138376
RFR: 8336036: Record equals Javadoc is incorrect for floating-point types
Fixes to Javadoc's default documentation on record classes, that all primitives are compared as if with their wrapper classes' `compare` method by default. - Commit messages: - Fix tests - only link tags get escaped - tests - corresponding - 8336036: Record equals Javadoc is incorrect for floating-point types Changes: https://git.openjdk.org/jdk/pull/20129/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20129&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8336036 Stats: 13 lines in 2 files changed: 6 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/20129.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20129/head:pull/20129 PR: https://git.openjdk.org/jdk/pull/20129
RFR: 8325369: @sealedGraph: Bad link to image for tag on nested classes
Fixes the link to the generated sealed graph svg generated by the sealed graph taglet. Affected interfaces includes `ClassFile.Option`, `AnnotationValue.OfConstant`, a few nested interfaces in `Signature`, and `TypeAnnotaton.TargetInfo`. - Commit messages: - 8325369: @sealedGraph: Bad link to image for tag on nested classes Changes: https://git.openjdk.org/jdk/pull/20122/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20122&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8325369 Stats: 11 lines in 1 file changed: 2 ins; 2 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/20122.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20122/head:pull/20122 PR: https://git.openjdk.org/jdk/pull/20122
Re: RFR: 6381729: Javadoc for generic constructor doesn't document type parameter
On Thu, 4 Jul 2024 22:32:19 GMT, Archie Cobbs wrote:
> The standard Javadoc doclet has a long-standing bug (since 2006, old enough
> to vote :) whereby constructor type parameters are simply omitted from the
> output. This results in confusing documentation that look like this:
>
> public MyClass(T obj) - Description of this constructor...
>
> with no explanation of what type `T` is.
>
> The fix itself is straightforward (a one liner), but it requires a bit of
> prerequisite refactoring:
>
> 1. The method `Signatures.appendTypeParameters()` would throw an NPE if given
> a constructor, because the `returnType` is null. This is easy to work around
> with a simple null check.
>
> 2. The code for generating the HTML for the type parameters was embedded in a
> method `AbstractMemberWriter.addModifiersAndType()` which assumed a normal
> method with a return type. In order to make this bit resuable for
> constructors too, it has been extracted out into a new method
> `AbstractExecutableMemberWriter.addTypeParameters()`.
>
> 3. `ConstructorWriter` has an optimization in which constructor modifiers are
> omitted if all constructors are `public`, a common case. This optimization is
> disabled using a field named `foundNonPubConstructor`. A side effect of this
> optimization is that the constructor summary table has only two columns
> instead of three (the first being unnecessary). However, type parameters
> should appear in the same column as modifiers, so this logic was generalized
> to check for both (a) all constructors `public`, and (b) no constructors with
> type parameters. The field `foundNonPubConstructor` has been renamed to
> `showConstructorModifiers`; this clarifies `ClassUseWriter` using it for that
> purpose when generating the use index.
>
> The above steps have been broken out into separate commits.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java
line 1:
> 1: /*
Need a copyright year bump
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Signatures.java
line 1:
> 1: /*
Need a copyright year bump
test/langtools/jdk/javadoc/doclet/testConstructorGenericParam/TestConstructorGenericParam.java
line 36:
> 34: import javadoc.tester.JavadocTester;
> 35:
> 36: public class TestConstructorGenericParam extends JavadocTester {
Can we consolidate this into `testTypeParams` as this is related to type
parameter declarations not being rendered on constructors?
test/langtools/jdk/javadoc/doclet/testConstructorGenericParam/pkg1/A.java line
24:
> 22: */
> 23:
> 24: package pkg1;
Older tests have standalone files for testing. Newer ones can just benefit from
text blocks to avoid copyright headers:
https://github.com/openjdk/jdk/commit/3a00fc732a959300a558d5062e5486220ea75192#diff-755f60e23aa546180d236ea3588409fd87f80159136b8dc03c65160ed244cfd5R203
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20044#discussion_r1666123001
PR Review Comment: https://git.openjdk.org/jdk/pull/20044#discussion_r1666122079
PR Review Comment: https://git.openjdk.org/jdk/pull/20044#discussion_r1666121425
PR Review Comment: https://git.openjdk.org/jdk/pull/20044#discussion_r1666121808
Re: RFR: 8335623: Cleanup HtmlTag.HtmlTag and make the Aria role attribute global
On Wed, 3 Jul 2024 14:17:14 GMT, Nizar Benalla wrote: > Can I please get a review for this small change? It removes some lines that > are no longer necessary and sets the Aria role as global. > Thanks in advance. All these removed keys are already marked global. - Marked as reviewed by liach (Committer). PR Review: https://git.openjdk.org/jdk/pull/20007#pullrequestreview-2156601635
Re: RFR: 8299080: Wrong default value of snippet lang attribute
On Mon, 1 Jul 2024 13:33:28 GMT, Pavel Rappo wrote: > Please review this bugfix to the way the language of a snippet is determined > and processed. > > The language of a snippet affects the form of snippet markup and enables > external syntax highlighting, such as that provided by prism.js. The language > of a snippet is > [[determined](https://docs.oracle.com/en/java/javase/22/docs/specs/javadoc/doc-comment-spec.html#snippet)](https://docs.oracle.com/en/java/javase/22/docs/specs/javadoc/doc-comment-spec.html#snippet) > as follows: > >> A snippet may specify a `lang` attribute, which identifies the kind of >> content in the snippet. For an inline snippet, the default value is `java`. >> For an external snippet, the default value is derived from the extension of >> the name of the file containing the snippet's content. > > There are two issues that this PR fixes. The first issue is a specification > issue. The spec says nothing about the language of a hybrid snippet, which > has features of both an inline and external snippets. It makes sense to > specify that in the absence of the `lang` attribute, the language of a hybrid > snippet is derived from the file extension. Put differently, when determining > the language, a hybrid snippet behaves like an external snippet, not like an > inline snippet. > > The second issue is an implementation issue. If the `lang` attribute or the > file extension is `java` or `properties`, then the form of markup corresponds > to that language and the HTML construct modelling the snippet is attributed > with `class=language-java` or `class=language-properties` respectively. This > is expected. However, if the `lang` attribute or the file extension is > neither of those, or the `lang` attribute is default, then the form of markup > is assumed to be that of `java`, but the HTML construct modelling the snippet > is not attributed, which means that the language is not passed through to the > 3rd party syntax highlighters. > > Stepping out of this PR for a moment, there is clearly a conflation between > the language of a snippet and the form of snippet markup. Those are linked > and controlled by a single knob. That and the design whereby every snippet in > an unsupported language can use markup for the Java language was purposeful: > it was considered simple and practical. > > This PR proposes that the language of a snippet is determined and processed > as follows: > > 1. If the `lang` attribute is present, then its value is the language; if > that value is empty, then the language is undefined > 2. Otherwise, >1. If the snippet is inline, then the language is ... src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/SnippetTaglet.java line 498: > 496: return null; > 497: } > 498: return (lastPeriod == fileName.length() - 1) ? null : > fileName.substring(lastPeriod + 1); Some files, like `.gitignore`, only has suffixes, yet they are valid languages. What do you think? test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetTag.java line 1868: > 1866: """, """ > 1867: # @highlight substring=hi: > 1868: hi there I see that these expected contents are duplicated in many places. Can we pull them out to local variables like `propertiesEvaluated` `javaEvaluated` and `noneEvaluated` for ease of future maintenance and clarity? - PR Review Comment: https://git.openjdk.org/jdk/pull/19971#discussion_r1661109580 PR Review Comment: https://git.openjdk.org/jdk/pull/19971#discussion_r1661106687
Re: RFR: 8322708: Global HTML attributes are not allowed [v6]
On Wed, 19 Jun 2024 09:59:26 GMT, Nizar Benalla wrote: >> Can I please get a review for this change, that aims to add support for >> Global HTML tags. >> Here is the >> [link](https://cr.openjdk.org/~nbenalla/javadocGlobalPR/pkg1/package-summary.html) >> to the generated docs. >> Thanks in advance. > > Nizar Benalla 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 12 additional > commits since the last revision: > > - Merge branch 'master' into html-attributes > - whitespace > - Simplify method > - Null safety > - Merge branch 'master' into html-attributes > - remove trailing whitespace > - -Add a boolean attribute to the enum type >-Simply regex in `visitAttribute` >-Simplified the Test >-Added a negative test > - no longer print summary > - no longer print summary > - Add small comment > - ... and 2 more: https://git.openjdk.org/jdk/compare/2e62e2ca...aa2f2d89 Thanks Nizar! - PR Comment: https://git.openjdk.org/jdk/pull/19652#issuecomment-2180485455
Re: RFR: 8322708: Global HTML attributes are not allowed [v4]
On Fri, 14 Jun 2024 19:00:41 GMT, Nizar Benalla wrote:
>> Can I please get a review for this change, that aims to add support for
>> Global HTML tags.
>> Here is the
>> [link](https://cr.openjdk.org/~nbenalla/javadocGlobalPR/pkg1/package-summary.html)
>> to the generated docs.
>> Thanks in advance.
>
> Nizar Benalla has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Simplify method
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/HtmlTag.java line
670:
> 668: public AttrKind getAttrKind(Name attrName) {
> 669: Attr attr = getAttr(attrName);
> 670: if (attr==null){
Suggestion:
if (attr == null) {
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1640233115
Re: RFR: 8322708: Global HTML attributes are not allowed [v2]
On Tue, 11 Jun 2024 12:45:10 GMT, Pavel Rappo wrote:
>> Nizar Benalla has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove classpath exception
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/HtmlTag.java line
> 556:
>
>> 554: }
>> 555:
>> 556: private final EnumSet GLOBAL_ATTRS = EnumSet.of(
>
> I get it, you cannot make it static because it's used in an enum ctor. But
> attaching it to every single enum constant seems wasteful.
If we are ordering the entries, we can use comparable to check that an attr is
greater than the start of the global attr, something like
private static boolean isGlobalAttr(Attr value) {
return value.compareTo(Attr.ACCESSKEY) >= 0;
}
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1634905873
Re: RFR: 8322708: Global HTML attributes are not allowed
On Tue, 11 Jun 2024 11:37:27 GMT, Nizar Benalla wrote: > Can I please get a review for this change, that aims to add support for > Global HTML tags. > Here is the > [link](https://cr.openjdk.org/~nbenalla/javadocGlobalAttrs/pkg1/package-summary.html) > to the generated docs. > Thanks in advance. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java line 694: > 692: // without checking the validity or applicability of the name > 693: // custom "data-*" attributes are also accepted > 694: var attrName=name.toString(); Suggestion: var attrName = name.toString(); src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/HtmlTag.java line 513: > 511: VSPACE, > 512: WIDTH, > 513: ACCESSKEY, Maybe add a comment `// Global attributes` above this line to indicate the start of global attributes, so future attributes are added to the correct section? - PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1634727172 PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1634727032
Re: RFR: 8331947: Preview creates checkbox for JEP-less preview feature [v4]
On Fri, 31 May 2024 10:01:17 GMT, Hannes Wallnöfer wrote: >> Please review a simple patch to exclude preview visitor classes meant to >> support future preview features from the Preview API page. >> >> The test adds an sample element annotated with the new >> `PreviewFeature.Feature.LANGUAGE_MODEL` constant (which does not have a >> `@JEP` annotation) to make sure it is not listed in the Preview API page. >> The test itself does not have to be modified, as it would fail without the >> change in `PreviewAPIListBuilder.java`. > > Hannes Wallnöfer has updated the pull request incrementally with one > additional commit since the last revision: > > Address review feedback Marked as reviewed by liach (Author). - PR Review: https://git.openjdk.org/jdk/pull/19344#pullrequestreview-2095041186
Re: RFR: 8331947: Preview creates checkbox for JEP-less preview feature [v3]
On Thu, 23 May 2024 13:59:19 GMT, Hannes Wallnöfer wrote:
>> Please review a simple patch to exclude preview visitor classes meant to
>> support future preview features from the Preview API page.
>>
>> The test adds an sample element annotated with the new
>> `PreviewFeature.Feature.LANGUAGE_MODEL` constant (which does not have a
>> `@JEP` annotation) to make sure it is not listed in the Preview API page.
>> The test itself does not have to be modified, as it would fail without the
>> change in `PreviewAPIListBuilder.java`.
>
> Hannes Wallnöfer has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains three commits:
>
> - Merge branch 'master' of https://git.openjdk.org/jdk into JDK-8331947
> - Only include preview features with a JEP
> - JDK-8331947: Preview creates checkbox for JEP-less preview feature
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/PreviewAPIListBuilder.java
line 88:
> 86: return new JEP(number, title, status);
> 87: }
> 88: return null;
I think we can create a `NULL_SENTINEL` special constant JEP object to indicate
a feature has no JEP (like how many collection implementations use a sentinel
for null)
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/SummaryAPIListBuilder.java
line 70:
> 68: * @param configuration the current configuration of the doclet
> 69: */
> 70: public SummaryAPIListBuilder(BaseConfiguration configuration) {
Suggestion:
protected SummaryAPIListBuilder(BaseConfiguration configuration) {
Nit: abstract classes.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19344#discussion_r1620691115
PR Review Comment: https://git.openjdk.org/jdk/pull/19344#discussion_r1620685264
Re: RFR: 8331947: Preview creates checkbox for JEP-less preview feature
On Wed, 22 May 2024 08:39:12 GMT, Hannes Wallnöfer wrote:
> Please review a simple patch to exclude preview visitor classes meant to
> support future preview features from the Preview API page.
>
> The test adds an sample element annotated with the new
> `PreviewFeature.Feature.LANGUAGE_MODEL` constant (which does not have a
> `@JEP` annotation) to make sure it is not listed in the Preview API page. The
> test itself does not have to be modified, as it would fail without the change
> in `PreviewAPIListBuilder.java`.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/PreviewAPIListBuilder.java
line 91:
> 89: if (jep.number == 0) {
> 90: // Remove preview support features without a valid JEP
> 91: jeps.remove(feature);
This feels like we are adding and removing a dummy jep to and from the map
every time we encounter the language model feature. Can we just bind a dummy
jep to that feature to avoid repeated processing, and only filter at the getter
methods?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19344#discussion_r1609781129
Integrated: 8332109: Convert remaining tests using com.sun.tools.classfile to ClassFile API
On Sun, 12 May 2024 08:36:44 GMT, Chen Liang wrote: > Some tests are not migrated to the ClassFile API in previous migrations. > > - Some are simple oversights that didn't remove usages of > com.sun.tools.classfile; > - The CallerSensitive ones used an old utility, replaced by CF API-based new > code; > - many in javac are because the files are compiled with older source > compatibility. Those patches are converted to have the source code stored in > text blocks and compiled within tests using `ToolBox#writeJavaFiles` and > `CompilerUtils.compile`; > - As described in the JBS issue, there are a few other tests not covered; > one is in #19193 while the others are blocked by CreateSymbols migration or > bugs. > > Testing: all modified tests pass. This pull request has now been integrated. Changeset: beeffd46 Author:Chen Liang Committer: Adam Sotona URL: https://git.openjdk.org/jdk/commit/beeffd4671649e5d8f9c96f0455ac90a82917234 Stats: 1539 lines in 19 files changed: 224 ins; 1098 del; 217 mod 8332109: Convert remaining tests using com.sun.tools.classfile to ClassFile API Reviewed-by: asotona - PR: https://git.openjdk.org/jdk/pull/19206
Re: RFR: 8332109: Convert remaining tests using com.sun.tools.classfile to ClassFile API
On Sun, 12 May 2024 08:36:44 GMT, Chen Liang wrote: > Some tests are not migrated to the ClassFile API in previous migrations. > > - Some are simple oversights that didn't remove usages of > com.sun.tools.classfile; > - The CallerSensitive ones used an old utility, replaced by CF API-based new > code; > - many in javac are because the files are compiled with older source > compatibility. Those patches are converted to have the source code stored in > text blocks and compiled within tests using `ToolBox#writeJavaFiles` and > `CompilerUtils.compile`; > - As described in the JBS issue, there are a few other tests not covered; > one is in #19193 while the others are blocked by CreateSymbols migration or > bugs. > > Testing: all modified tests pass. Thanks for the review! - PR Comment: https://git.openjdk.org/jdk/pull/19206#issuecomment-2117380960
Re: RFR: 8332109: Convert remaining tests using com.sun.tools.classfile to ClassFile API
On Fri, 17 May 2024 06:03:00 GMT, Adam Sotona wrote:
>> Some tests are not migrated to the ClassFile API in previous migrations.
>>
>> - Some are simple oversights that didn't remove usages of
>> com.sun.tools.classfile;
>> - The CallerSensitive ones used an old utility, replaced by CF API-based
>> new code;
>> - many in javac are because the files are compiled with older source
>> compatibility. Those patches are converted to have the source code stored in
>> text blocks and compiled within tests using `ToolBox#writeJavaFiles` and
>> `CompilerUtils.compile`;
>> - As described in the JBS issue, there are a few other tests not covered;
>> one is in #19193 while the others are blocked by CreateSymbols migration or
>> bugs.
>>
>> Testing: all modified tests pass.
>
> test/jdk/jdk/internal/reflect/CallerSensitive/CheckCSMs.java line 151:
>
>> 149:
>> 150: boolean needsCsm = false;
>> 151: for (var element : code) {
>
> Scanning the instructions is a bit different approach than in the original
> test.
Same as the other method name check, it was manually extracted ReferenceFinder
logic, we should retire ReferenceFinder with the old class file library.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19206#discussion_r1604405594
Re: RFR: 8332109: Convert remaining tests using com.sun.tools.classfile to ClassFile API
On Fri, 17 May 2024 06:07:47 GMT, Adam Sotona wrote:
>> Some tests are not migrated to the ClassFile API in previous migrations.
>>
>> - Some are simple oversights that didn't remove usages of
>> com.sun.tools.classfile;
>> - The CallerSensitive ones used an old utility, replaced by CF API-based
>> new code;
>> - many in javac are because the files are compiled with older source
>> compatibility. Those patches are converted to have the source code stored in
>> text blocks and compiled within tests using `ToolBox#writeJavaFiles` and
>> `CompilerUtils.compile`;
>> - As described in the JBS issue, there are a few other tests not covered;
>> one is in #19193 while the others are blocked by CreateSymbols migration or
>> bugs.
>>
>> Testing: all modified tests pass.
>
> test/jdk/jdk/internal/reflect/CallerSensitive/CallerSensitiveFinder.java line
> 125:
>
>> 123: && invoke.method() instanceof MethodRefEntry ref
>> 124: && ref.owner().name().equalsString(className)
>> 125: && ref.name().equalsString(methodName)) {
>
> Is this additional test necessary, I don't see it in the original test.
This was part of the now-removed ReferenceFinder logic; it scans cp as a fast
path and then does instruction scan for all methods to find relevant methods to
pass to visitor.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19206#discussion_r1604402631
Re: RFR: 8298405: Implement JEP 467: Markdown Documentation Comments [v69]
On Wed, 15 May 2024 21:04:36 GMT, Jonathan Gibbons wrote: >> Please review a patch to add support for Markdown syntax in documentation >> comments, as described in the associated JEP. >> >> Notable features: >> >> * support for `///` documentation comments in `JavaTokenizer` >> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` >> library >> * updates to `DocCommentParser` to treat `///` comments as Markdown >> * updates to the standard doclet to render Markdown comments in HTML > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > update tests for dangling doc comments, per review feedback Thanks for these explanations! Makes sense, also great that we can have Javadoc features delivered easily because its loose coupling with Java language. - PR Comment: https://git.openjdk.org/jdk/pull/16388#issuecomment-2115515659
Re: RFR: 8298405: Implement JEP 467: Markdown Documentation Comments [v69]
On Wed, 15 May 2024 21:04:36 GMT, Jonathan Gibbons wrote: >> Please review a patch to add support for Markdown syntax in documentation >> comments, as described in the associated JEP. >> >> Notable features: >> >> * support for `///` documentation comments in `JavaTokenizer` >> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` >> library >> * updates to `DocCommentParser` to treat `///` comments as Markdown >> * updates to the standard doclet to render Markdown comments in HTML > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > update tests for dangling doc comments, per review feedback My rationale for a potential preview is that we changed `-Xlint:dangling-doc-comments` as `///` is now dangling doc comment. Is this considered a Java programming language change? There were some community comments objecting the use of `///` for markdown documentation, and called for alternative syntaxes like `/*markdown */`. - PR Comment: https://git.openjdk.org/jdk/pull/16388#issuecomment-2115466695
Re: RFR: 8298405: Implement JEP 467: Markdown Documentation Comments [v69]
On Wed, 15 May 2024 21:04:36 GMT, Jonathan Gibbons wrote: >> Please review a patch to add support for Markdown syntax in documentation >> comments, as described in the associated JEP. >> >> Notable features: >> >> * support for `///` documentation comments in `JavaTokenizer` >> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` >> library >> * updates to `DocCommentParser` to treat `///` comments as Markdown >> * updates to the standard doclet to render Markdown comments in HTML > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > update tests for dangling doc comments, per review feedback A meta question about the JEP: Why don't Javadoc features (like snippets and markdown comments) don't go through preview, while Compiler features mostly do? Is there any bar for previews? - PR Comment: https://git.openjdk.org/jdk/pull/16388#issuecomment-2115199037
Re: RFR: 8332239: Improve CSS for block tags
On Thu, 16 May 2024 10:56:26 GMT, Hannes Wallnöfer wrote: > Please review a change to improve the layout of definition lists used to > display block tags: > > - Add indentation to the `` elements used for block tag details > - Set the margin of lists within block tag details so they do not appear as > nested lists, except for lists with CSS classes `tag-list` or > `tag-list-long`, which are used for block tags containing lists, such as > `@see`. > > Before/after comparison (contains `java.base` and `java.compiler` modules): > https://cr.openjdk.org/~hannesw/8332239/api.00/index.html > https://cr.openjdk.org/~hannesw/8332239/api.01/index.html > > Comparison for block tag layout: > https://cr.openjdk.org/~hannesw/8332239/api.00/java.base/java/lang/Object.html#hashCode() > https://cr.openjdk.org/~hannesw/8332239/api.01/java.base/java/lang/Object.html#hashCode() > > Example of very long block tag details containing a list: > https://cr.openjdk.org/~hannesw/8332239/api.00/java.compiler/javax/lang/model/util/package-summary.html > https://cr.openjdk.org/~hannesw/8332239/api.01/java.compiler/javax/lang/model/util/package-summary.html > > Note that the indentation also applies to the definition lists at the top of > class/interface documentation as they use the same markup as block tags: > https://cr.openjdk.org/~hannesw/8332239/api.00/java.base/java/lang/Exception.html > https://cr.openjdk.org/~hannesw/8332239/api.01/java.base/java/lang/Exception.html > > This was not an intended change but might or might not be desirable. It > could be avoided with some additional CSS changes. Marked as reviewed by liach (Author). - PR Review: https://git.openjdk.org/jdk/pull/19264#pullrequestreview-2060496263
RFR: 8332109: Convert remaining tests using com.sun.tools.classfile to ClassFile API
Some tests are not migrated to the ClassFile API in previous migrations. - Some are simple oversights that didn't remove usages of com.sun.tools.classfile; - The CallerSensitive ones used an old utility, replaced by CF API-based new code; - many in javac are because the files are compiled with older source compatibility. Those patches are converted to have the source code stored in text blocks and compiled within tests using `ToolBox#writeJavaFiles` and `CompilerUtils.compile`; - As described in the JBS issue, there are a few other tests not covered; one is in #19193 while the others are blocked by CreateSymbols migration or bugs. Testing: all modified tests pass. - Commit messages: - Missing copyright years - 8332109: Convert remaining tests using com.sun.tools.classfile to ClassFile API Changes: https://git.openjdk.org/jdk/pull/19206/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19206&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8332109 Stats: 1539 lines in 19 files changed: 224 ins; 1098 del; 217 mod Patch: https://git.openjdk.org/jdk/pull/19206.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19206/head:pull/19206 PR: https://git.openjdk.org/jdk/pull/19206
Re: RFR: 8325088: Overloads that differ in type parameters may be lost [v3]
On Thu, 4 Apr 2024 14:53:09 GMT, Pavel Rappo wrote: >> That's actually a sign of relief that the same elements will have the same >> id with the same input source code > > Reading your reply, I think that one of us is likely confused. Your question > to which I replied "no" was this: > >> Will the Number public method be consistently erased whether or not -private >> is set? > > I replied "no", which means that the id that the public method has DO depend > on whether `-private` is specified. Let me expand on that. > > If `-private` is specified, the "Method Details" section will have this for > the public method: > > > method > > and this for the private method: > > > method > > Otherwise, the "Method Details" section will only have this for the public > method: > > > method > > As you can see, depending on whether `-private` is specified, the section id > for the public method will be different. > > Is that how you understood my reply and felt relief after that? It's fine, we always have an anchor to the erased version. - PR Review Comment: https://git.openjdk.org/jdk/pull/18519#discussion_r1551955976
Re: RFR: 8325088: Overloads that differ in type parameters may be lost [v3]
On Thu, 4 Apr 2024 12:28:13 GMT, Pavel Rappo wrote:
>> 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?
Looks great, thanks!
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18519#discussion_r1551777805
Re: RFR: 8325088: Overloads that differ in type parameters may be lost [v3]
On Thu, 4 Apr 2024 11:41:37 GMT, Pavel Rappo wrote:
>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlIds.java
>> line 562:
>>
>>> 560: && e.getKind() != ElementKind.METHOD)
>>> 561: throw new
>>> IllegalArgumentException(String.valueOf(e.getKind()));
>>> 562: var vmt = configuration.getVisibleMemberTable((TypeElement)
>>> e.getEnclosingElement());
>>
>> Will the vmt differ if we specify `-private`? And say if we have 2 methods
>> like:
>>
>> private void method(T arg) {}
>> public void method(T arg) {}
>>
>> Will the Number public method be consistently erased whether or not
>> `-private` is set?
>
> The answer to your latter question is no. The id that will be assigned to
> that public method do depend on whether the private overload positioned
> before it is documented. You cannot do much about it with the current design
> of `VisibleMemberTable` without unduly complicating this PR. On the bright
> side, I reckon such a case is extremely rare.
That's actually a sign of relief that the same elements will have the same id
with the same input source code
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18519#discussion_r1551738452
Re: RFR: 8325088: Overloads that differ in type parameters may be lost [v3]
On Wed, 3 Apr 2024 18:14:39 GMT, Pavel Rappo wrote:
>> Creating a link to a constructor or a method or comparing constructors or
>> methods __does not__ factor in type parameters. When constructors or methods
>> are overloaded and differ only in type parameters -- a situation which is
>> absent in JDK API, but present elsewhere -- that causes significant defects,
>> such as:
>>
>> - missing entries in summary tables, lists and indexes,
>> - duplicating links in the table of contents.
>>
>> This PR fixes those defects, and the fix is two-fold. Firstly, we update
>> comparators to consider type parameters. That takes care of missing
>> constructors and methods. Secondly, we update id (anchor) and link
>> generation to always use the "erased" notation. That takes care of
>> duplicating links.
>>
>> What's the "erased" notation? Suppose we have the following method:
>>
>> T m(T arg)
>>
>> The current notation refers to it as `m(T)`. That works fine until there's
>> no other method, such as
>>
>> T m(T arg)
>>
>> In which case, the current notation will produce a collision: `m(T)`. By
>> contrast, the erased notation for those two methods is `m(java.lang.String)`
>> and `m(java.lang.Object)` respectively. No collision.
>>
>> While longer, I believe that the erased notation is collision-proof. Why?
>> Because [JLS 8.4.2][] says that "it is a compile-time error to declare two
>> methods with override-equivalent signatures in a class". Which means that
>> for any two constructors or methods the erasure of their signatures must
>> differ, or else it won't compile.
>>
>> The change is pretty straightforward, except for some test fallout that
>> required attention.
>>
>> [JLS 8.4.2]:
>> https://docs.oracle.com/javase/specs/jls/se22/html/jls-8.html#jls-8.4.2
>
> 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 4f0282694fd
> + update test/langtools/jdk/javadoc/doclet/testOrdering/TestOrdering
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ConstructorWriter.java
line 195:
> 193: HtmlId erasureAnchor = htmlIds.forErasure(constructor);
> 194: if (erasureAnchor != null
> 195: && !erasureAnchor.name().equals(memberAnchor.name())) {
Is this redundant now that the erasure handling is encapsulated within
forMember?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlIds.java
line 562:
> 560: && e.getKi
Re: RFR: 8325088: Overloads that differ in type parameters may be lost
On Wed, 27 Mar 2024 17:19:35 GMT, Pavel Rappo wrote: > Creating a link to a constructor or a method or comparing constructors or > methods __does not__ factor in type parameters. When constructors or methods > are overloaded and differ only in type parameters -- a situation which is > absent in JDK API, but present elsewhere -- that causes significant defects, > such as: > > - missing entries in summary tables, lists and indexes, > - duplicating links in the table of contents. > > This PR fixes those defects, and the fix is two-fold. Firstly, we update > comparators to consider type parameters. That takes care of missing > constructors and methods. Secondly, we update id (anchor) and link generation > to always use the "erased" notation. That takes care of duplicating links. > > What's the "erased" notation? Suppose we have the following method: > > T m(T arg) > > The current notation refers to it as `m(T)`. That works fine until there's no > other method, such as > > T m(T arg) > > In which case, the current notation will produce a collision: `m(T)`. By > contrast, the erased notation for those two methods is `m(java.lang.String)` > and `m(java.lang.Object)` respectively. No collision. > > While longer, I believe that the erased notation is collision-proof. Why? > Because [JLS 8.4.2][] says that "it is a compile-time error to declare two > methods with override-equivalent signatures in a class". Which means that for > any two constructors or methods the erasure of their signatures must differ, > or else it won't compile. > > The change is pretty straightforward, except for some test fallout that > required attention. > > [JLS 8.4.2]: > https://docs.oracle.com/javase/specs/jls/se22/html/jls-8.html#jls-8.4.2 A compatibility problem is that if library C wants to link to library A on Java 17 with generic anchors and library B on 23 with erased anchors. Is there a way for Javadoc to generate the correct link to both older versions, such as by detecting which anchor format was used by scanning index.html? A similar problem occured before when Javadoc changed `--` around method parameters to `()`. - PR Comment: https://git.openjdk.org/jdk/pull/18519#issuecomment-2024059820
Integrated: 8327824: Type annotation placed on incorrect array nesting levels
On Tue, 12 Mar 2024 03:33:38 GMT, Chen Liang wrote: > Please review this patch that fixes placement of type annotations on array > types in Javadoc output. This oversight seems to have existed since JDK 8 but > was never noticed or reported. This pull request has now been integrated. Changeset: 35b00e64 Author:Chen Liang Committer: Pavel Rappo URL: https://git.openjdk.org/jdk/commit/35b00e640b7b3f29796433aee86838ae46167419 Stats: 128 lines in 3 files changed: 103 ins; 1 del; 24 mod 8327824: Type annotation placed on incorrect array nesting levels Reviewed-by: prappo - PR: https://git.openjdk.org/jdk/pull/18211
Re: RFR: 8327824: Type annotation placed on incorrect array nesting levels
On Tue, 12 Mar 2024 03:33:38 GMT, Chen Liang wrote: > Please review this patch that fixes placement of type annotations on array > types in Javadoc output. This oversight seems to have existed since JDK 8 but > was never noticed or reported. Now fixed merge conflict and added a test case for JLS 10.2-2, with the 2 styles of declarations present in the parameter and the return type respectively. - PR Comment: https://git.openjdk.org/jdk/pull/18211#issuecomment-1992660784
Re: RFR: 8327824: Type annotation placed on incorrect array nesting levels [v2]
> Please review this patch that fixes placement of type annotations on array > types in Javadoc output. This oversight seems to have existed since JDK 8 but > was never noticed or reported. Chen Liang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits: - Check output from body instead of summary, summary changes css class when new members are added - Add test case for JLS example 10.2-2 - Merge branch 'master' of https://github.com/openjdk/jdk into fix/jd-array-anno-order - Fix whitespace - Make sure varargs work too - Fix bugs, add tests, fix wrong anticipated test results - 8327824: Type annotation placed on incorrect array nesting levels - Changes: https://git.openjdk.org/jdk/pull/18211/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18211&range=01 Stats: 128 lines in 3 files changed: 103 ins; 1 del; 24 mod Patch: https://git.openjdk.org/jdk/pull/18211.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18211/head:pull/18211 PR: https://git.openjdk.org/jdk/pull/18211
Integrated: 8325433: Type annotations on primitive types are not linked
On Sun, 10 Mar 2024 20:19:44 GMT, Chen Liang wrote:
> Please review this patch that fixes the issue where type annotations on
> primitive types are not linked.
>
> Tested with file
> https://cr.openjdk.org/~liach/8325433-arrayanno/ArrayAnno.java
>
> import java.lang.annotation.*;
>
> public class ArrayAnno {
> @Retention(RetentionPolicy.RUNTIME)
> @Target(ElementType.TYPE_USE)
> @Documented
> public @interface Anno { int value(); }
>
> public void method(@Anno(1) int @Anno(2) [] @Anno(3) [] arg) {}
> public void method(@Anno(1) String @Anno(2) [] @Anno(3) [] arg) {}
> }
>
> JDK 21: https://cr.openjdk.org/~liach/8325433-arrayanno/old/ArrayAnno.html
> This patch: https://cr.openjdk.org/~liach/8325433-arrayanno/new/ArrayAnno.html
>
> Note that a bug within javac (reported in
> https://bugs.openjdk.org/browse/JDK-8327824) causes the annotations to become
> `@Anno(1) String @Anno(3) [] @Anno(2) []` in the output files; this bug also
> affects output class files so I assume this is a bug within javac's tree
> building. (Intersting, the buggy javadoc output was copied wholesale in the
> original `TestTypeAnnotations` output for `array2Deep` cases, but no one paid
> attention to it)
This pull request has now been integrated.
Changeset: 2cf35246
Author:Chen Liang
Committer: Pavel Rappo
URL:
https://git.openjdk.org/jdk/commit/2cf35246e6041e3c721efc24441545deb7a671a8
Stats: 53 lines in 5 files changed: 44 ins; 0 del; 9 mod
8325433: Type annotations on primitive types are not linked
Reviewed-by: gli, prappo
-
PR: https://git.openjdk.org/jdk/pull/18179
Re: RFR: 8325433: Type annotations on primitive types are not linked [v3]
On Mon, 11 Mar 2024 13:57:23 GMT, Chen Liang wrote:
>> Please review this patch that fixes the issue where type annotations on
>> primitive types are not linked.
>>
>> Tested with file
>> https://cr.openjdk.org/~liach/8325433-arrayanno/ArrayAnno.java
>>
>> import java.lang.annotation.*;
>>
>> public class ArrayAnno {
>> @Retention(RetentionPolicy.RUNTIME)
>> @Target(ElementType.TYPE_USE)
>> @Documented
>> public @interface Anno { int value(); }
>>
>> public void method(@Anno(1) int @Anno(2) [] @Anno(3) [] arg) {}
>> public void method(@Anno(1) String @Anno(2) [] @Anno(3) [] arg) {}
>> }
>>
>> JDK 21: https://cr.openjdk.org/~liach/8325433-arrayanno/old/ArrayAnno.html
>> This patch:
>> https://cr.openjdk.org/~liach/8325433-arrayanno/new/ArrayAnno.html
>>
>> Note that a bug within javac (reported in
>> https://bugs.openjdk.org/browse/JDK-8327824) causes the annotations to
>> become `@Anno(1) String @Anno(3) [] @Anno(2) []` in the output files; this
>> bug also affects output class files so I assume this is a bug within javac's
>> tree building. (Intersting, the buggy javadoc output was copied wholesale in
>> the original `TestTypeAnnotations` output for `array2Deep` cases, but no one
>> paid attention to it)
>
> Chen Liang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Redundant imports
Assuming this passes tests, at least it passes tier1 on GitHub.
-
PR Comment: https://git.openjdk.org/jdk/pull/18179#issuecomment-1991502789
RFR: 8327824: Type annotation placed on incorrect array nesting levels
Please review this patch that fixes placement of type annotations on array types in Javadoc output. This oversight seems to have existed since JDK 8 but was never noticed or reported. - Commit messages: - Fix whitespace - Make sure varargs work too - Fix bugs, add tests, fix wrong anticipated test results - 8327824: Type annotation placed on incorrect array nesting levels Changes: https://git.openjdk.org/jdk/pull/18211/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18211&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8327824 Stats: 128 lines in 3 files changed: 102 ins; 1 del; 25 mod Patch: https://git.openjdk.org/jdk/pull/18211.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18211/head:pull/18211 PR: https://git.openjdk.org/jdk/pull/18211
Re: RFR: 8325433: Type annotations on primitive types are not linked [v2]
On Mon, 11 Mar 2024 13:31:06 GMT, Chen Liang wrote:
>> Please review this patch that fixes the issue where type annotations on
>> primitive types are not linked.
>>
>> Tested with file
>> https://cr.openjdk.org/~liach/8325433-arrayanno/ArrayAnno.java
>>
>> import java.lang.annotation.*;
>>
>> public class ArrayAnno {
>> @Retention(RetentionPolicy.RUNTIME)
>> @Target(ElementType.TYPE_USE)
>> @Documented
>> public @interface Anno { int value(); }
>>
>> public void method(@Anno(1) int @Anno(2) [] @Anno(3) [] arg) {}
>> public void method(@Anno(1) String @Anno(2) [] @Anno(3) [] arg) {}
>> }
>>
>> JDK 21: https://cr.openjdk.org/~liach/8325433-arrayanno/old/ArrayAnno.html
>> This patch:
>> https://cr.openjdk.org/~liach/8325433-arrayanno/new/ArrayAnno.html
>>
>> Note that a bug within javac (reported in
>> https://bugs.openjdk.org/browse/JDK-8327824) causes the annotations to
>> become `@Anno(1) String @Anno(3) [] @Anno(2) []` in the output files; this
>> bug also affects output class files so I assume this is a bug within javac's
>> tree building. (Intersting, the buggy javadoc output was copied wholesale in
>> the original `TestTypeAnnotations` output for `array2Deep` cases, but no one
>> paid attention to it)
>
> Chen Liang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Removed unused methods
Fixed imports, and the javac bug with path assignment for type annotation on
array is reported as https://bugs.openjdk.org/browse/JDK-8327824.
-
PR Comment: https://git.openjdk.org/jdk/pull/18179#issuecomment-1988494667
Re: RFR: 8325433: Type annotations on primitive types are not linked [v2]
On Mon, 11 Mar 2024 13:45:19 GMT, Guoxiong Li wrote: >> Chen Liang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Removed unused methods > > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlLinkFactory.java > line 31: > >> 29: import java.util.EnumSet; >> 30: import java.util.List; >> 31: import java.util.Locale; > > I can't find the code which uses `Locale`. Is it redundant? Was trying to add a visitor override for PrimitiveType and doing `primitiveType.getKind().toString().toLowerCase(Locale.ROOT)` but discovered `utils.getTypeSignature` was already doing the same; forgot to remove imports. - PR Review Comment: https://git.openjdk.org/jdk/pull/18179#discussion_r1519754693
Re: RFR: 8325433: Type annotations on primitive types are not linked [v3]
> Please review this patch that fixes the issue where type annotations on
> primitive types are not linked.
>
> Tested with file
> https://cr.openjdk.org/~liach/8325433-arrayanno/ArrayAnno.java
>
> import java.lang.annotation.*;
>
> public class ArrayAnno {
> @Retention(RetentionPolicy.RUNTIME)
> @Target(ElementType.TYPE_USE)
> @Documented
> public @interface Anno { int value(); }
>
> public void method(@Anno(1) int @Anno(2) [] @Anno(3) [] arg) {}
> public void method(@Anno(1) String @Anno(2) [] @Anno(3) [] arg) {}
> }
>
> JDK 21: https://cr.openjdk.org/~liach/8325433-arrayanno/old/ArrayAnno.html
> This patch: https://cr.openjdk.org/~liach/8325433-arrayanno/new/ArrayAnno.html
>
> Note that a bug within javac (reported in
> https://bugs.openjdk.org/browse/JDK-8327824) causes the annotations to become
> `@Anno(1) String @Anno(3) [] @Anno(2) []` in the output files; this bug also
> affects output class files so I assume this is a bug within javac's tree
> building. (Intersting, the buggy javadoc output was copied wholesale in the
> original `TestTypeAnnotations` output for `array2Deep` cases, but no one paid
> attention to it)
Chen Liang has updated the pull request incrementally with one additional
commit since the last revision:
Redundant imports
-
Changes:
- all: https://git.openjdk.org/jdk/pull/18179/files
- new: https://git.openjdk.org/jdk/pull/18179/files/c7de229e..d4fc545a
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=18179&range=02
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=18179&range=01-02
Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/18179.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18179/head:pull/18179
PR: https://git.openjdk.org/jdk/pull/18179
Re: RFR: 8325433: Type annotations on primitive types are not linked [v2]
On Mon, 11 Mar 2024 06:36:49 GMT, Guoxiong Li wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removed unused methods
>
> test/langtools/jdk/javadoc/doclet/testTypeAnnotations/typeannos/MethodReturnType.java
> line 54:
>
>> 52:
>> 53: @MRtnA int primitive() { return 0; }
>> 54: @MRtnA int @MRtnB [] primitiveArray1Deep() { return null; }
>
> These two fields have not been verified?
I'm pretty sure the default scoped one is verified. I've removed the unused
modified scoped ones.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18179#discussion_r1519736994
Re: RFR: 8325433: Type annotations on primitive types are not linked [v2]
> Please review this patch that fixes the issue where type annotations on
> primitive types are not linked.
>
> Tested with file
> https://cr.openjdk.org/~liach/8325433-arrayanno/ArrayAnno.java
>
> import java.lang.annotation.*;
>
> public class ArrayAnno {
> @Retention(RetentionPolicy.RUNTIME)
> @Target(ElementType.TYPE_USE)
> @Documented
> public @interface Anno { int value(); }
>
> public void method(@Anno(1) int @Anno(2) [] @Anno(3) [] arg) {}
> public void method(@Anno(1) String @Anno(2) [] @Anno(3) [] arg) {}
> }
>
> JDK 21: https://cr.openjdk.org/~liach/8325433-arrayanno/old/ArrayAnno.html
> This patch: https://cr.openjdk.org/~liach/8325433-arrayanno/new/ArrayAnno.html
>
> Note that a bug within javac causes the annotations to become `@Anno(1)
> String @Anno(3) [] @Anno(2) []` in the output files; this bug also affects
> output class files so I assume this is a bug within javac's tree building.
> (Intersting, the buggy javadoc output was copied wholesale in the original
> `TestTypeAnnotations` output for `array2Deep` cases, but no one paid
> attention to it)
Chen Liang has updated the pull request incrementally with one additional
commit since the last revision:
Removed unused methods
-
Changes:
- all: https://git.openjdk.org/jdk/pull/18179/files
- new: https://git.openjdk.org/jdk/pull/18179/files/e33b685f..c7de229e
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=18179&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=18179&range=00-01
Stats: 6 lines in 2 files changed: 0 ins; 6 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/18179.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18179/head:pull/18179
PR: https://git.openjdk.org/jdk/pull/18179
Re: RFR: 8325433: Type annotations on primitive types are not linked
On Mon, 11 Mar 2024 06:35:27 GMT, Guoxiong Li wrote:
>> Please review this patch that fixes the issue where type annotations on
>> primitive types are not linked.
>>
>> Tested with file
>> https://cr.openjdk.org/~liach/8325433-arrayanno/ArrayAnno.java
>>
>> import java.lang.annotation.*;
>>
>> public class ArrayAnno {
>> @Retention(RetentionPolicy.RUNTIME)
>> @Target(ElementType.TYPE_USE)
>> @Documented
>> public @interface Anno { int value(); }
>>
>> public void method(@Anno(1) int @Anno(2) [] @Anno(3) [] arg) {}
>> public void method(@Anno(1) String @Anno(2) [] @Anno(3) [] arg) {}
>> }
>>
>> JDK 21: https://cr.openjdk.org/~liach/8325433-arrayanno/old/ArrayAnno.html
>> This patch:
>> https://cr.openjdk.org/~liach/8325433-arrayanno/new/ArrayAnno.html
>>
>> Note that a bug within javac causes the annotations to become `@Anno(1)
>> String @Anno(3) [] @Anno(2) []` in the output files; this bug also affects
>> output class files so I assume this is a bug within javac's tree building.
>> (Intersting, the buggy javadoc output was copied wholesale in the original
>> `TestTypeAnnotations` output for `array2Deep` cases, but no one paid
>> attention to it)
>
> test/langtools/jdk/javadoc/doclet/testTypeAnnotations/typeannos/Fields.java
> line 72:
>
>> 70:
>> 71: public final @FldA int primitive = 0;
>> 72: public final @FldA int @FldB [] primitiveArray1Deep = null;
>
> These two fields have not been verified?
I shall remove these fields; they serve no real purpose here indeed.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18179#discussion_r1519693430
Re: RFR: 8325433: Type annotations on primitive types are not linked
On Sun, 10 Mar 2024 20:19:44 GMT, Chen Liang wrote:
> Please review this patch that fixes the issue where type annotations on
> primitive types are not linked.
>
> Tested with file
> https://cr.openjdk.org/~liach/8325433-arrayanno/ArrayAnno.java
>
> import java.lang.annotation.*;
>
> public class ArrayAnno {
> @Retention(RetentionPolicy.RUNTIME)
> @Target(ElementType.TYPE_USE)
> @Documented
> public @interface Anno { int value(); }
>
> public void method(@Anno(1) int @Anno(2) [] @Anno(3) [] arg) {}
> public void method(@Anno(1) String @Anno(2) [] @Anno(3) [] arg) {}
> }
>
> JDK 21: https://cr.openjdk.org/~liach/8325433-arrayanno/old/ArrayAnno.html
> This patch: https://cr.openjdk.org/~liach/8325433-arrayanno/new/ArrayAnno.html
>
> Note that a bug within javac causes the annotations to become `@Anno(1)
> String @Anno(3) [] @Anno(2) []` in the output files; this bug also affects
> output class files so I assume this is a bug within javac's tree building.
> (Intersting, the buggy javadoc output was copied wholesale in the original
> `TestTypeAnnotations` output for `array2Deep` cases, but no one paid
> attention to it)
The 2-D array case is broken: currently source code `@A int @B [] @C []` is
compiled to `@A int @C [] @B []` in both class file and javadoc output, so I
don't want to add a buggy case that needs to be updated if this annotation
placement bug is fixed.
-
PR Comment: https://git.openjdk.org/jdk/pull/18179#issuecomment-1988412676
RFR: 8325433: Type annotations on primitive types are not linked
Please review this patch that fixes the issue where type annotations on
primitive types are not linked.
Tested with file https://cr.openjdk.org/~liach/8325433-arrayanno/ArrayAnno.java
import java.lang.annotation.*;
public class ArrayAnno {
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE_USE)
@Documented
public @interface Anno { int value(); }
public void method(@Anno(1) int @Anno(2) [] @Anno(3) [] arg) {}
public void method(@Anno(1) String @Anno(2) [] @Anno(3) [] arg) {}
}
JDK 21: https://cr.openjdk.org/~liach/8325433-arrayanno/old/ArrayAnno.html
This patch: https://cr.openjdk.org/~liach/8325433-arrayanno/new/ArrayAnno.html
Note that a bug within javac causes the annotations to become `@Anno(1) String
@Anno(3) [] @Anno(2) []` in the output files; this bug also affects output
class files so I assume this is a bug within javac's tree building.
(Intersting, the buggy javadoc output was copied wholesale in the original
`TestTypeAnnotations` output for `array2Deep` cases, but no one paid attention
to it)
-
Commit messages:
- 8325433: Type annotations on primitive types are not linked
Changes: https://git.openjdk.org/jdk/pull/18179/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18179&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8325433
Stats: 61 lines in 5 files changed: 52 ins; 0 del; 9 mod
Patch: https://git.openjdk.org/jdk/pull/18179.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18179/head:pull/18179
PR: https://git.openjdk.org/jdk/pull/18179
Integrated: 8323698: Class use page does not include extends/implements type arguments
On Sun, 14 Jan 2024 01:00:36 GMT, Chen Liang wrote:
> Currently in the javadoc tool, the generated class use page does not track
> the occurrences of a class in the type arguments of the extends or implements
> list, where they can appear. (See more details on the JBS issue) For example:
>
> public class One {}
>
>
> import java.util.*;
> public class Two extends ArrayList implements Comparator {
> }
>
>
> This patch proposes to add new 2 usage categories, "Classes that extend types
> with arguments of type" and "Classes that implement types with arguments of
> type" to describe the missing usage scenarios.
>
> A preview of the new use page with the 2 new categories, generated based on
> the example above, can be found here:
> https://cr.openjdk.org/~liach/8323698-javadocuse/doc/class-use/One.html
This pull request has now been integrated.
Changeset: 3a00fc73
Author:Chen Liang
Committer: Pavel Rappo
URL:
https://git.openjdk.org/jdk/commit/3a00fc732a959300a558d5062e5486220ea75192
Stats: 145 lines in 4 files changed: 133 ins; 0 del; 12 mod
8323698: Class use page does not include extends/implements type arguments
Reviewed-by: prappo, hannesw
-
PR: https://git.openjdk.org/jdk/pull/17414
Re: RFR: 8323698: Class use page does not include extends/implements type arguments [v6]
On Fri, 23 Feb 2024 05:33:07 GMT, Chen Liang wrote:
>> Currently in the javadoc tool, the generated class use page does not track
>> the occurrences of a class in the type arguments of the extends or
>> implements list, where they can appear. (See more details on the JBS issue)
>> For example:
>>
>> public class One {}
>>
>>
>> import java.util.*;
>> public class Two extends ArrayList implements Comparator {
>> }
>>
>>
>> This patch proposes to add new 2 usage categories, "Classes that extend
>> types with arguments of type" and "Classes that implement types with
>> arguments of type" to describe the missing usage scenarios.
>>
>> A preview of the new use page with the 2 new categories, generated based on
>> the example above, can be found here:
>> https://cr.openjdk.org/~liach/8323698-javadocuse/doc/class-use/One.html
>
> Chen Liang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Apply suggestions from code review
>
> Co-authored-by: Pavel Rappo <32523691+pavelra...@users.noreply.github.com>
I will wait 24 hours before integration to allow for more reviews.
-
PR Comment: https://git.openjdk.org/jdk/pull/17414#issuecomment-1960760470
Re: RFR: 8323698: Class use page does not include extends/implements type arguments [v7]
> Currently in the javadoc tool, the generated class use page does not track
> the occurrences of a class in the type arguments of the extends or implements
> list, where they can appear. (See more details on the JBS issue) For example:
>
> public class One {}
>
>
> import java.util.*;
> public class Two extends ArrayList implements Comparator {
> }
>
>
> This patch proposes to add new 2 usage categories, "Classes that extend types
> with arguments of type" and "Classes that implement types with arguments of
> type" to describe the missing usage scenarios.
>
> A preview of the new use page with the 2 new categories, generated based on
> the example above, can be found here:
> https://cr.openjdk.org/~liach/8323698-javadocuse/doc/class-use/One.html
Chen Liang 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' of https://github.com/openjdk/jdk into
feature/jd-use-super-typearg
- Improve documentation
- Apply suggestions from code review
Co-authored-by: Pavel Rappo <32523691+pavelra...@users.noreply.github.com>
- Distinguish subinterfaces, use clearer terms
- Merge branch 'master' of https://github.com/openjdk/jdk into
feature/jd-use-super-typearg
- Review changes
- Merge branch 'master' of https://github.com/openjdk/jdk into
feature/jd-use-super-typearg
- Fix tests
- Improve localization
- Merge branch 'master' of https://github.com/openjdk/jdk into
feature/jd-use-super-typearg
- ... and 1 more: https://git.openjdk.org/jdk/compare/ae586ba9...98587bc6
-
Changes:
- all: https://git.openjdk.org/jdk/pull/17414/files
- new: https://git.openjdk.org/jdk/pull/17414/files/e18aec67..98587bc6
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=17414&range=06
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=17414&range=05-06
Stats: 4170 lines in 42 files changed: 3340 ins; 406 del; 424 mod
Patch: https://git.openjdk.org/jdk/pull/17414.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17414/head:pull/17414
PR: https://git.openjdk.org/jdk/pull/17414
Re: RFR: 8323698: Class use page does not include extends/implements type arguments [v5]
On Thu, 22 Feb 2024 13:26:34 GMT, Pavel Rappo wrote:
>> Chen Liang 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 eight additional
>> commits since the last revision:
>>
>> - Distinguish subinterfaces, use clearer terms
>> - Merge branch 'master' of https://github.com/openjdk/jdk into
>> feature/jd-use-super-typearg
>> - Review changes
>> - Merge branch 'master' of https://github.com/openjdk/jdk into
>> feature/jd-use-super-typearg
>> - Fix tests
>> - Improve localization
>> - Merge branch 'master' of https://github.com/openjdk/jdk into
>> feature/jd-use-super-typearg
>> - Use page to check extends/implements type arguments
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
> line 286:
>
>> 284: subclasses with parameters of type A. \
>> 285: You can access this page by first going to the package, class or
>> interface, then clicking on \
>> 286: the USE link in the navigation bar.
>
> Thanks for catching this! Consider this wording:
>
>
> doclet.help.use.body=\
> Each documented package, class or interface has its own Use page, which
> lists \
> packages, classes, interfaces, methods, constructors and fields that use
> any \
> part of that package, class or interface. Given a class or interface A,
> its \
> Use page includes subclasses or subinterfaces of A, fields declared as A,
> \
> methods that return A, methods and constructors with parameters of type
> A, \
> and subclasses or subinterfaces with parameters of type A. You can access
> \
> this page by first going to the package, class or interface, then
> clicking \
> on the USE link in the navigation bar.
>
>
> There are a couple of other issues there which should be fixed in a separate
> PR.
This is an improvement. Applying as I don't see any problem in your wording.
("class or interface" is an established term)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17414#discussion_r1500226393
Re: RFR: 8323698: Class use page does not include extends/implements type arguments [v6]
> Currently in the javadoc tool, the generated class use page does not track
> the occurrences of a class in the type arguments of the extends or implements
> list, where they can appear. (See more details on the JBS issue) For example:
>
> public class One {}
>
>
> import java.util.*;
> public class Two extends ArrayList implements Comparator {
> }
>
>
> This patch proposes to add new 2 usage categories, "Classes that extend types
> with arguments of type" and "Classes that implement types with arguments of
> type" to describe the missing usage scenarios.
>
> A preview of the new use page with the 2 new categories, generated based on
> the example above, can be found here:
> https://cr.openjdk.org/~liach/8323698-javadocuse/doc/class-use/One.html
Chen Liang has updated the pull request incrementally with one additional
commit since the last revision:
Apply suggestions from code review
Co-authored-by: Pavel Rappo <32523691+pavelra...@users.noreply.github.com>
-
Changes:
- all: https://git.openjdk.org/jdk/pull/17414/files
- new: https://git.openjdk.org/jdk/pull/17414/files/bdd03743..e18aec67
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=17414&range=05
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=17414&range=04-05
Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/17414.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17414/head:pull/17414
PR: https://git.openjdk.org/jdk/pull/17414
Re: RFR: 8323698: Class use page does not include extends/implements type arguments [v5]
On Thu, 22 Feb 2024 04:53:20 GMT, Chen Liang wrote:
>> Currently in the javadoc tool, the generated class use page does not track
>> the occurrences of a class in the type arguments of the extends or
>> implements list, where they can appear. (See more details on the JBS issue)
>> For example:
>>
>> public class One {}
>>
>>
>> import java.util.*;
>> public class Two extends ArrayList implements Comparator {
>> }
>>
>>
>> This patch proposes to add new 2 usage categories, "Classes that extend
>> types with arguments of type" and "Classes that implement types with
>> arguments of type" to describe the missing usage scenarios.
>>
>> A preview of the new use page with the 2 new categories, generated based on
>> the example above, can be found here:
>> https://cr.openjdk.org/~liach/8323698-javadocuse/doc/class-use/One.html
>
> Chen Liang 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 eight additional commits since
> the last revision:
>
> - Distinguish subinterfaces, use clearer terms
> - Merge branch 'master' of https://github.com/openjdk/jdk into
> feature/jd-use-super-typearg
> - Review changes
> - Merge branch 'master' of https://github.com/openjdk/jdk into
> feature/jd-use-super-typearg
> - Fix tests
> - Improve localization
> - Merge branch 'master' of https://github.com/openjdk/jdk into
> feature/jd-use-super-typearg
> - Use page to check extends/implements type arguments
Thanks for your review, Pavel! I've addressed your changes and updated the
preview on CR.
1. `interface MyList> extends java.util.List`
This case is already perfectly handled by existing
`doclet.ClassUse_TypeParameter`, since `One` `Two` `Three` here exists in a
type parameter's bounds, which will be scanned by a visitor and recorded
correctly.
2. For generic interfaces, I have reworded the usage categories; closer to the
simple subclass version.
3. Have split out the subinterfaces in a separate category and added it to
tests.
4. These being used as a type argument and as a bound are unfortunately not
distinguished by our existing type parameter handling, in both type parameter
declarations in classes/methods and parameterized type usages in field/method
types. I don't think it's in this patch's scope to change this behavior, and if
this should be changed, all those sites should be updated as well.
Feel free to suggest improvments to the wordings!
-
PR Comment: https://git.openjdk.org/jdk/pull/17414#issuecomment-1958706531
Re: RFR: 8323698: Class use page does not include extends/implements type arguments [v5]
> Currently in the javadoc tool, the generated class use page does not track
> the occurrences of a class in the type arguments of the extends or implements
> list, where they can appear. (See more details on the JBS issue) For example:
>
> public class One {}
>
>
> import java.util.*;
> public class Two extends ArrayList implements Comparator {
> }
>
>
> This patch proposes to add new 2 usage categories, "Classes that extend types
> with arguments of type" and "Classes that implement types with arguments of
> type" to describe the missing usage scenarios.
>
> A preview of the new use page with the 2 new categories, generated based on
> the example above, can be found here:
> https://cr.openjdk.org/~liach/8323698-javadocuse/doc/class-use/One.html
Chen Liang 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 eight additional commits since
the last revision:
- Distinguish subinterfaces, use clearer terms
- Merge branch 'master' of https://github.com/openjdk/jdk into
feature/jd-use-super-typearg
- Review changes
- Merge branch 'master' of https://github.com/openjdk/jdk into
feature/jd-use-super-typearg
- Fix tests
- Improve localization
- Merge branch 'master' of https://github.com/openjdk/jdk into
feature/jd-use-super-typearg
- Use page to check extends/implements type arguments
-
Changes:
- all: https://git.openjdk.org/jdk/pull/17414/files
- new: https://git.openjdk.org/jdk/pull/17414/files/384a8ea0..bdd03743
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=17414&range=04
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=17414&range=03-04
Stats: 43964 lines in 1372 files changed: 21530 ins; 14004 del; 8430 mod
Patch: https://git.openjdk.org/jdk/pull/17414.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17414/head:pull/17414
PR: https://git.openjdk.org/jdk/pull/17414
Re: RFR: 8323698: Class use page does not include extends/implements type arguments [v4]
On Tue, 30 Jan 2024 04:16:54 GMT, Chen Liang wrote:
>> Currently in the javadoc tool, the generated class use page does not track
>> the occurrences of a class in the type arguments of the extends or
>> implements list, where they can appear. (See more details on the JBS issue)
>> For example:
>>
>> public class One {}
>>
>>
>> import java.util.*;
>> public class Two extends ArrayList implements Comparator {
>> }
>>
>>
>> This patch proposes to add new 2 usage categories, "Classes that extend
>> types with arguments of type" and "Classes that implement types with
>> arguments of type" to describe the missing usage scenarios.
>>
>> A preview of the new use page with the 2 new categories, generated based on
>> the example above, can be found here:
>> https://cr.openjdk.org/~liach/8323698-javadocuse/doc/class-use/One.html
>
> Chen Liang 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 six additional commits since
> the last revision:
>
> - Review changes
> - Merge branch 'master' of https://github.com/openjdk/jdk into
> feature/jd-use-super-typearg
> - Fix tests
> - Improve localization
> - Merge branch 'master' of https://github.com/openjdk/jdk into
> feature/jd-use-super-typearg
> - Use page to check extends/implements type arguments
@hns @jonathan-gibbons Sorry for pinging, can you review this pr again?
-
PR Comment: https://git.openjdk.org/jdk/pull/17414#issuecomment-1954982082
Re: RFR: 8323698: Class use page does not include extends/implements type arguments [v4]
> Currently in the javadoc tool, the generated class use page does not track
> the occurrences of a class in the type arguments of the extends or implements
> list, where they can appear. (See more details on the JBS issue) For example:
>
> public class One {}
>
>
> import java.util.*;
> public class Two extends ArrayList implements Comparator {
> }
>
>
> This patch proposes to add new 2 usage categories, "Classes that extend types
> with arguments of type" and "Classes that implement types with arguments of
> type" to describe the missing usage scenarios.
>
> A preview of the new use page with the 2 new categories, generated based on
> the example above, can be found here:
> https://cr.openjdk.org/~liach/8323698-javadocuse/doc/class-use/One.html
Chen Liang 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 six additional commits since the
last revision:
- Review changes
- Merge branch 'master' of https://github.com/openjdk/jdk into
feature/jd-use-super-typearg
- Fix tests
- Improve localization
- Merge branch 'master' of https://github.com/openjdk/jdk into
feature/jd-use-super-typearg
- Use page to check extends/implements type arguments
-
Changes:
- all: https://git.openjdk.org/jdk/pull/17414/files
- new: https://git.openjdk.org/jdk/pull/17414/files/aeca337b..384a8ea0
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=17414&range=03
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=17414&range=02-03
Stats: 15704 lines in 1204 files changed: 4544 ins; 1396 del; 9764 mod
Patch: https://git.openjdk.org/jdk/pull/17414.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17414/head:pull/17414
PR: https://git.openjdk.org/jdk/pull/17414
Re: RFR: 8323698: Class use page does not include extends/implements type arguments [v3]
> Currently in the javadoc tool, the generated class use page does not track
> the occurrences of a class in the type arguments of the extends or implements
> list, where they can appear. (See more details on the JBS issue) For example:
>
> public class One {}
>
>
> import java.util.*;
> public class Two extends ArrayList implements Comparator {
> }
>
>
> This patch proposes to add new 2 usage categories, "Classes that extend types
> with arguments of type" and "Classes that implement types with arguments of
> type" to describe the missing usage scenarios.
>
> A preview of the new use page with the 2 new categories, generated based on
> the example above, can be found here:
> https://cr.openjdk.org/~liach/8323698-javadocuse/doc/class-use/One.html
Chen Liang has updated the pull request incrementally with one additional
commit since the last revision:
Fix tests
-
Changes:
- all: https://git.openjdk.org/jdk/pull/17414/files
- new: https://git.openjdk.org/jdk/pull/17414/files/5001306d..aeca337b
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=17414&range=02
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=17414&range=01-02
Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/17414.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17414/head:pull/17414
PR: https://git.openjdk.org/jdk/pull/17414
