Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v3]
On Tue, 31 May 2022 20:32:13 GMT, Joe Darcy wrote: >> Time to start getting ready for JDK 20... > > Joe Darcy 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 36 additional commits since > the last revision: > > - Update deprecated options test. > - Merge branch 'master' into JDK-8284858 > - Respond to review feedback. > - Update symbol information for JDK 19 b24. > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Update symbol information for JDK 19 b23. > - ... and 26 more: > https://git.openjdk.java.net/jdk/compare/44ff5c88...eedd36bd langtools files look OK, although I did skim through the auto-generated new data/symbols files. There are possibilities in the langtools file to simplify naming and default initialization of member fields in various places, in a separate PR, separate from these changes. - Marked as reviewed by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: 8286654: Add an optional description accessor on ToolProvider interface
On Wed, 18 May 2022 14:56:47 GMT, Christian Stein wrote: > This PR adds an optional description accessor on `ToolProvider` interface. > > This PR also adds short description for each of the listed tool: > - `jar` > - `javac` > - `javadoc` > - `javap` and `jdeps` > - `jlink` and `jmod` > - `jpackage` Marked as reviewed by jjg (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8772
Re: RFR: 8286654: Add an optional description accessor on ToolProvider interface
On Wed, 18 May 2022 14:56:47 GMT, Christian Stein wrote: > This PR adds an optional description accessor on `ToolProvider` interface. > > This PR also adds short description for each of the listed tool: > - `jar` > - `javac` > - `javadoc` > - `javap` and `jdeps` > - `jlink` and `jmod` > - `jpackage` Main issue is the one I raised at the beginning, about what grammatical form you should be using for the short description. Also, is the description a phrase (probably not capitalized, and no terminating period) or a sentence (capitalized with a terminating period.) src/jdk.compiler/share/classes/com/sun/tools/javac/main/JavacToolProvider.java line 30: > 28: import com.sun.tools.javac.util.JavacMessages; > 29: import java.io.PrintWriter; > 30: import java.util.Optional; at least in javac, we normally sort `java.*` and `javax.*` imports before other imports. src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties line 387: > 385: > 386: javac.description=Read Java declarations and compile them into class > files > 387: for your general consideration, what grammatical style do you recommend here? Should it be 3rd person (Reads) instead of 2nd person (Read) - PR: https://git.openjdk.java.net/jdk/pull/8772
Re: RFR: 8284209: Replace remaining usages of 'a the' in source code
On Wed, 18 May 2022 14:46:42 GMT, Alexey Ivanov wrote: > Replaces usages of articles that follow each other in all combinations: > a/the, an?/an?, the/the… > > It's the last issue in the series, and it still touches different areas of > the code. javac and javadoc changes look OK test/langtools/tools/javac/modules/T8168854/module-info.java line 4: > 2: * @test > 3: * @bug 8168854 > 4: * @summary javac erroneously reject a service interface inner class in a > provides clause FYI, this duplication was in the JBS issue summary; now fixed there. - Marked as reviewed by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8771
Withdrawn: JDK-8286347: incorrect use of `{@link}`
On Fri, 6 May 2022 23:30:44 GMT, Jonathan Gibbons wrote:
> Please review a small doc fix to update incorrect use of `{@link}` on arrays
> of primitive types.
This pull request has been closed without being integrated.
-
PR: https://git.openjdk.java.net/jdk/pull/8584
Re: RFR: JDK-8286347: incorrect use of `{@link}`
On Sat, 7 May 2022 01:27:57 GMT, Joe Darcy wrote: > Looks fine in and of itself, but not sure how it will interact with the > (presumed) integration of JEP 424: "Foreign Function & Memory API (Preview)" > which will at least move the file, if not otherwise modify it. I assume the changes are small and localized enough to not trigger any significant conflict. - PR: https://git.openjdk.java.net/jdk/pull/8584
RFR: JDK-8286347: incorrect use of `{@link}`
Please review a small doc fix to update incorrect use of `{@link}` on arrays of
primitive types.
-
Commit messages:
- JDK-8286347: incorrect use of `{@link}`
Changes: https://git.openjdk.java.net/jdk/pull/8584/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8584&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8286347
Stats: 7 lines in 1 file changed: 0 ins; 0 del; 7 mod
Patch: https://git.openjdk.java.net/jdk/pull/8584.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8584/head:pull/8584
PR: https://git.openjdk.java.net/jdk/pull/8584
Re: RFR: JDK-8285977: Add links to IEEE 754 specification
On Mon, 2 May 2022 22:55:43 GMT, Joe Darcy wrote: > Please review the addition of @-see links from classes that mention the IEEE > 754 floating-point standard to an IEEE page about the standard. The URL in > the initial version of the PR is the top search result on the IEEE home page > for "754 standard". > > Another candidate page to use is > > https://ieeexplore.ieee.org/servlet/opac?punumber=8766227 > > which is (apparently) a stable citation page for the standard. > > These links may be upgraded to the the forthcoming @-spec facility in the > future. ( JDK-6251738). I endorse the use of a URL with a host-name like `standards.ieee.org` - Marked as reviewed by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8511
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces
On Tue, 26 Apr 2022 22:24:26 GMT, Joe Darcy wrote:
> To enable more complete doclint checking (courtesy @jonathan-gibbons), please
> review this PR to add type-level @param tags where they are missing.
>
> To the maintainers of java.util.concurrent, those changes could be separated
> out in another bug if that would ease maintenance of that code.
>
> Making these library fixes is a blocker for correcting and expanding the
> doclint checks (JDK-8285496).
>
> I'll update copyright years before pushing.
src/java.base/share/classes/java/lang/ClassValue.java line 43:
> 41: * it can use a {@code ClassValue} to cache information needed to
> 42: * perform the message send quickly, for each class encountered.
> 43: * @param type of the derived value
stylistically, compared to other comments, you are missing an initial "the"
-
PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: JDK-8283480: Make AbstractStringBuilder sealed
On Tue, 22 Mar 2022 00:01:59 GMT, Joe Darcy wrote: > As part of updating the core libraries to be sealed, the package-access > AbstractStringBuilder, implementation superclass of StringBuilder and > StringBuffer, can be marked as sealed with those two subclasses on its > permits list. Marked as reviewed by jjg (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7898
Re: RFR: 8257733: Move module-specific data from make to respective module [v13]
On Mon, 21 Mar 2022 16:29:25 GMT, Magnus Ihse Bursie wrote: >> A lot (but not all) of the data in make/data is tied to a specific module. >> For instance, the publicsuffixlist is used by java.base, and fontconfig by >> java.desktop. (A few directories, like mainmanifest, is *actually* used by >> make for the whole build.) >> >> These data files should move to the module they belong to. The are, after >> all, "source code" for that module that is "compiler" into resulting >> deliverables, for that module. (But the "source code" language is not Java >> or C, but typically a highly domain specific language or data format, and >> the "compilation" is, often, a specialized transformation.) >> >> This misplacement of the data directory is most visible at code review time. >> When such data is changed, most of the time build-dev (or the new build >> label) is involved, even though this has nothing to do with the build. While >> this is annoying, a worse problem is if the actual team that needs to review >> the patch (i.e., the team owning the module) is missed in the review. >> >> ### Modules reviewed >> >> - [x] java.base >> - [x] java.desktop >> - [x] jdk.compiler >> - [x] java.se > > Magnus Ihse Bursie 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' into shuffle-data > - Correct name for bfc files > - Merge tag 'jdk-19+14' into shuffle-data > >Added tag jdk-19+14 for changeset 08cadb47 > - Move x11wrappergen from share/data to unix/data > - Fix fontconfig according to review request > - Fix typos > - Restore charsetmapping and cldr to make/data > - Update copyright year > - Merge branch 'master' into shuffle-data-reborn > - Fix merge > - ... and 10 more: > https://git.openjdk.java.net/jdk/compare/19d34bdf...1c47d82d As before, the changes for `jdk.compiler` and `idk.javadoc` look OK, and if there is general consensus on the overall proposed move, then I think that overall it is a good idea. I did not review the `jdk.compiler` in low-level detail, but scanning the webrev index file, the changes seemed OK, and a pure move (no lines changed.) It might have been better to have handled this on a per-module basis, rather than all-at-once, but whatever ... - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: JDK-8283230: Improve @jls usage in ElementType
On Wed, 16 Mar 2022 00:33:57 GMT, Joe Darcy wrote: > Improve some semantic links into JLS with actual links. Marked as reviewed by jjg (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7831
Re: RFR: 8280825: Modules that "provide" ToolProvider should document the name that can be used
On Thu, 10 Feb 2022 11:24:23 GMT, Lance Andersen wrote:
>> Perhaps like this?
>>
>>
>> /**
>> * ...
>> * @provides java.util.spi.ToolProvider
>> * Module {@code jdk.jartool} provides a tool named {@code "jar"}.
>> * Invoke {@link java.util.spi.ToolProvider#findFirst
>> ToolProvider.findFirst("jar")}
>> * to create an instance of this tool.
>> * ...
>> */
>
>> Perhaps like this?
>>
>> ```java
>> /**
>> * ...
>> * @provides java.util.spi.ToolProvider
>> * Module {@code jdk.jartool} provides a tool named {@code "jar"}.
>> * Invoke {@link java.util.spi.ToolProvider#findFirst
>> ToolProvider.findFirst("jar")}
>> * to create an instance of this tool.
>> * ...
>> */
>> ```
>
> What about
>
> `Module {@code jdk.jartool) provides the equivalent of command-line access to
> the {@code "jar"} tool`
The focus should be to document the service specified in the `@provides`
directive, and how to access to access an instance of the service.
How about:
Use `TP.findFirst("NAME")` to obtain an instance of a `ToolProvider` that
provides the equivalent of command-line access to the {@code "NAME"} tool.
-
PR: https://git.openjdk.java.net/jdk/pull/7406
Re: RFR: 8280825: Modules that "provide" ToolProvider should document the name that can be used
On Wed, 9 Feb 2022 20:33:02 GMT, Alan Bateman wrote:
>> A number of modules declare that the "provide" ToolProvider.
>>
>> These modules now specify the "name" of the argument used by
>> `ToolProvider.findFirst` to access an instance of the tool provider within
>> the description part of a `@provides` API tag.
>
> src/jdk.jartool/share/classes/module-info.java line 45:
>
>> 43: * Pass {@code "jar"} as the name to
>> 44: * {@link java.util.spi.ToolProvider#findFirst
>> ToolProvider.findFirst}
>> 45: * in order to obtain an instance of the tool.
>
> I'm not sure about the wording. It might be better to say that it provides a
> tool named "jar". Invoke findFirst("jar") to create an instance of this tool.
What is "it" in "it provides..." ?
-
PR: https://git.openjdk.java.net/jdk/pull/7406
Re: RFR: 8280825: Modules that "provide" ToolProvider should document the name that can be used
On Wed, 9 Feb 2022 16:37:00 GMT, Christian Stein wrote: > A number of modules declare that the "provide" ToolProvider. > > These modules now specify the "name" of the argument used by > `ToolProvider.findFirst` to access an instance of the tool provider within > the description part of a `@provides` API tag. Would it be more pedantically accurate to talk about getting an instance of the _tool provider_ (not just _tool_)? - Marked as reviewed by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7406
Re: RFR: 8280825: Modules that "provide" ToolProvider should document the name that can be used
On Wed, 9 Feb 2022 16:37:00 GMT, Christian Stein wrote: > A number of modules declare that the "provide" ToolProvider. > > These modules now specify the "name" of the argument used by > `ToolProvider.findFirst` to access an instance of the tool provider within > the description part of a `@provides` API tag. Minor format suggestion: add a newline after the service name, to start the description on its own line, as in: @provides This form would extend well to any modules that provide the service in different ways ... i.e. with different names for `TP.findFirst` to obtain different instances. - PR: https://git.openjdk.java.net/jdk/pull/7406
[jdk18] Integrated: JDK-8279179: Update nroff pages in JDK 18 before RC
On Fri, 21 Jan 2022 18:37:50 GMT, Jonathan Gibbons wrote: > Please review this semi-automated update to the nroff man pages from the > upstream repo. This pull request has now been integrated. Changeset: 7d2ef9d9 Author:Jonathan Gibbons URL: https://git.openjdk.java.net/jdk18/commit/7d2ef9d984f96cd260dc233c4acf58669615227f Stats: 34 lines in 3 files changed: 32 ins; 0 del; 2 mod 8279179: Update nroff pages in JDK 18 before RC Reviewed-by: iris, mchung - PR: https://git.openjdk.java.net/jdk18/pull/112
[jdk18] RFR: JDK-8279179: Update nroff pages in JDK 18 before RC
Please review this semi-automated update to the nroff man pages from the upstream repo. - Commit messages: - JDK-8279179: Update nroff pages in JDK 18 before RC Changes: https://git.openjdk.java.net/jdk18/pull/112/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk18&pr=112&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8279179 Stats: 34 lines in 3 files changed: 32 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk18/pull/112.diff Fetch: git fetch https://git.openjdk.java.net/jdk18 pull/112/head:pull/112 PR: https://git.openjdk.java.net/jdk18/pull/112
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 14:01:04 GMT, Pavel Rappo wrote: >> - Most of the typos are of a trivial kind: missing whitespace. >> - If any of the typos should be fixed in the upstream projects instead, >> please say so; I will drop those typos from the patch. >> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant >> formatting artefact and thus can be removed. >> - `'` is an apostrophe, which does not require to be encoded. > > Pavel Rappo has updated the pull request incrementally with one additional > commit since the last revision: > > Additional typos jdk.compiler and jdk.javadoc look good - Marked as reviewed by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 11:40:20 GMT, Lance Andersen wrote: >> OK, so lines 264, 295, 329, 364, 431 are arguably wrong as well? >> Separating the [] completely looks quite rare. >> I'll leave it up to you. 8-) > > I think that can be a follow on clean up. The strange formatting of `long []updateCounts` looks very unusual and well worth a followup cleanup. - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8271079: JavaFileObject#toUri and multi-release jars [v4]
On Sat, 11 Dec 2021 11:29:50 GMT, Christian Stein wrote: >> Prior to this PR, `toUri()` of class `ZipPath` in module `jdk.zipfs` and >> class `PathFileObject` in module `jdk.compiler` were always composed by base >> path names. Even for versioned entries of a multi-release JAR file. >> >> Now, a `URI` for an entry is composed of its real path names using an >> existing lookup function in the associated zip file system object. >> >> This PR also removes a superseded work around for >> [JDK-8134451](https://bugs.openjdk.java.net/browse/JDK-8134451). >> >> Fixes https://bugs.openjdk.java.net/browse/JDK-8271079 > > Christian Stein has updated the pull request incrementally with one > additional commit since the last revision: > > Add bug number `8271079` to ZipFS tester Regarding the javac workaround, as best I can tell, the `createJarUri` code goes all the way back to the original file manager code in JDK 6. The workaround comment appeared in JDK 9, as part of the general conversion from the old File-based file manager to the new Path-based file manager. So yes, there does not appear to be a specific test for the workaround, and it's not clear it's worth bisecting the changes in the transition from JDK 8 to JDK 9 to investigate further. - PR: https://git.openjdk.java.net/jdk/pull/6768
Re: [jdk18] RFR: JDK-8273179: Update nroff pages in JDK 18 before RC
On Fri, 10 Dec 2021 01:46:03 GMT, Jonathan Gibbons wrote: > Please review this semi-automatic update for the nroff man pages for JDK 18. > The changes update the version number, copyright year, and incorporate the > changes from the latest upstream files. hmmm, I thought we had taken care of that test. I will investigate - PR: https://git.openjdk.java.net/jdk18/pull/5
[jdk18] Integrated: JDK-8273179: Update nroff pages in JDK 18 before RC
On Fri, 10 Dec 2021 01:46:03 GMT, Jonathan Gibbons wrote: > Please review this semi-automatic update for the nroff man pages for JDK 18. > The changes update the version number, copyright year, and incorporate the > changes from the latest upstream files. This pull request has now been integrated. Changeset: ed5d53ae Author: Jonathan Gibbons URL: https://git.openjdk.java.net/jdk18/commit/ed5d53ae0eb0b12de11fb3d79ae0371c093ce434 Stats: 858 lines in 28 files changed: 448 ins; 146 del; 264 mod 8273179: Update nroff pages in JDK 18 before RC Reviewed-by: dholmes - PR: https://git.openjdk.java.net/jdk18/pull/5
Re: [jdk18] RFR: JDK-8273179: Update nroff pages in JDK 18 before RC
On Fri, 10 Dec 2021 01:46:03 GMT, Jonathan Gibbons wrote: > Please review this semi-automatic update for the nroff man pages for JDK 18. > The changes update the version number, copyright year, and incorporate the > changes from the latest upstream files. Hi David, The copyright year will naturally sort itself out in a few weeks time ;-) When these changes make their way down from 18 to 19, we will probably want to regenerate these files with 19-EA. -- Jon We will also want to regenerate any appropriate files if any more updates to the man pages are made during the ramp down period. - PR: https://git.openjdk.java.net/jdk18/pull/5
[jdk18] RFR: JDK-8273179: Update nroff pages in JDK 18 before RC
Please review this semi-automatic update for the nroff man pages for JDK 18. The changes update the version number, copyright year, and incorporate the changes from the latest upstream files. - Commit messages: - JDK-8273179: Update nroff pages in JDK 18 before RC Changes: https://git.openjdk.java.net/jdk18/pull/5/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk18&pr=5&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273179 Stats: 858 lines in 28 files changed: 448 ins; 146 del; 264 mod Patch: https://git.openjdk.java.net/jdk18/pull/5.diff Fetch: git fetch https://git.openjdk.java.net/jdk18 pull/5/head:pull/5 PR: https://git.openjdk.java.net/jdk18/pull/5
Re: RFR: JDK-8276674 : Malformed Javadoc inline tags in JDK source in java/util/stream
On Thu, 18 Nov 2021 03:22:50 GMT, Tim Prinzing wrote: > JDK-8276674 : Malformed Javadoc inline tags in JDK source in java/util/stream Remarkable to have not been noticed for so long! - Marked as reviewed by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6443
Re: RFR: 8277806: 4 tools/jar failures per platform after JDK-8272728
On Wed, 24 Nov 2021 20:08:50 GMT, Lance Andersen wrote: > HI all, > > Attached is a patch for 4 failing MR tests due the issue that was resolved > via JDK-8272728 > > Best > Lance Marked as reviewed by jjg (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6546
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v5]
On Mon, 1 Nov 2021 16:10:26 GMT, Ichiroh Takiguchi wrote: >> JEP-400 (UTF-8 by Default) was eabled on JDK18-b13. >> After JDK18-b13, javac and some other langtool command's usage were garbled >> on Japanese Windows. >> These commands use PrintWriter instead of standard out/err with PrintStream. > > Ichiroh Takiguchi 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 five additional > commits since the last revision: > > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - Langtools command's usage were grabled on Japanese Windows Generally, I think it would be a good goal for JEP-400 to not require any source-code changes to any use-sites, at least for this common idiom of wrapping a `PrintStream` in a `PrintWriter`. While we may have the ability to change JDK use-sites, we do not have the ability to change any usages outside of JDK, and we should try not to break those usages in the way that the JDK tools have been broken. - PR: https://git.openjdk.java.net/jdk/pull/5771
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v5]
On Wed, 10 Nov 2021 19:46:09 GMT, Alan Bateman wrote: > If there is resistance to fixing the tools then we might have to re-visit > this. It's not just the JDK tools that are an issue. I think that wrapping some PrintStream into a PrintWriter is a common enough idiom that it is not reasonable to fix all occurrences -- i.e. including those outside of JDK. - PR: https://git.openjdk.java.net/jdk/pull/5771
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v5]
On Wed, 10 Nov 2021 20:00:39 GMT, Alan Bateman wrote: > > 1. `PrintStream(OutputStream)` and `PrintStream(OutputStream, boolean)` > > should be redefined so that they internally check if the stream arg is a > > PrintStream, in which case they use the encoding from the `PrinStream` > > instead of the default. > > I think you mean the PrintWriter constructors here. Yes, there is merit in > that. PrintStream is a bit unusual in that it's an OutputStream but it has a > charset because it prints text output. Yes, sorry for the confusion. - PR: https://git.openjdk.java.net/jdk/pull/5771
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v5]
On Mon, 1 Nov 2021 16:10:26 GMT, Ichiroh Takiguchi wrote: >> JEP-400 (UTF-8 by Default) was eabled on JDK18-b13. >> After JDK18-b13, javac and some other langtool command's usage were garbled >> on Japanese Windows. >> These commands use PrintWriter instead of standard out/err with PrintStream. > > Ichiroh Takiguchi 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 five additional > commits since the last revision: > > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - Langtools command's usage were grabled on Japanese Windows Informally, I suggest one of the following: 1. `PrintStream(OutputStream)` and `PrintStream(OutputStream, boolean)` should be redefined so that they internally check if the stream arg is a PrintStream, in which case they use the encoding from the `PrinStream` instead of the default. 2. or, add new overloads for `PrintStream(PrintStream)` and `PrintStream(PrintStream, boolean)` that are defined to use the character encoding from the `PrintStream` arg. I note that `PrintStream` does not expose a "getter" for the encoding. That seems like a bug in itself, but even without fixing that, `PrintWriter` ought to be able to access the encoding "behind the scenes". - PR: https://git.openjdk.java.net/jdk/pull/5771
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v5]
On Mon, 1 Nov 2021 16:10:26 GMT, Ichiroh Takiguchi wrote: >> JEP-400 (UTF-8 by Default) was eabled on JDK18-b13. >> After JDK18-b13, javac and some other langtool command's usage were garbled >> on Japanese Windows. >> These commands use PrintWriter instead of standard out/err with PrintStream. > > Ichiroh Takiguchi has updated the pull request with a new target base due to > a merge or a rebase. The pull request now contains five commits: > > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - Langtools command's usage were grabled on Japanese Windows I strongly dislike this proposed changeset. In my opinion, the original change that has provoked the changes here is a bad/incompatible change, and should maybe be reconsidered. The fact that a change in the Java runtime has triggered the need for so many changes in application-style code is some sort of "canary in the coalmine". Generally, the issue is related to the fact that we wrap a PrintStream in a PrintWriter ... and, if I understand correctly, the writer ends up with the wrong character encoding. Is it possible to change PrintWriter and/or PrintStream so that the correct underlying encoding used by the PrintStream is also used by the PrintWriter. That way, all existing uses where a PrintWriter wraps a PrintStream would continue to work without any modification. cc: @jddarcy with his CSR hat on, for the compatibility issues relating to the issue that caused the problems being addressed here. - Changes requested by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5771
Re: RFR: 8275512: Upgrade required version of jtreg to 6.1 [v2]
On Tue, 19 Oct 2021 17:24:17 GMT, Weijun Wang wrote: >> As a follow up of JEP 411, we will soon disallow security manager by >> default. jtreg 6.1 does not set its own security manager if JDK version is >> >= 18. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > upgrade the version in GHA config > > only in patch2: > unchanged: Marked as reviewed by jjg (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6012
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v3]
On Wed, 6 Oct 2021 05:04:28 GMT, Ichiroh Takiguchi wrote: >> JEP-400 (UTF-8 by Default) was eabled on JDK18-b13. >> After JDK18-b13, javac and some other langtool command's usage were garbled >> on Japanese Windows. >> These commands use PrintWriter instead of standard out/err with PrintStream. > > Ichiroh Takiguchi has updated the pull request incrementally with one > additional commit since the last revision: > > 8274544: Langtools command's usage were garbled on Japanese Windows This is pretty ugly code to be replicating so many times. What if the tools have been run in an environment where `System.out` and `System.err` have already been redirected in some manner, with `System.setOut` or `System.setErr`? You should not assume that `System.out` and `System.err` will always refer to the console. - PR: https://git.openjdk.java.net/jdk/pull/5771
Re: Dangling class-level javadoc comments in JDK
The poor-man's solution would be to detect common cases, with the comment appearing before a package or import statement. A more advanced solution would have to be in the lexer or parser, detecting that there is a "unused" comment when creating a tree node. -- Jon On 8/24/21 8:07 AM, Pavel Rappo wrote: On 24 Aug 2021, at 15:38, Jonathan Gibbons wrote: IIRC, the one in javadoc CommentUtils has recently been fixed. It might be worth a javac -Xlint option to detect/report such dangling comments. How would you currently implement that? Aren't comments on non-documentable constructs discarded early? At what point during compilation and how would you detect, for example, that this doc comment dangles? /** * IOUtils: A collection of IO-related public static methods. */ package sun.security.util; -Pavel -- Jon On 8/23/21 11:50 PM, Andrey Turbanov wrote: Hello. I found a few internal classes in the JDK codebase which don't have proper javadoc, but have dangling javadoc-like comments. Dangling Javadoc comments are ignored by the javadoc tool and IDE. Perhaps it was intentional to not add proper javadoc to them? I believe it's better to convert them to proper javadoc to make developing JDK a bit more friendly within IDE. What do you think? List of classes: com.sun.beans.editors.BooleanEditor com.sun.beans.editors.ByteEditor com.sun.beans.editors.DoubleEditor com.sun.beans.editors.FloatEditor com.sun.beans.editors.IntegerEditor com.sun.beans.editors.LongEditor com.sun.beans.editors.NumberEditor com.sun.beans.editors.ShortEditor com.sun.jndi.toolkit.dir.ContainmentFilter com.sun.jndi.toolkit.dir.LazySearchEnumerationImpl com.sun.security.auth.module.Crypt java.math.MutableBigInteger java.net.DefaultInterface javax.swing.GraphicsWrapper jdk.internal.access.JavaAWTFontAccess jdk.javadoc.internal.doclets.toolkit.CommentUtils sun.awt.X11.XAtom sun.awt.X11.XAwtState sun.java2d.xr.XRBackend sun.java2d.xr.XRDrawLine sun.jvm.hotspot.debugger.PageCache sun.jvm.hotspot.runtime.JavaThreadFactory sun.jvm.hotspot.utilities.Interval sun.jvm.hotspot.utilities.MessageQueueBackend sun.jvm.hotspot.utilities.RBTree sun.launcher.LauncherHelper sun.net.www.content.text.plain sun.net.www.protocol.file.FileURLConnection sun.nio.ch.Interruptible sun.security.pkcs.ParsingException sun.security.provider.SeedGenerator sun.security.util.ByteArrayTagOrder sun.security.util.IOUtils Andrey Turbanov
Re: Dangling class-level javadoc comments in JDK
IIRC, the one in javadoc CommentUtils has recently been fixed. It might be worth a javac -Xlint option to detect/report such dangling comments. -- Jon On 8/23/21 11:50 PM, Andrey Turbanov wrote: Hello. I found a few internal classes in the JDK codebase which don't have proper javadoc, but have dangling javadoc-like comments. Dangling Javadoc comments are ignored by the javadoc tool and IDE. Perhaps it was intentional to not add proper javadoc to them? I believe it's better to convert them to proper javadoc to make developing JDK a bit more friendly within IDE. What do you think? List of classes: com.sun.beans.editors.BooleanEditor com.sun.beans.editors.ByteEditor com.sun.beans.editors.DoubleEditor com.sun.beans.editors.FloatEditor com.sun.beans.editors.IntegerEditor com.sun.beans.editors.LongEditor com.sun.beans.editors.NumberEditor com.sun.beans.editors.ShortEditor com.sun.jndi.toolkit.dir.ContainmentFilter com.sun.jndi.toolkit.dir.LazySearchEnumerationImpl com.sun.security.auth.module.Crypt java.math.MutableBigInteger java.net.DefaultInterface javax.swing.GraphicsWrapper jdk.internal.access.JavaAWTFontAccess jdk.javadoc.internal.doclets.toolkit.CommentUtils sun.awt.X11.XAtom sun.awt.X11.XAwtState sun.java2d.xr.XRBackend sun.java2d.xr.XRDrawLine sun.jvm.hotspot.debugger.PageCache sun.jvm.hotspot.runtime.JavaThreadFactory sun.jvm.hotspot.utilities.Interval sun.jvm.hotspot.utilities.MessageQueueBackend sun.jvm.hotspot.utilities.RBTree sun.launcher.LauncherHelper sun.net.www.content.text.plain sun.net.www.protocol.file.FileURLConnection sun.nio.ch.Interruptible sun.security.pkcs.ParsingException sun.security.provider.SeedGenerator sun.security.util.ByteArrayTagOrder sun.security.util.IOUtils Andrey Turbanov
[jdk17] Integrated: JDK-8270872: Final nroff manpage update for JDK 17
On Thu, 5 Aug 2021 19:20:50 GMT, Jonathan Gibbons wrote: > Please review a semi-automatic update of the nroff man pages from the > upstream files. This pull request has now been integrated. Changeset: dfacda48 Author:Jonathan Gibbons URL: https://git.openjdk.java.net/jdk17/commit/dfacda488bfbe2e11e8d607a6d08527710286982 Stats: 289 lines in 27 files changed: 117 ins; 31 del; 141 mod 8270872: Final nroff manpage update for JDK 17 Reviewed-by: darcy, mr, iris, naoto - PR: https://git.openjdk.java.net/jdk17/pull/303
Re: [jdk17] RFR: JDK-8270872: Final nroff manpage update for JDK 17
On Thu, 5 Aug 2021 19:57:59 GMT, Naoto Sato wrote: >> Please review a semi-automatic update of the nroff man pages from the >> upstream files. > > src/jdk.hotspot.agent/share/man/jhsdb.1 line 1: > >> 1: .\" Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights >> reserved. > > This seems not correct? According to the comments in the makefile (`closed/make/UpdateOpenManPages.gmk`) the copyright line is taken from the original Markdown file, so if the year is wrong there, it will be wrong in the generated nroff file. I think it would be incorrect to edit the dates locally in these files, because they'll just be overwritten when we generate the files again. Ideally, the dates should be fixed (if necessary) in the Markdown files, but that seems out of scope for this P1. This is "just" an issue with copyright dates in source files ... and yes, while I know copyright dates are important, this problem is arguably part of an ongoing more general problem. I note that the generated files *do* correctly identify themselves with `2021` in the visible output generated to the console by the `man` command. - PR: https://git.openjdk.java.net/jdk17/pull/303
[jdk17] RFR: JDK-8270872: Final nroff manpage update for JDK 17
Please review a semi-automatic update of the nroff man pages from the upstream files. - Commit messages: - JDK-8270872: Final nroff manpage update for JDK 17 Changes: https://git.openjdk.java.net/jdk17/pull/303/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17&pr=303&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8270872 Stats: 289 lines in 27 files changed: 117 ins; 31 del; 141 mod Patch: https://git.openjdk.java.net/jdk17/pull/303.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/303/head:pull/303 PR: https://git.openjdk.java.net/jdk17/pull/303
[jdk17] Integrated: JDK-8249646: Runtime.exec(String, String[], File) documentation contains literal {@link ...}
On Tue, 29 Jun 2021 04:39:28 GMT, Jonathan Gibbons wrote:
> Please review a trivial `noreg-doc` fix for some javadoc tags for
> `java.lang.Runtime` for JDK17
This pull request has now been integrated.
Changeset: 25f9f19a
Author:Jonathan Gibbons
URL:
https://git.openjdk.java.net/jdk17/commit/25f9f19af9831e151a39518020aefa2c18fd7217
Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
8249646: Runtime.exec(String, String[], File) documentation contains literal
{@link ...}
Reviewed-by: sundar, iris
-
PR: https://git.openjdk.java.net/jdk17/pull/167
[jdk17] RFR: JDK-8249646: Runtime.exec(String, String[], File) documentation contains literal {@link ...}
Please review a trivial `noreg-doc` fix for some javadoc tags for
`java.lang.Runtime` for JDK17
-
Commit messages:
- JDK-8249646: Runtime.exec(String, String[], File) documentation contains
literal {@link ...}
Changes: https://git.openjdk.java.net/jdk17/pull/167/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk17&pr=167&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8249646
Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
Patch: https://git.openjdk.java.net/jdk17/pull/167.diff
Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/167/head:pull/167
PR: https://git.openjdk.java.net/jdk17/pull/167
Re: RFR: 8268768: idea.sh has been updated in surprising and incompatible ways
On Tue, 15 Jun 2021 15:35:14 GMT, Maurizio Cimadamore wrote: > I can push to 17 if desired, but I'll need a new PR for that pushing to 17 would be nice; the script is equally broken and unusable there. - PR: https://git.openjdk.java.net/jdk/pull/4492
Re: RFR: 8268768: idea.sh has been updated in surprising and incompatible ways
On Tue, 15 Jun 2021 14:04:56 GMT, Maurizio Cimadamore wrote: > As the title says (please also refer to the JBS issue which describes all the > issues in more details), the IDE support for IntelliJ has been updated with > many enhancements as part of a seemingly innocuous "path handling" fix. The > IDE doesn't appear to be usable in the same way it was in the past and many > functionalities have been broken as a result (including support for jtreg > test execution using the plugin). > > For the above reasons, I'm reverting the plugin and idea.sh code to last > known working version. Any _targeted_ fix can be re-applied after the revert. > Larger enhancements need to be discussed in the proper venue: > > https://openjdk.java.net/groups/ide-support/ Does this need backported to 17 (or pushed to 17 and "back"ported into 18)? - PR: https://git.openjdk.java.net/jdk/pull/4492
Integrated: JDK-8268150: tier2: test/jdk/tools/jpackage/junit/junit.java needs updating for jtreg 6
On Thu, 3 Jun 2021 00:47:45 GMT, Jonathan Gibbons wrote: > Please review a small test fix, to include hamcrest.jar, as required by the > latest version of JUnit in jtreg 6. This pull request has now been integrated. Changeset: ef01e478 Author:Jonathan Gibbons URL: https://git.openjdk.java.net/jdk/commit/ef01e478586c5676747195ea67c1864639305c0f Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8268150: tier2: test/jdk/tools/jpackage/junit/junit.java needs updating for jtreg 6 Reviewed-by: almatvee - PR: https://git.openjdk.java.net/jdk/pull/4328
RFR: JDK-8268150: tier2: test/jdk/tools/jpackage/junit/junit.java needs updating for jtreg 6
Please review a small test fix, to include hamcrest.jar, as required by the latest version of JUnit in jtreg 6. - Commit messages: - JDK-8268150: tier2: test/jdk/tools/jpackage/junit/junit.java needs updating for jtreg 6 Changes: https://git.openjdk.java.net/jdk/pull/4328/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4328&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268150 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4328.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4328/head:pull/4328 PR: https://git.openjdk.java.net/jdk/pull/4328
Integrated: JDK-8268147: need to update reference to testng module for jtreg6
On Wed, 2 Jun 2021 22:22:22 GMT, Jonathan Gibbons wrote: > Please review a test fix to refer to the new name of the TestNG module. This pull request has now been integrated. Changeset: d46a2c8e Author: Jonathan Gibbons URL: https://git.openjdk.java.net/jdk/commit/d46a2c8ecfac785ae2c935a507c3bcae2e76aba9 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8268147: need to update reference to testng module for jtreg6 Reviewed-by: dholmes, psandoz, naoto - PR: https://git.openjdk.java.net/jdk/pull/4325
RFR: JDK-8268147: need to update reference to testng module for jtreg6
Please review a test fix to refer to the new name of the TestNG module. - Commit messages: - JDK-8268147: need to update reference to testng module for jtreg6 Changes: https://git.openjdk.java.net/jdk/pull/4325/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4325&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268147 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4325.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4325/head:pull/4325 PR: https://git.openjdk.java.net/jdk/pull/4325
Re: RFR: JDK-8266254: Update to use jtreg 6 [v2]
> Please review the change to update to using jtreg 6. > > The primary change is to the jib-profiles.js file, which specifies the > version of jtreg to use, for those systems that rely on this file. In > addition, the `requiredVersion` has been updated in the various `TEST.ROOT` > files. > > All the tests that could be updated ahead of time have been updated. There > are a few tests remaining that need to be done at this time, because of the > change in the module name for TestNG 7.3. It changed from a default of > `testng` to and explicit `org.testng`. Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision: Update copyright years - Changes: - all: https://git.openjdk.java.net/jdk/pull/4315/files - new: https://git.openjdk.java.net/jdk/pull/4315/files/0d1e554a..4ef5614f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4315&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4315&range=00-01 Stats: 6 lines in 6 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/4315.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4315/head:pull/4315 PR: https://git.openjdk.java.net/jdk/pull/4315
Re: RFR: 8267706: bin/idea.sh tries to use cygpath on WSL [v2]
On Thu, 27 May 2021 17:45:40 GMT, Nikita Gubarkov wrote: >> 8267706: bin/idea.sh tries to use cygpath on WSL > > Nikita Gubarkov has updated the pull request incrementally with one > additional commit since the last revision: > > 8267706: Break long line in make/ide/idea/jdk/idea.gmk I filed a new issue yesterday, linked to the old one. There should be no need to create another. https://bugs.openjdk.java.net/browse/JDK-8268083 - PR: https://git.openjdk.java.net/jdk/pull/4190
RFR: JDK-8266254: Update to use jtreg 6
Please review the change to update to using jtreg 6. The primary change is to the jib-profiles.js file, which specifies the version of jtreg to use, for those systems that rely on this file. In addition, the `requiredVersion` has been updated in the various `TEST.ROOT` files. All the tests that could be updated ahead of time have been updated. There are a few tests remaining that need to be done at this time, because of the change in the module name for TestNG 7.3. It changed from a default of `testng` to and explicit `org.testng`. - Commit messages: - JDK-8266254: Update to use jtreg 6 Changes: https://git.openjdk.java.net/jdk/pull/4315/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4315&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266254 Stats: 17 lines in 11 files changed: 0 ins; 1 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/4315.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4315/head:pull/4315 PR: https://git.openjdk.java.net/jdk/pull/4315
Re: RFR: 8267706: bin/idea.sh tries to use cygpath on WSL [v2]
On Thu, 27 May 2021 17:45:40 GMT, Nikita Gubarkov wrote: >> 8267706: bin/idea.sh tries to use cygpath on WSL > > Nikita Gubarkov has updated the pull request incrementally with one > additional commit since the last revision: > > 8267706: Break long line in make/ide/idea/jdk/idea.gmk The fix was supposed to be just about disentangling cygwin and WSL on Windows ... not to "improve project setup on all platforms" ... and break project setup on macOS as a result. If nothing else, if you're changing the set of required dependencies, this needs to be documented. And installing `coreutils` seems to bring in a whole lot of unnecessary stuff. - PR: https://git.openjdk.java.net/jdk/pull/4190
Re: RFR: 8267706: bin/idea.sh tries to use cygpath on WSL [v2]
On Tue, 1 Jun 2021 22:20:25 GMT, Nikita Gubarkov wrote: >> The fix fails on a Mac, where `realpath` is not available by default. > > @jonathan-gibbons this can be fixed with `brew install coreutils`. We > probably need to check `realpath` availability in idea.sh and suggest > installing `coreutils` if it's not available @YaaZ I'm aware of the workaround, but the current situation is not acceptable and needs to be fixed. @erikj79 is there precedent for requiring the use of `brew` to install packages? - PR: https://git.openjdk.java.net/jdk/pull/4190
Re: RFR: 8267706: bin/idea.sh tries to use cygpath on WSL [v2]
On Thu, 27 May 2021 17:45:40 GMT, Nikita Gubarkov wrote: >> 8267706: bin/idea.sh tries to use cygpath on WSL > > Nikita Gubarkov has updated the pull request incrementally with one > additional commit since the last revision: > > 8267706: Break long line in make/ide/idea/jdk/idea.gmk The fix fails on a Mac, where `realpath` is not available by default. - PR: https://git.openjdk.java.net/jdk/pull/4190
Re: RFR: 8265961: Fix comments in logging.properties [v2]
On Mon, 26 Apr 2021 17:14:29 GMT, Pavel Rappo wrote: >> I had been looking for an example of a "properties" file when spotted a typo >> in `logging.properties`. I decided to proofread the file. That resulted in >> finding a few other issues. > > Pavel Rappo has updated the pull request incrementally with one additional > commit since the last revision: > > Rollback "in" -> "with"; remove more trailing whitespace Marked as reviewed by jjg (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3688
Re: RFR: 8263105: security-libs doclint cleanup
On Sat, 6 Mar 2021 07:31:09 GMT, Bradford Wetmore wrote:
> Fix various things pointed out by the most recent doclint run in the
> security-libs area.
>
> This is docs only: I will be checking doccheck/doclint, and will be running
> tier1/tier2 tests. Minor spot checks on generated files.
I've read the first 10 files. The edits are definitely in the right direction,
and will address the "missing comments" issues.
I'll leave it up to you and the others in your team to decide what level of
stylistic consistency you would like for the new comments, but just having a
relevant comment at all is a great start.
src/java.base/share/classes/java/security/BasicPermission.java line 497:
> 495: /**
> 496: * @serialData Default fields.
> 497: */
FWIW, this doc comment will be ignored, because it will be superseded by the
new comment on line 499. At some point doen the road, you may get a warning
from javac about an ignored doc comment.
src/java.base/share/classes/java/security/GuardedObject.java line 64:
> 62:
> 63: /**
> 64: * The guard object
add a period?
src/java.base/share/classes/java/security/PermissionCollection.java line 105:
> 103: * Whether this permission collection is read-only.
> 104: *
> 105: * If set, add() will throw an exception.
maybe use `{@code}` or `{@link}` on add?
src/java.base/share/classes/java/security/Permissions.java line 581:
> 579: /**
> 580: * @serialData Default fields.
> 581: */
Another ignored comment. I suggest just changing these to `/*` comments.
-
PR: https://git.openjdk.java.net/jdk/pull/2856
RFR: JDK-8263104: fix warnings for empty paragraphs
Please review some simple cleanup for empty `` tags. Two of the tags are completely redundant, and simply deleted. The other three, in _package.html_ files are generally undesirable. Although the presumed intent of the `id` declaration is to label the `@see` info, proximity in the source code does not ensure proximity in the generated code. The actual result is an empty paragraph at the end of the enclosing generated ``, and before the generated output for the `@since` tag. The better solution is to move the `id` declaration into the `@see `. - Commit messages: - JDK-8263104: fix warnings for empty paragraphs Changes: https://git.openjdk.java.net/jdk/pull/2850/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2850&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263104 Stats: 8 lines in 5 files changed: 0 ins; 4 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/2850.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2850/head:pull/2850 PR: https://git.openjdk.java.net/jdk/pull/2850
Integrated: JDK-8262892: minor typo in implSpec comment
On Tue, 2 Mar 2021 22:01:49 GMT, Jonathan Gibbons wrote: > Please review a trivial fix to insert a missing word. This pull request has now been integrated. Changeset: 93ffe6a6 Author: Jonathan Gibbons URL: https://git.openjdk.java.net/jdk/commit/93ffe6a6 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8262892: minor typo in implSpec comment Reviewed-by: bpb - PR: https://git.openjdk.java.net/jdk/pull/2798
RFR: JDK-8262892: minor typo in implSpec comment
Please review a trivial fix to insert a missing word. - Commit messages: - JDK-8262892: minor typo in implSpec comment Changes: https://git.openjdk.java.net/jdk/pull/2798/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2798&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8262892 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2798.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2798/head:pull/2798 PR: https://git.openjdk.java.net/jdk/pull/2798
Integrated: JDK-8262875: doccheck: empty paragraphs, etc in java.base module
On Tue, 2 Mar 2021 19:35:47 GMT, Jonathan Gibbons wrote: > Please review some minor doc fixes, for issues found by _doccheck_.There > are two kinds of errors that are addressed. > > 1. Incorrect use of ``. In HTML, `` marks the *beginning* of a > paragraph. It is not a terminator, to mark the end of a paragraph, or a > separator to mark the boundary between paragraphs. In particular, it should > not be used at the end of a description before a javadoc block tag, such as > `@param` or before other HTML block tags, like `` or ``. > > 2. References to the id `package-description`, following the recent > standardization of all ids generated by javadoc, This pull request has now been integrated. Changeset: 20b9ba53 Author:Jonathan Gibbons URL: https://git.openjdk.java.net/jdk/commit/20b9ba53 Stats: 9 lines in 8 files changed: 0 ins; 1 del; 8 mod 8262875: doccheck: empty paragraphs, etc in java.base module Reviewed-by: alanb, darcy, lancea - PR: https://git.openjdk.java.net/jdk/pull/2795
Re: RFR: JDK-8262875: doccheck: empty paragraphs, etc in java.base module
Bernd, With respect, I disagree. There is a difference between `text` and `text`. Anyway, the significant comment in this context is that it is being used as a terminator, at the logical end of the paragraph. doccheck is using the standard `htmltidy` command to detect issues in HTML, and it is that program that is reporting the empty paragraphs. As for the guide that you reference, it is very, very old. I note that it begins: Introduction *Principles* At Java Software, we have several guidelines i.e. note the "Java Software" ... -- Jon On 3/2/21 12:04 PM, Bernd Eckenfels wrote: Hello, Actually, in HTML was a separator, and in xhtml it should enclose paragraphs. However I was under the impression Javadoc always used the separator style (it would be strange to start the first sentence in Javadoc with . Is this doccheck enforcing a new policy? This officially Oracle guide for example has a different example: https://www.oracle.com/de/technical-resources/articles/java/javadoc-tool.html#format Gruss Bernd -- http://bernd.eckenfels.net Von: net-dev im Auftrag von Jonathan Gibbons Gesendet: Tuesday, March 2, 2021 8:41:03 PM An: core-libs-dev@openjdk.java.net ; hotspot-compiler-...@openjdk.java.net ; net-...@openjdk.java.net ; security-...@openjdk.java.net Betreff: RFR: JDK-8262875: doccheck: empty paragraphs, etc in java.base module Please review some minor doc fixes, for issues found by _doccheck_.There are two kinds of errors that are addressed. 1. Incorrect use of ``. In HTML, `` marks the *beginning* of a paragraph. It is not a terminator, to mark the end of a paragraph, or a separator to mark the boundary between paragraphs. In particular, it should not be used at the end of a description before a javadoc block tag, such as `@param` or before other HTML block tags, like `` or ``. 2. References to the id `package-description`, following the recent standardization of all ids generated by javadoc, - Commit messages: - JDK-8262875: doccheck: empty paragraphs, etc in java.base module Changes: https://git.openjdk.java.net/jdk/pull/2795/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2795&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8262875 Stats: 9 lines in 8 files changed: 0 ins; 1 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/2795.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2795/head:pull/2795 PR: https://git.openjdk.java.net/jdk/pull/2795
RFR: JDK-8262875: doccheck: empty paragraphs, etc in java.base module
Please review some minor doc fixes, for issues found by _doccheck_.There are two kinds of errors that are addressed. 1. Incorrect use of ``. In HTML, `` marks the *beginning* of a paragraph. It is not a terminator, to mark the end of a paragraph, or a separator to mark the boundary between paragraphs. In particular, it should not be used at the end of a description before a javadoc block tag, such as `@param` or before other HTML block tags, like `` or ``. 2. References to the id `package-description`, following the recent standardization of all ids generated by javadoc, - Commit messages: - JDK-8262875: doccheck: empty paragraphs, etc in java.base module Changes: https://git.openjdk.java.net/jdk/pull/2795/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2795&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8262875 Stats: 9 lines in 8 files changed: 0 ins; 1 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/2795.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2795/head:pull/2795 PR: https://git.openjdk.java.net/jdk/pull/2795
Integrated: JDK-8262433: doclint: reference error in module jdk.incubator.foreign
On Fri, 26 Feb 2021 00:41:18 GMT, Jonathan Gibbons wrote: > Please review a simple fix to ensure that a link reference is resolved and > displayed correctly. This pull request has now been integrated. Changeset: fce57656 Author:Jonathan Gibbons URL: https://git.openjdk.java.net/jdk/commit/fce57656 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8262433: doclint: reference error in module jdk.incubator.foreign Reviewed-by: bpb, lancea - PR: https://git.openjdk.java.net/jdk/pull/2739
RFR: JDK-8262433: doclint: reference error in module jdk.incubator.foreign
Please review a simple fix to ensure that a link reference is resolved and displayed correctly. - Commit messages: - JDK-8262433: doclint: reference error in module jdk.incubator.foreign Changes: https://git.openjdk.java.net/jdk/pull/2739/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2739&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8262433 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2739.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2739/head:pull/2739 PR: https://git.openjdk.java.net/jdk/pull/2739
Integrated: JDK-8262428: doclint warnings in java.xml module
On Thu, 25 Feb 2021 22:41:46 GMT, Jonathan Gibbons wrote: > Please review a small doc fix to remove some superfluous `` tags and an > erroneous `` tag, all reported by doclint.. This pull request has now been integrated. Changeset: 059ede0d Author:Jonathan Gibbons URL: https://git.openjdk.java.net/jdk/commit/059ede0d Stats: 7 lines in 1 file changed: 0 ins; 5 del; 2 mod 8262428: doclint warnings in java.xml module Reviewed-by: bpb, lancea, naoto, iris - PR: https://git.openjdk.java.net/jdk/pull/2733
RFR: JDK-8262428: doclint warnings in java.xml module
Please review a small doc fix to remove some superfluous `` tags and an erroneous `` tag, all reported by doclint.. - Commit messages: - JDK-8262428: doclint warnings in java.xml module Changes: https://git.openjdk.java.net/jdk/pull/2733/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2733&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8262428 Stats: 7 lines in 1 file changed: 0 ins; 5 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2733.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2733/head:pull/2733 PR: https://git.openjdk.java.net/jdk/pull/2733
Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v13]
On Fri, 8 Jan 2021 01:58:07 GMT, Jonathan Gibbons wrote: >> Jan Lahoda has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 57 commits: >> >> - Fixing tests after a merge. >> - Merging master into JDK-8250768 >> - Merging recent master changes into JDK-8250768 >> - Fixing navigator for the PREVIEW page. >> - Fixing typo. >> - Removing obsolette @PreviewFeature. >> - Merging master into JDK-8250768 >> - Removing unnecessary property keys. >> - Cleanup - removing unnecessary code. >> - Merging master into JDK-8250768-dev4 >> - ... and 47 more: >> https://git.openjdk.java.net/jdk/compare/81c06242...a8046dde > > I've looked at all the files that were marked as changed since I last looked > at them. > > There's one suggested enhancement to reduce string bashing between `Utils` > and `ClassWriterImpl` that could be done now or later. > > There's a pending conflict with a PR of mine to change to use a new type > `HtmlId` for HTML ids. This JEP12 work has been in progress for a while, and > so it would be good to get it in before the `HtmlId` work, and I'll deal with > the merge conflict in due course. +1 -- Jon On 1/7/21 12:19 PM, Jan Lahoda wrote: > > I've merged the PR with the recent mainline, and I'd like to integrate > sometime soon. Please let me know if there's any issue with that. Thanks! > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/703*issuecomment-756360085__;Iw!!GqivPVa7Brio!Oxxf6GpefmtstD2fCT8IKF4r-blOVCGWCibSjA4m4l24mI8j6j-RDEJAiBRtFfHCMqNb7Q$>, > > or unsubscribe > <https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AOUXBRTWQAZF7NEGTD3FSGDSYYJOPANCNFSM4STPEYJQ__;!!GqivPVa7Brio!Oxxf6GpefmtstD2fCT8IKF4r-blOVCGWCibSjA4m4l24mI8j6j-RDEJAiBRtFfFTnE3pGg$>. > - PR: https://git.openjdk.java.net/jdk/pull/703
Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v13]
On Thu, 7 Jan 2021 20:23:16 GMT, Jan Lahoda wrote:
>> This is an update to javac and javadoc, to introduce support for Preview
>> APIs, and generally improve javac and javadoc behavior to more closely
>> adhere to JEP 12.
>>
>> The notable changes are:
>>
>> * adding support for Preview APIs (javac until now supported primarily only
>> preview language features, and APIs associated with preview language
>> features). This includes:
>> * the @PreviewFeature annotation has boolean attribute "reflective",
>> which should be true for reflective Preview APIs, false otherwise. This
>> replaces the existing "essentialAPI" attribute with roughly inverted meaning.
>> * the preview warnings for preview APIs are auto-suppressed as
>> described in the JEP 12. E.g. the module that declares the preview API is
>> free to use it without warnings
>> * improving error/warning messages. Please see [1] for a list of
>> cases/examples.
>> * class files are only marked as preview if they are using a preview
>> feature. [1] also shows if a class file is marked as preview or not.
>> * the PreviewFeature annotation has been moved to jdk.internal.javac
>> package (originally was in the jdk.internal package).
>> * Preview API in JDK's javadoc no longer needs to specify @preview tag in
>> the source files. javadoc will auto-generate a note for @PreviewFeature
>> elements, see e.g. [2] and [3] (non-reflective and reflective API,
>> respectively). A summary of preview elements is also provided [4]. Existing
>> uses of @preview have been updated/removed.
>> * non-JDK javadoc is also enhanced to auto-generate preview notes for uses
>> of Preview elements, and for declaring elements using preview language
>> features [5].
>>
>> Please also see the CSR [6] for more information.
>>
>> [1]
>> http://cr.openjdk.java.net/~jlahoda/8250768/JEP12-errors-warnings-v6.html
>> [2]
>> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.base/java/lang/Record.html
>> [3]
>> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.compiler/javax/lang/model/element/RecordComponentElement.html
>> [4]
>> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/preview-list.html
>> [5] http://cr.openjdk.java.net/~jlahoda/8250768/test.javadoc.00/
>> [6] https://bugs.openjdk.java.net/browse/JDK-8250769
>
> Jan Lahoda has updated the pull request with a new target base due to a merge
> or a rebase. The pull request now contains 57 commits:
>
> - Fixing tests after a merge.
> - Merging master into JDK-8250768
> - Merging recent master changes into JDK-8250768
> - Fixing navigator for the PREVIEW page.
> - Fixing typo.
> - Removing obsolette @PreviewFeature.
> - Merging master into JDK-8250768
> - Removing unnecessary property keys.
> - Cleanup - removing unnecessary code.
> - Merging master into JDK-8250768-dev4
> - ... and 47 more:
> https://git.openjdk.java.net/jdk/compare/81c06242...a8046dde
I've looked at all the files that were marked as changed since I last looked at
them.
There's one suggested enhancement to reduce string bashing between `Utils` and
`ClassWriterImpl` that could be done now or later.
There's a pending conflict with a PR of mine to change to use a new type
`HtmlId` for HTML ids. This JEP12 work has been in progress for a while, and so
it would be good to get it in before the `HtmlId` work, and I'll deal with the
merge conflict in due course.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriterImpl.java
line 194:
> 192:
> 193: @Override @SuppressWarnings("preview")
> 194: public void addClassSignature(String modifiers, Content
> classInfoTree) {
It seems less than ideal for this method to take a `String` and to then have to
take it apart and reassemble it. It looks like it should be possible (and
conceptually better) to change the signature to `List` and to make the
corresponding change to `utils.modifiersToString`, probably renaming it as
`utils.modifiersToStrings` or something like that, and dropping the
always-`true` argument for `trailingSpace`.
But, the code is OK as is, and the suggestion is just for an enhancement, so is
OK to defer it, if you would prefer.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SummaryListWriter.java
line 2:
> 1: /*
> 2: * Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights
> reserved.
(minor) now 2021
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SummaryListWriter.java
line 61:
> 59: public abstract class SummaryListWriter
> extends SubWriterHolderWriter {
> 60:
> 61: private String getAnchorName(SummaryElementKind kind) {
Heads-up: at some point this will conflict with another change in
progress/review, to introduce a new type `HtmlId` to use instead of `String`
for ids.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SummaryListWriter.java
Re: RFR: 8248122: java.base should not handle JavaFX application in a specific way
On 10/31/20 8:37 AM, Kartik Ohri wrote: Further investigation reveals that some JavaFX specific code is also present in the `javadoc` tool. For instance, https://github.com/openjdk/jdk/blob/master/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java#L90-L96 https://github.com/openjdk/jdk/blob/master/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java#L347-L353 I think we should remove this code as well and the related tests for it. This would need to be discussed on javadoc-dev (at least), but at this time, there are no plans to remove JavaFX support from the javadoc tool. -- Jon
Re: RFR: 8193031: Collections.addAll is likely to perform worse than Collection.addAll [v2]
On Mon, 21 Dec 2020 14:37:57 GMT, Сергей Цыпанов wrote: >> I don't think so, the code is fine, for me. > > @forax I've sent a mail into compiler-dev list: > http://mail.openjdk.java.net/pipermail/compiler-dev/2020-December/015857.html > > Let's see what they decide. FYI, despite the confusing (historical) name, `syms.trustMeType` is the `SafeVarargs` annotation. See `Symtab.java`, line 579. - PR: https://git.openjdk.java.net/jdk/pull/1764
[jdk16] Integrated: JDK-8247994: Localize javadoc search
On Sun, 13 Dec 2020 00:19:59 GMT, Jonathan Gibbons wrote: > This is for JDK16, as a precursor to fixing JDK-8258002. > > While it is good to be using localized strings in the generated output, the > significance for JDK-8258002 is that the strings are now obtained from a > resource file, and not hardcoded in JavaScript file itself. > > The source file `search.js` is renamed to `search.js.template`, and (unlike > other template files) is copied as-is into the generated image. The values in > the template are resolved when javadoc is executed, depending on the locale > in use at the time. Because of the change in the file's extension, two > makefiles are updated to accommodate the new extension: one is for the > "interim" javadoc used to generate the API docs; the other is for the primary > javadoc in the main JDK image. This pull request has now been integrated. Changeset: 30ca0a5d Author:Jonathan Gibbons URL: https://git.openjdk.java.net/jdk16/commit/30ca0a5d Stats: 120 lines in 9 files changed: 87 ins; 6 del; 27 mod 8247994: Localize javadoc search Reviewed-by: hannesw, ihse - PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search [v4]
> This is for JDK16, as a precursor to fixing JDK-8258002. > > While it is good to be using localized strings in the generated output, the > significance for JDK-8258002 is that the strings are now obtained from a > resource file, and not hardcoded in JavaScript file itself. > > The source file `search.js` is renamed to `search.js.template`, and (unlike > other template files) is copied as-is into the generated image. The values in > the template are resolved when javadoc is executed, depending on the locale > in use at the time. Because of the change in the file's extension, two > makefiles are updated to accommodate the new extension: one is for the > "interim" javadoc used to generate the API docs; the other is for the primary > javadoc in the main JDK image. Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision: Address review comment - Changes: - all: https://git.openjdk.java.net/jdk16/pull/16/files - new: https://git.openjdk.java.net/jdk16/pull/16/files/1a406f35..de6802c0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk16&pr=16&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk16&pr=16&range=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk16/pull/16.diff Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/16/head:pull/16 PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search [v3]
> This is for JDK16, as a precursor to fixing JDK-8258002. > > While it is good to be using localized strings in the generated output, the > significance for JDK-8258002 is that the strings are now obtained from a > resource file, and not hardcoded in JavaScript file itself. > > The source file `search.js` is renamed to `search.js.template`, and (unlike > other template files) is copied as-is into the generated image. The values in > the template are resolved when javadoc is executed, depending on the locale > in use at the time. Because of the change in the file's extension, two > makefiles are updated to accommodate the new extension: one is for the > "interim" javadoc used to generate the API docs; the other is for the primary > javadoc in the main JDK image. Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision: Address review comment - Changes: - all: https://git.openjdk.java.net/jdk16/pull/16/files - new: https://git.openjdk.java.net/jdk16/pull/16/files/ec85..1a406f35 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk16&pr=16&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk16&pr=16&range=01-02 Stats: 3 lines in 2 files changed: 0 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk16/pull/16.diff Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/16/head:pull/16 PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search [v2]
> This is for JDK16, as a precursor to fixing JDK-8258002. > > While it is good to be using localized strings in the generated output, the > significance for JDK-8258002 is that the strings are now obtained from a > resource file, and not hardcoded in JavaScript file itself. > > The source file `search.js` is renamed to `search.js.template`, and (unlike > other template files) is copied as-is into the generated image. The values in > the template are resolved when javadoc is executed, depending on the locale > in use at the time. Because of the change in the file's extension, two > makefiles are updated to accommodate the new extension: one is for the > "interim" javadoc used to generate the API docs; the other is for the primary > javadoc in the main JDK image. Jonathan Gibbons 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 two additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into localize.search.js - JDK-8247994: Localize javadoc search - Changes: - all: https://git.openjdk.java.net/jdk16/pull/16/files - new: https://git.openjdk.java.net/jdk16/pull/16/files/8eb7f27b..ec85 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk16&pr=16&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk16&pr=16&range=00-01 Stats: 1532 lines in 52 files changed: 1207 ins; 121 del; 204 mod Patch: https://git.openjdk.java.net/jdk16/pull/16.diff Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/16/head:pull/16 PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search
On Mon, 14 Dec 2020 09:11:31 GMT, Hannes Wallnöfer wrote: >> This is for JDK16, as a precursor to fixing JDK-8258002. >> >> While it is good to be using localized strings in the generated output, the >> significance for JDK-8258002 is that the strings are now obtained from a >> resource file, and not hardcoded in JavaScript file itself. >> >> The source file `search.js` is renamed to `search.js.template`, and (unlike >> other template files) is copied as-is into the generated image. The values >> in the template are resolved when javadoc is executed, depending on the >> locale in use at the time. Because of the change in the file's extension, >> two makefiles are updated to accommodate the new extension: one is for the >> "interim" javadoc used to generate the API docs; the other is for the >> primary javadoc in the main JDK image. > > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/search.js.template > line 40: > >> 38: var MIN_RESULTS = 3; >> 39: var MAX_RESULTS = 500; >> 40: var UNNAMED = "##REPLACE:doclet.search.unnamed##"; > > `` is not a string that is ever shown to the user, it is what is > used by javadoc to denote the default package (see > `toolkit.util.DocletConstants.DEFAULT_PACKAGE_NAME`). This variable shouldn't > be localized as that would break behaviour for code in the default package > (and show the localized string as package name, instead of no package name). Noted, thanks - PR: https://git.openjdk.java.net/jdk16/pull/16
[jdk16] RFR: JDK-8247994: Localize javadoc search
This is for JDK16, as a precursor to fixing JDK-8258002. While it is good to be using localized strings in the generated output, the significance for JDK-8258002 is that the strings are now obtained from a resource file, and not hardcoded in JavaScript file itself. The source file `search.js` is renamed to `search.js.template`, and (unlike other template files) is copied as-is into the generated image. The values in the template are resolved when javadoc is executed, depending on the locale in use at the time. Because of the change in the file's extension, two makefiles are updated to accommodate the new extension: one is for the "interim" javadoc used to generate the API docs; the other is for the primary javadoc in the main JDK image. - Commit messages: - JDK-8247994: Localize javadoc search Changes: https://git.openjdk.java.net/jdk16/pull/16/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk16&pr=16&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8247994 Stats: 122 lines in 9 files changed: 88 ins; 6 del; 28 mod Patch: https://git.openjdk.java.net/jdk16/pull/16.diff Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/16/head:pull/16 PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search
On Sun, 13 Dec 2020 00:19:59 GMT, Jonathan Gibbons wrote: > This is for JDK16, as a precursor to fixing JDK-8258002. > > While it is good to be using localized strings in the generated output, the > significance for JDK-8258002 is that the strings are now obtained from a > resource file, and not hardcoded in JavaScript file itself. > > The source file `search.js` is renamed to `search.js.template`, and (unlike > other template files) is copied as-is into the generated image. The values in > the template are resolved when javadoc is executed, depending on the locale > in use at the time. Because of the change in the file's extension, two > makefiles are updated to accommodate the new extension: one is for the > "interim" javadoc used to generate the API docs; the other is for the primary > javadoc in the main JDK image. make/CompileInterimLangtools.gmk line 77: > 75: Standard.java, \ > 76: EXTRA_FILES := > $(BUILDTOOLS_OUTPUTDIR)/gensrc/$1.interim/module-info.java, \ > 77: COPY := .gif .png .xml .css .js .js.template .txt > javax.tools.JavaCompilerTool, \ Build-folk: it would be nice if this macro could use `$(jdk.javadoc_COPY)` instead of having to duplicate entries. (Future RFE?) - PR: https://git.openjdk.java.net/jdk16/pull/16
Re: RFR: JDK-8256950: Add record attribute support to symbol generator CreateSymbols [v5]
On Thu, 3 Dec 2020 09:22:18 GMT, Jan Lahoda wrote: >> Adding support for record classes in the historical data for ct.sym. This >> includes a few changes not strictly needed for the change: >> -updating and moving tests into test/langtools, so that it is easier to run >> them. >> -fixing Record attribute reading in javac's ClassReader (used for tests, but >> seems like the proper thing to do anyway). >> -fixing the -Xprint annotation processor to print record component >> annotations. >> >> Changes to jdk.jdeps' classfile library are needed so that the ct.sym >> creation works. > > Jan Lahoda has updated the pull request incrementally with one additional > commit since the last revision: > > Blank lines do not count for the text block indentation, removing them. Marked as reviewed by jjg (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1480
Re: RFR: 6882207: Convert javap to use diamond operator internally
On Mon, 7 Dec 2020 20:30:07 GMT, Andrey Turbanov wrote: > 6882207: Convert javap to use diamond operator internally Nice; thanks - Marked as reviewed by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1677
Re: RFR: 8257450: Start of release updates for JDK 17 [v2]
On Mon, 7 Dec 2020 19:38:41 GMT, Joe Darcy wrote:
>> Start of JDK 17 updates.
>
> Joe Darcy 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:
>
> - Merge branch 'master' into JDK-8257450
> - Update tests.
> - Merge branch 'master' into JDK-8257450
> - Merge branch 'JDK-8257450' of https://github.com/jddarcy/jdk into
> JDK-8257450
> - Merge branch 'JDK-8257450' of https://github.com/jddarcy/jdk into
> JDK-8257450
> - JDK-8257450
> - JDK-8257450
> - JDK-8257450
I've read all the files, and approve all the langtools related ones (i.e. not
hotspot)
As you've noticed elsewhere, there's a pending conflict with Magnus' work to
move files around.
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Source.java line 108:
> 106:
> 107: /**
> 108: * 16, tbd
The "tbd" can presumably be filled in now
test/jdk/java/lang/module/ClassFileVersionsTest.java line 107:
> 105: { 61, 0, Set.of(STATIC, TRANSITIVE) },
> 106:
> 107: { 62, 0, Set.of()}, // JDK 18
This seems unduly repetitive. Could this be dynamically generated, perhaps in a
future release?
test/langtools/tools/javac/preview/classReaderTest/Client.preview.out line 1:
> 1: - compiler.warn.preview.feature.use.classfile: Bar.class, 17
Is this a test can could be automated? (i.e. no manual change per release?)
test/langtools/tools/javac/versions/Versions.java line 26:
> 24: /*
> 25: * @test
> 26: * @bug 4981566 5028634 5094412 6304984 7025786 7025789 8001112 8028545
> 8000961 8030610 8028546 8188870 8173382 8173382 8193290 8205619 8028563
> 8245147 8245586 8257453
long lines are annoying
test/langtools/tools/javac/versions/Versions.java line 297:
> 295:expectedPass(args, List.of("New7.java", "New8.java",
> "New10.java", "New11.java",
> 296: "New14.java", "New15.java"));
> 297: expectedFail(args, List.of("New16.java"));
(minor) looks like bad indentation
-
Marked as reviewed by jjg (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/1531
Re: RFR: 8257733: Move module-specific data from make to respective module [v2]
On Mon, 7 Dec 2020 14:27:45 GMT, Magnus Ihse Bursie wrote: >> A lot (but not all) of the data in make/data is tied to a specific module. >> For instance, the publicsuffixlist is used by java.base, and fontconfig by >> java.desktop. (A few directories, like mainmanifest, is *actually* used by >> make for the whole build.) >> >> These data files should move to the module they belong to. The are, after >> all, "source code" for that module that is "compiler" into resulting >> deliverables, for that module. (But the "source code" language is not Java >> or C, but typically a highly domain specific language or data format, and >> the "compilation" is, often, a specialized transformation.) >> >> This misplacement of the data directory is most visible at code review time. >> When such data is changed, most of the time build-dev (or the new build >> label) is involved, even though this has nothing to do with the build. While >> this is annoying, a worse problem is if the actual team that needs to review >> the patch (i.e., the team owning the module) is missed in the review. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Move to share/data, and move jdwp.spec to java.se I have reviewed all lines in the patch file with or near instances of `jdk.compiler` - Marked as reviewed by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v11]
On Fri, 6 Nov 2020 18:41:15 GMT, Jan Lahoda wrote: >> This is an update to javac and javadoc, to introduce support for Preview >> APIs, and generally improve javac and javadoc behavior to more closely >> adhere to JEP 12. >> >> The notable changes are: >> >> * adding support for Preview APIs (javac until now supported primarily only >> preview language features, and APIs associated with preview language >> features). This includes: >> * the @PreviewFeature annotation has boolean attribute "reflective", >> which should be true for reflective Preview APIs, false otherwise. This >> replaces the existing "essentialAPI" attribute with roughly inverted meaning. >> * the preview warnings for preview APIs are auto-suppressed as >> described in the JEP 12. E.g. the module that declares the preview API is >> free to use it without warnings >> * improving error/warning messages. Please see [1] for a list of >> cases/examples. >> * class files are only marked as preview if they are using a preview >> feature. [1] also shows if a class file is marked as preview or not. >> * the PreviewFeature annotation has been moved to jdk.internal.javac >> package (originally was in the jdk.internal package). >> * Preview API in JDK's javadoc no longer needs to specify @preview tag in >> the source files. javadoc will auto-generate a note for @PreviewFeature >> elements, see e.g. [2] and [3] (non-reflective and reflective API, >> respectively). A summary of preview elements is also provided [4]. Existing >> uses of @preview have been updated/removed. >> * non-JDK javadoc is also enhanced to auto-generate preview notes for uses >> of Preview elements, and for declaring elements using preview language >> features [5]. >> >> Please also see the CSR [6] for more information. >> >> [1] >> http://cr.openjdk.java.net/~jlahoda/8250768/JEP12-errors-warnings-v6.html >> [2] >> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.base/java/lang/Record.html >> [3] >> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.compiler/javax/lang/model/element/RecordComponentElement.html >> [4] >> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/preview-list.html >> [5] http://cr.openjdk.java.net/~jlahoda/8250768/test.javadoc.00/ >> [6] https://bugs.openjdk.java.net/browse/JDK-8250769 > > Jan Lahoda has updated the pull request incrementally with one additional > commit since the last revision: > > Fixing navigator for the PREVIEW page. I have a mild queasiness about this new overloaded use of the word "Summary", since "summary tables" are normally the summary of the contents of a declaration, like fields and methods of a class. That being said, the usage is primarily internal, and I have no overwhelmingly wonderful alternative, and (overloading aside) the term is accurate. So, OK for now. We can change it later if we want to. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java line 2949: > 2947: > 2948: /** > 2949: * Return the set of preview language features used to declare the > given element. "Returns" - Marked as reviewed by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/703
Re: Integrated: 8256066: Tests use deprecated TestNG API that is no longer available in new versions
On Tue, 10 Nov 2020 17:41:28 GMT, Mandy Chung wrote: > @ExpectedExceptions is no longer available in TestNG 7.*. Update the tests > to use @Test(expectedExceptions = "...") instead. Marked as reviewed by jjg (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1145
Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v6]
On Thu, 5 Nov 2020 12:43:03 GMT, Jan Lahoda wrote: >> Thanks @jonathan-gibbons for your comments! I've tried to update the code >> based on them, mostly in >> https://github.com/lahodaj/jdk/commit/743f516c660b577035cdda4510a0bb97937fd9b2 >> and >> https://github.com/lahodaj/jdk/commit/e4b02827998fc2e8f19f983aabfb3d720b03d111 >> >> A big chunk of the update is generalization of the deprecated and preview >> list builders and writers into a "summary" list builder and writer. These >> should also now handle records. For record components, those are a little >> tricky, as (AFAIK) can't currently have deprecation/preview-ness for them >> (and hence there is no good way to test any support for record components in >> these). But the summary build and writer are looking for record components >> and will fail in case a record component is sent into them. > > FWIW, a javadoc generated with the current version of the patch: > http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.01/api/index.html > > And a specdiff comparing it to the javadoc built from the corresponding > master: > http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.specdiff.01/overview-summary.html The page and comments are generally good, but there is a bug that needs to be fixed. After clicking on the PREVIEW link in the top bar to go to the Preview page, the word DEPRECATED is highlighted in the top navbar instead of PREVIEW. -- Jon On 11/5/20 10:13 AM, mlbridge[bot] wrote: > > /Mailing list message from Alex Buckley > <mailto:alex.buck...@oracle.com> on kulla-dev > <mailto:kulla-...@openjdk.java.net>:/ > > On 11/5/2020 4:45 AM, Jan Lahoda wrote: > > FWIW, a javadoc generated with the current version of the patch: > http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.01/api/index.html > > Allow me to draw people's attention to the PREVIEW link in the banner of > the generated javadoc. It shows all the preview APIs in the release on > one page. This is very helpful for understanding the surface area of a > preview feature. > > For example, with Sealed Classes being the only preview feature likely > to target JDK 16, the PREVIEW page shows that the feature's API is > solely about reflection. It's clear that Sealed Classes do not > introduce, say, a java.lang.Sealed class analogous to the > java.lang.Record class introduced by Records in JDK 14/15 (and which > would have appeared on the PREVIEW page had it existed then). > > Alex > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/703*issuecomment-722549484__;Iw!!GqivPVa7Brio!ONrE9EESNKSCZJPGXBa8TTa4Ey9OqEVTynHLfgVOrBRIkb1Cr2iqLALb01M_DlGDeQEPpw$>, > > or unsubscribe > <https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AOUXBRRTA7I6T2AUM3QT7QTSOLTLXANCNFSM4STPEYJQ__;!!GqivPVa7Brio!ONrE9EESNKSCZJPGXBa8TTa4Ey9OqEVTynHLfgVOrBRIkb1Cr2iqLALb01M_DlE4QxPnyw$>. > - PR: https://git.openjdk.java.net/jdk/pull/703
Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v4]
On Tue, 27 Oct 2020 16:09:45 GMT, Jan Lahoda wrote:
>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java
>> line 88:
>>
>>> 86:
>>> 87: @Override
>>> 88: protected Content getDeprecatedOrPreviewLink(Element member) {
>>
>> This name is a side-effect of the `ElementFlag` question. We should either
>> use explicit field and method names, or we should use `ElementFlag` more
>> consistently. This method name works OK for now, but if if ever gets to
>> have another `orFoo` component in the name, the signature should be
>> parameterized with something like `ElementFlag` or its equivalent.
>
> Note this method returns the same link for deprecate or preview - it just was
> named "getDeprecatedLink", and when using it work preview, I renamed it
> "getDeprecatedOrPreviewLink". We may need to think of a better name.
This name is OK for now. Maybe we'll have m ore insight into a better name
if/when we add a third variant ;-)
-
PR: https://git.openjdk.java.net/jdk/pull/703
Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v6]
On Fri, 16 Oct 2020 16:07:41 GMT, Maurizio Cimadamore
wrote:
>> Jan Lahoda has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 46 commits:
>>
>> - Removing trailing whitespace.
>> - Merging master into JDK-8250768.
>> - Updating tests after records are a final feature.
>> - Fixing tests.
>> - Finalizing removal of record preview hooks.
>> - Merging master into JDK-8250768
>> - Reflecting review comments.
>> - Merge branch 'master' into JDK-8250768
>> - Removing unnecessary cast.
>> - Using a more correct way to get URLs.
>> - ... and 36 more:
>> https://git.openjdk.java.net/jdk/compare/d93e3a7d...2e403900
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/util/RawDiagnosticFormatter.java
> line 166:
>
>> 164: s = "compiler.misc.tree.tag." +
>> StringUtils.toLowerCase(((Tag) arg).name());
>> 165: } else if (arg instanceof Source && arg == Source.DEFAULT &&
>> 166:
>> CODES_NEEDING_SOURCE_NORMALIZATION.contains(diag.getCode())) {
>
> Nice trick to keep raw output constant across versions :-)
👍
-
PR: https://git.openjdk.java.net/jdk/pull/703
Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v6]
On Mon, 2 Nov 2020 18:15:09 GMT, Jan Lahoda wrote:
>> This is an update to javac and javadoc, to introduce support for Preview
>> APIs, and generally improve javac and javadoc behavior to more closely
>> adhere to JEP 12.
>>
>> The notable changes are:
>>
>> * adding support for Preview APIs (javac until now supported primarily only
>> preview language features, and APIs associated with preview language
>> features). This includes:
>> * the @PreviewFeature annotation has boolean attribute "reflective",
>> which should be true for reflective Preview APIs, false otherwise. This
>> replaces the existing "essentialAPI" attribute with roughly inverted meaning.
>> * the preview warnings for preview APIs are auto-suppressed as
>> described in the JEP 12. E.g. the module that declares the preview API is
>> free to use it without warnings
>> * improving error/warning messages. Please see [1] for a list of
>> cases/examples.
>> * class files are only marked as preview if they are using a preview
>> feature. [1] also shows if a class file is marked as preview or not.
>> * the PreviewFeature annotation has been moved to jdk.internal.javac
>> package (originally was in the jdk.internal package).
>> * Preview API in JDK's javadoc no longer needs to specify @preview tag in
>> the source files. javadoc will auto-generate a note for @PreviewFeature
>> elements, see e.g. [2] and [3] (non-reflective and reflective API,
>> respectively). A summary of preview elements is also provided [4]. Existing
>> uses of @preview have been updated/removed.
>> * non-JDK javadoc is also enhanced to auto-generate preview notes for uses
>> of Preview elements, and for declaring elements using preview language
>> features [5].
>>
>> Please also see the CSR [6] for more information.
>>
>> [1]
>> http://cr.openjdk.java.net/~jlahoda/8250768/JEP12-errors-warnings-v6.html
>> [2]
>> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.base/java/lang/Record.html
>> [3]
>> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.compiler/javax/lang/model/element/RecordComponentElement.html
>> [4]
>> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/preview-list.html
>> [5] http://cr.openjdk.java.net/~jlahoda/8250768/test.javadoc.00/
>> [6] https://bugs.openjdk.java.net/browse/JDK-8250769
>
> Jan Lahoda has updated the pull request with a new target base due to a merge
> or a rebase. The pull request now contains 46 commits:
>
> - Removing trailing whitespace.
> - Merging master into JDK-8250768.
> - Updating tests after records are a final feature.
> - Fixing tests.
> - Finalizing removal of record preview hooks.
> - Merging master into JDK-8250768
> - Reflecting review comments.
> - Merge branch 'master' into JDK-8250768
> - Removing unnecessary cast.
> - Using a more correct way to get URLs.
> - ... and 36 more:
> https://git.openjdk.java.net/jdk/compare/d93e3a7d...2e403900
I have read all the files.
I have added a n umber of various minor non-blocking comments (no need for
re-review( to fix these. But I have a couple of comments/questions before
finally giving approval.
There's a comment in `PreviewListWriter` about annotation members that needs
too be addressed, and I wonder is RECORD and RECORD_COMPONENT need to be added
into PreviewElementKind.
src/java.base/share/classes/jdk/internal/javac/PreviewFeature.java line 75:
> 73: * A key for testing.
> 74: */
> 75: TEST,
Slightly weird
src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTaskPool.java line
257:
> 255: //when patching modules (esp. java.base), it may be
> impossible to
> 256: //clear the symbols read from the patch path:
> 257: polluted |=
> get(JavaFileManager.class).hasLocation(StandardLocation.PATCH_MODULE_PATH);
OK, but looks unrelated to primary work
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Preview.java line 218:
> 216: return Errors.PreviewFeatureDisabledClassfile(classfile,
> majorVersionToSource.get(majorVersion).name);
> 217: }
> 218:
Up above in isPreview, lines 185-188, I'm supervised it's not a `switch`
statement. (Can't annotate those lines directly)
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java
line 89:
> 87: @Override
> 88: protected Content getDeprecatedOrPreviewLink(Element member) {
> 89: Content content = new ContentBuilder();
Yeah the shorter name is good here and more in keeping with the code style
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java
line 93:
> 91: if (!utils.isConstructor(member)) {
> 92: content.add(".");
> 93: content.add(member.getSimpleName());
this is OK, but generally FYI, `Content` is now set up to allow chained method
calls.
src/jdk.javadoc/share
Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v4]
On Thu, 29 Oct 2020 09:26:05 GMT, Jan Lahoda wrote: >> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ElementsTable.java >> line 1288: >> >>> 1286: case FIELD: case INSTANCE_INIT: case LOCAL_VARIABLE: >>> case PARAMETER: >>> 1287: case RESOURCE_VARIABLE: case STATIC_INIT: case >>> TYPE_PARAMETER: >>> 1288: case RECORD_COMPONENT: >> >> I'm not saying this is wrong, but I'd like to understand why it is necessary. > > HtmlDocletWriter.getPreviewNotes analyzes classes and their members, to see > if some are using aspects that are under preview. When looking at the > members, it uses utils.isIncluded on the member, and that eventually gets > here. And if the member is a RECORD_COMPONENT, it would fail here. But we can > avoid asking for RECORD_COMPONENTS, if needed. ok - PR: https://git.openjdk.java.net/jdk/pull/703
Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v4]
On Thu, 29 Oct 2020 09:43:56 GMT, Jan Lahoda wrote: >> I don't think there should be much interaction with -source . >> We don't support preview features from previous version or preview class >> files from previous versions, so I think it should be enough to handle the >> currently present preview features. > > We don't support preview features from previous releases, AFAIK. So javadoc > for JDK 16 should not accept e.g. record class when running with -source 15. Yeah, my recollection is that I was wondering whether preview-related code needs to be "guarded" to only work in the current release. But, I guess we may get the right effect (of forbidding preview features in older code) from the javac front end, so that in javadoc we can be assured that there are no instances of what may still be preview features in older code (i.e with older --source/--rlease options) - PR: https://git.openjdk.java.net/jdk/pull/703
Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v4]
tion cannot be obtained from public API ...
which may require updating the public API as well. For example, should these
methods be predicates on the Language Model `Elements` class?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java
line 89:
> 87: @Override
> 88: protected Content getDeprecatedOrPreviewLink(Element member) {
> 89: Content deprecatedLinkContent = new ContentBuilder();
name does not match usage. I suggest simplifying it to just "content".
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java
line 88:
> 86:
> 87: @Override
> 88: protected Content getDeprecatedOrPreviewLink(Element member) {
This name is a side-effect of the `ElementFlag` question. We should either use
explicit field and method names, or we should use `ElementFlag` more
consistently. This method name works OK for now, but if if ever gets to have
another `orFoo` component in the name, the signature should be parameterized
with something like `ElementFlag` or its equivalent.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AllClassesIndexWriter.java
line 172:
> 170: description.add(getDeprecatedPhrase(klass));
> 171: List tags =
> utils.getDeprecatedTrees(klass);
> 172: if (!tags.isEmpty()) {
There is potential for future parameterization using `ElementFlag` or its
equivalent.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AnnotationTypeRequiredMemberWriterImpl.java
line 205:
> 203: protected Content getDeprecatedOrPreviewLink(Element member) {
> 204: String name = utils.getFullyQualifiedName(member) + "." +
> member.getSimpleName();
> 205: return
> writer.getDocLink(LinkInfoImpl.Kind.MEMBER_DEPRECATED_PREVIEW, member, name);
I suspect the MEMBER_DEPRECATED_PREVIEW will become more general in future
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriterImpl.java
line 210:
> 208: pre.add(modifiersPart);
> 209: pre.add(new
> HtmlTree(TagName.SUP).add(links.createLink(getPreviewSectionAnchor(typeElement),
> 210:
> contents.previewMark)));
Possible future enhancement: use a set of modifiers that need flags
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriterImpl.java
line 281:
> 279: pre.add(DocletConstants.NL);
> 280: pre.add("permits");
> 281: pre.add(new
> HtmlTree(TagName.SUP).add(links.createLink(getPreviewSectionAnchor(typeElement),
@hns is it better to use `` or CSS? Either way is OK in this review.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDoclet.java
line 187:
> 185: PreviewListWriter.generate(configuration);
> 186: }
> 187:
This may need to be updated, by comparing against similar code for DEPRECATED,
and/or you need to take `HtmlDocletWriter.ConditionalPage` into account, again
by comparing with latest code for deprecated items.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 112:
> 110: import jdk.javadoc.internal.doclets.toolkit.util.VisibleMemberTable;
> 111: import jdk.javadoc.internal.doclets.toolkit.util.Utils;
> 112: import
> jdk.javadoc.internal.doclets.toolkit.util.Utils.DeclarationPreviewLanguageFeatures;
Uuugh on the long class name ;-)
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 1039:
> 1037: } else if (utils.isVariableElement(element) ||
> utils.isTypeElement(element)) {
> 1038: return getLink(new LinkInfoImpl(configuration, context,
> typeElement)
> 1039:
> .label(label).where(links.getName(element.getSimpleName().toString())).targetMember(element));
Note to self (@jonathan-gibbons ) links.getName should accept a `CharSequence`
to avoid the need for `toString()`
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 2219:
> 2217: if (previewTree != null) {
> 2218: div.add(HtmlTree.SPAN(HtmlStyle.previewLabel,
> 2219: new
> RawHtml(utils.getPreviewTreeSummaryOrDetails(previewTree, true;
There's a big cringe here that there is a method in Utils returning HTML.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 2281:
> 2279: RawHtml note2 =
> 2280: new
> RawHtml(resources.getText("doclet.PreviewTrailingNote2",
> 2281:
Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v2]
On Wed, 21 Oct 2020 12:43:51 GMT, Hannes Wallnöfer wrote:
>> Jan Lahoda has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 35 commits:
>>
>> - Merge branch 'JDK-8250768-dev' of https://github.com/lahodaj/jdk into
>> JDK-8250768
>> - More fixing tests.
>> - Fixing tests.
>> - Using unique sections for preview warning sections, as suggested.
>> - Merge branch 'master' into JDK-8250768
>> - Reflecting review comments.
>> - Fixing tests.
>> - Various cleanup.
>> - The Preview taglet is not needed anymore.
>> - There is not jdk.internal package anymore
>> - ... and 25 more:
>> https://git.openjdk.java.net/jdk/compare/98ec4a67...be1d8651
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java
> line 3164:
>
>> 3162: }
>> 3163:
>> 3164: public Set elementFlags(Element el) {
>
> It seems like the sole use of this method and the `ElementFlag` enum below is
> to determine whether a preview warning note should be generated for an
> element. Is there something that speaks against simplifying it to reflect
> that purpose, e.g. change its name to `hasPreviewNote` or `hasPreviewContent`
> and change the return type to `boolean`? Of course that's unless you foresee
> adding more `ElementFlag` values in the future.
There's a number of predicates on an element that the doclet might be
interested in that could be cached/reified as "flags". Today, we have "preview"
and "deprecated" that have similar handling; there have been discussions about
handling native methods in a similar fashion.
-
PR: https://git.openjdk.java.net/jdk/pull/703
Integrated: JDK-8255262: Remove use of legacy custom @spec tag
On Thu, 22 Oct 2020 17:16:23 GMT, Jonathan Gibbons wrote: > The change is (just) to remove legacy usages of a JDK-private custom tag. This pull request has now been integrated. Changeset: 0aa3c925 Author: Jonathan Gibbons URL: https://git.openjdk.java.net/jdk/commit/0aa3c925 Stats: 209 lines in 69 files changed: 0 ins; 209 del; 0 mod 8255262: Remove use of legacy custom @spec tag Reviewed-by: lancea, mr, iris, alanb, darcy, mchung - PR: https://git.openjdk.java.net/jdk/pull/814
RFR: JDK-8255262: Remove use of legacy custom @spec tag
The change is (just) to remove legacy usages of a JDK-private custom tag. - Commit messages: - JDK-8255262: Remove use of legacy custom @spec tag Changes: https://git.openjdk.java.net/jdk/pull/814/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=814&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8255262 Stats: 209 lines in 69 files changed: 0 ins; 209 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/814.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/814/head:pull/814 PR: https://git.openjdk.java.net/jdk/pull/814
Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12
On Fri, 16 Oct 2020 15:20:03 GMT, Jan Lahoda wrote:
> This is an update to javac and javadoc, to introduce support for Preview
> APIs, and generally improve javac and javadoc
> behavior to more closely adhere to JEP 12.
> The notable changes are:
>
> * adding support for Preview APIs (javac until now supported primarily only
> preview language features, and APIs
>associated with preview language features). This includes:
> * the @PreviewFeature annotation has boolean attribute "reflective",
> which should be true for reflective Preview APIs,
>false otherwise. This replaces the existing "essentialAPI" attribute
> with roughly inverted meaning.
> * the preview warnings for preview APIs are auto-suppressed as described
> in the JEP 12. E.g. the module that declares the
>preview API is free to use it without warnings
> * improving error/warning messages. Please see [1] for a list of
> cases/examples.
> * class files are only marked as preview if they are using a preview
> feature. [1] also shows if a class file is marked as
>preview or not.
> * the PreviewFeature annotation has been moved to jdk.internal.javac package
> (originally was in the jdk.internal package).
> * Preview API in JDK's javadoc no longer needs to specify @preview tag in
> the source files. javadoc will auto-generate a
>note for @PreviewFeature elements, see e.g. [2] and [3] (non-reflective
> and reflective API, respectively). A summary of
>preview elements is also provided [4]. Existing uses of @preview have been
> updated/removed.
> * non-JDK javadoc is also enhanced to auto-generate preview notes for uses
> of Preview elements, and for declaring
>elements using preview language features [5].
>
> Please also see the CSR [6] for more information.
>
> [1] http://cr.openjdk.java.net/~jlahoda/8250768/JEP12-errors-warnings-v6.html
> [2]
> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.base/java/lang/Record.html
> [3]
>
> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.compiler/javax/lang/model/element/RecordComponentElement.html
> [4]
> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/preview-list.html
> [5]
> http://cr.openjdk.java.net/~jlahoda/8250768/test.javadoc.00/ [6]
> https://bugs.openjdk.java.net/browse/JDK-8250769
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriterImpl.java
line 209:
> 207: "sealed".equals(modifiersPart) ||
> "non-sealed".equals(modifiersPart)) {
> 208: pre.add(modifiersPart);
> 209: pre.add(new
> HtmlTree(TagName.SUP).add(HtmlTree.A("#preview", contents.previewMark)));
This will likely clash with a public field named "preview". You should choose a
name for the anchor that is not a Java
identifier.
Also, in general, it is better to use the `Links` class if possible to create
links.
-
PR: https://git.openjdk.java.net/jdk/pull/703
Re: RFR: 8216497: javadoc should auto-link to platform classes [v3]
On Wed, 23 Sep 2020 14:20:12 GMT, Hannes Wallnöfer wrote: >> This pull request is identical with the RFR previously sent for the >> Mercurial repository: >> >> https://mail.openjdk.java.net/pipermail/javadoc-dev/2020-August/001796.html >> >> I'm copy-pasting the comments from the original RFR below. >> >> Most of the new code is added to the Extern class where it fits in quite >> nicely and can use the existing supporting >> code for setting up external links. >> The default behaviour is to generate links to docs.oracle.com for released >> versions and download.java.net for >> prereleases. Platform documentation URLs can be configured using the new >> --link-platform-properties option to >> provide a properties file with URLs pointing to to alternative locations. >> See the CSR linked above for more details on >> the new options. The feature can be disabled using the --no-platform-link >> option, generating the same output as >> previously. One problem I had to solve was how to handle the transition >> from prerelease versions to final releases, >> since the location of the documentation changes in this transition. For >> obvious reasons we don’t want to make that >> switch via code change at a time shortly before release. The way it is done >> is that we determine if the current >> javadoc instance is a prerelease version as indicated by the Version >> returned by BaseConfiguration#getDocletVersion(), >> and then check whether the release/source version of the current javadoc >> execution uses the same (latest) version. This >> means that that only the latest version will ever generate prerelease links >> (e.g. running current javadoc 16 with >> source version 15 will generate links to the final release documentation) >> but I think this is acceptable. Another >> issue I spent some time getting right was tests. New releases require a new >> element-list resource*), so tests have to >> pick up new releases. On the other hand, we don’t want hundreds of tests to >> fail when a new release is added, ideally >> there should be one test with a clear message. Because of this, when a >> release is encountered for which no >> element-list is available a warning is generated instead of an error, and >> the documentation is generated without >> platform links as if running with --no-platform-link option. This allows >> most existing tests to pass and just the new >> test to fail with a relatively clear message of what is wrong. >> *) I also thought about generating the element-list for the current release >> at build time. It’s quite involved, and we >> still need to maintain element-lists for older versions, so I’m not sure >> it’s worth it. >> >> For existing tests that check output affected by the new option I added the >> --no-platform-link option to disable the >> feature. Otherwise we’d have to update those tests with each new release (or >> else freeze the tests to use some static >> release or source version, which we don’t want either). I updated the CSR >> to the new code. It also needs to be >> reviewed: https://bugs.openjdk.java.net/browse/JDK-8251181 >> >> Thanks, >> Hannes > > Hannes Wallnöfer has updated the pull request incrementally with one > additional commit since the last revision: > > Rename --no-platform-link option plus minor cleanup I agree with the judgement call to _not_ introduce default javadoc options. It was just a suggestion to consider, and I agree it would make the calls less intuitively obvious, for the short term gain of editing fewer tests here. It also helped to realize that the support default platform links does _not_ involve any network access. FWIW, the precedent in JavadocTester that I was referrng to is `setAutomaticCheckLinks`, `setAutomaticCheckAccessibility`, but those are about default actions after javadoc has been run, and not about default methods. - Marked as reviewed by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/171
Re: RFR: 8216497: javadoc should auto-link to platform classes [v2]
On Mon, 21 Sep 2020 10:47:40 GMT, Hannes Wallnöfer wrote:
>> This pull request is identical with the RFR previously sent for the
>> Mercurial repository:
>>
>> https://mail.openjdk.java.net/pipermail/javadoc-dev/2020-August/001796.html
>>
>> I'm copy-pasting the comments from the original RFR below.
>>
>> Most of the new code is added to the Extern class where it fits in quite
>> nicely and can use the existing supporting
>> code for setting up external links.
>> The default behaviour is to generate links to docs.oracle.com for released
>> versions and download.java.net for
>> prereleases. Platform documentation URLs can be configured using the new
>> --link-platform-properties option to
>> provide a properties file with URLs pointing to to alternative locations.
>> See the CSR linked above for more details on
>> the new options. The feature can be disabled using the --no-platform-link
>> option, generating the same output as
>> previously. One problem I had to solve was how to handle the transition
>> from prerelease versions to final releases,
>> since the location of the documentation changes in this transition. For
>> obvious reasons we don’t want to make that
>> switch via code change at a time shortly before release. The way it is done
>> is that we determine if the current
>> javadoc instance is a prerelease version as indicated by the Version
>> returned by BaseConfiguration#getDocletVersion(),
>> and then check whether the release/source version of the current javadoc
>> execution uses the same (latest) version. This
>> means that that only the latest version will ever generate prerelease links
>> (e.g. running current javadoc 16 with
>> source version 15 will generate links to the final release documentation)
>> but I think this is acceptable. Another
>> issue I spent some time getting right was tests. New releases require a new
>> element-list resource*), so tests have to
>> pick up new releases. On the other hand, we don’t want hundreds of tests to
>> fail when a new release is added, ideally
>> there should be one test with a clear message. Because of this, when a
>> release is encountered for which no
>> element-list is available a warning is generated instead of an error, and
>> the documentation is generated without
>> platform links as if running with --no-platform-link option. This allows
>> most existing tests to pass and just the new
>> test to fail with a relatively clear message of what is wrong.
>> *) I also thought about generating the element-list for the current release
>> at build time. It’s quite involved, and we
>> still need to maintain element-lists for older versions, so I’m not sure
>> it’s worth it.
>>
>> For existing tests that check output affected by the new option I added the
>> --no-platform-link option to disable the
>> feature. Otherwise we’d have to update those tests with each new release (or
>> else freeze the tests to use some static
>> release or source version, which we don’t want either). I updated the CSR
>> to the new code. It also needs to be
>> reviewed: https://bugs.openjdk.java.net/browse/JDK-8251181
>>
>> Thanks,
>> Hannes
>
> Hannes Wallnöfer has updated the pull request incrementally with three
> additional commits since the last revision:
>
> - Merge pull request #1 from lahodaj/JDK-8216497
>
>Automatically generate the elements-list data from the ct.sym for releases
> 11+, including the current release under
>development
> - Generating current release list for javadoc; using hardcoded lists for
> versions < 11
> - Attempting to (mostly) generate the javadoc release manifests from ct.sym
> historical data.
Generally excellent. Some feedback inline.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
line 350:
> 348:
> 349: doclet.usage.excludedocfilessubdir.parameters=\
> 350: :..
3 dots for ellipsis? 2 dots is "parent directory"
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
line 384:
> 382:
> 383: doclet.usage.no-platform-link.description=\
> 384: Do not generate links to platform documentation
Suggest:
Do not generate links to the platform documentation
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java
line 194:
> 192:
> 193: /**
> 194: * Argument for command-line option {@code --no-platform-link}.
minor: would "--no-platform-links" (plural) be a better name for the option?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java
line 435:
> 433: },
> 434:
> 435: new Option(resources, "--no-platform-link") {
Repeating preceding comment: would `--no-platform-links` (plural) be a better
name?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java
line 236:
> 234: * @param linkPlatformProperties path or URL to properties fi
Re: RFR: 8216497: javadoc should auto-link to platform classes [v2]
On Tue, 22 Sep 2020 17:24:19 GMT, Jonathan Gibbons wrote:
>> Hannes Wallnöfer has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Merge pull request #1 from lahodaj/JDK-8216497
>>
>>Automatically generate the elements-list data from the ct.sym for
>> releases 11+, including the current release under
>>development
>> - Generating current release list for javadoc; using hardcoded lists for
>> versions < 11
>> - Attempting to (mostly) generate the javadoc release manifests from ct.sym
>> historical data.
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java
> line 323:
>
>> 321: props.load(inputStream);
>> 322: }
>> 323: url = props.getProperty("doclet.platform.docs." + version);
>
> Similar to other file-or-url arguments: good!
As a possibly-later cleanup, should we have a single utility method somewhere
(in this class) to open a stream on a
file-or-url?
-
PR: https://git.openjdk.java.net/jdk/pull/171
Re: RFR: 8252999: Cleanup: replace .equals("") with .isEmpty() within all codebase
On Thu, 10 Sep 2020 12:04:48 GMT, Dmitriy Dumanskiy wrote: > I have in mind dozens of improvements all over the code like this one. That sounds scary. Broad updates like these cause unnecessary churn in the codebase, and can make merging and back porting harder. Changes should be discussed ahead of time with the appropriate teams. - PR: https://git.openjdk.java.net/jdk/pull/29
Re: Fix for Javadoc errors in java.base
On 8/13/20 10:13 AM, Sean Mullan wrote: On 8/13/20 11:05 AM, Julia Boes wrote: Hi Vipin, Thanks for providing this fix, I'm happy to sponsor your change. To complete the review, we still need someone to green light the remaining changes below. I'm looping in net-dev and security-dev to have a look. Bug: https://bugs.openjdk.java.net/browse/JDK-8251542 Webrev: http://cr.openjdk.java.net/~jboes/webrevs/8251542/webrev/ Once the review is completed, please provide me with a patch that includes a changeset comment as described here: https://openjdk.java.net/guide/producingChangeset.html If you have any questions, let me know. Cheers, Julia --- old/src/java.base/share/classes/com/sun/crypto/provider/DHPrivateKey.java 2020-07-25 23:46:21.233726447 +0530 +++ new/src/java.base/share/classes/com/sun/crypto/provider/DHPrivateKey.java 2020-07-25 23:46:20.721720857 +0530 @@ -96,8 +96,6 @@ * @param p the prime modulus * @param g the base generator * @param l the private-value length - * - * @exception InvalidKeyException if the key cannot be encoded This should actually remain, but it should be ProviderException which is a RuntimeException. See the other DHPrivateKey ctor as that specifies it correctly. --Sean I note the use of `@exception`, as compared to `@throws`, which is more common. Stats: `@exception` 7322 occurrences `@throws` 21173 occurrences That's probably too many `@exception` to clean up. :-( -- Jon
RFR; [docs,15] JDK-8248060 small HTML issues in java.xml package-info.java files
Please review a couple of trivial fixes for a couple of issues in
java.xml, reported by doclint.
I realize we typically don't touch upstream code, but I'm hoping this is
small enough to
be reasonable.
In the first case, there is an unnecessary `` before a ``.
In the second case, there's a bad `{@link newFactory}`. Not only is the
syntax bad,
but the method is defined in multiple classes in the same package, and
overloaded as
well: i.e. there is no easy/reasonable target for the link. The
simple/localized fix is
just to change `{@link}` to `{@code}`. A better solution (for someone
else to do)
would be to rewrite the sentence altogether.
-- Jon
JBS: https://bugs.openjdk.java.net/browse/JDK-8248060
Patch:
hg diff -R open open/src/java.xml
diff -r 9cfa0137612f
src/java.xml/share/classes/javax/xml/stream/package-info.java
--- a/src/java.xml/share/classes/javax/xml/stream/package-info.java Mon
Jun 22 13:37:41 2020 -0700
+++ b/src/java.xml/share/classes/javax/xml/stream/package-info.java Mon
Jun 22 15:44:55 2020 -0700
@@ -41,8 +41,8 @@
*
* StAX supports plugability with {@link XMLInputFactory} and
* {@link XMLOutputFactory} that define how an implementation is
- * located through a process as described in the {@link newFactory}
- * method.
+ * located through a process as described in the {@code newFactory}
+ * methods.
*
*
* @since 1.6
diff -r 9cfa0137612f
src/java.xml/share/classes/org/xml/sax/package-info.java
--- a/src/java.xml/share/classes/org/xml/sax/package-info.java Mon Jun
22 13:37:41 2020 -0700
+++ b/src/java.xml/share/classes/org/xml/sax/package-info.java Mon Jun
22 15:44:55 2020 -0700
@@ -27,7 +27,6 @@
* Provides the interfaces for the Simple API for XML (SAX). Supports both
* the SAX1 and SAX2 APIs.
*
- *
* SAX2 Standard Feature Flags
*
*
RFR: [15,docs] JDK-8247959,doclint errors in NIO code
Please review some fixes to address issues found by doclint.
In a couple of places, imports were required in order to resolve symbols.
In another case, the name also had a typo in it.
In the last case, the incorrect syntax was used for a type parameter in
@param.
-- Jon
JBS: https://bugs.openjdk.java.net/browse/JDK-8247959
Patch inline:
diff -r 086c7f077fc6
src/jdk.nio.mapmode/share/classes/jdk/nio/mapmode/ExtendedMapMode.java
---
a/src/jdk.nio.mapmode/share/classes/jdk/nio/mapmode/ExtendedMapMode.java
Fri Jun 19 15:22:19 2020 -0400
+++
b/src/jdk.nio.mapmode/share/classes/jdk/nio/mapmode/ExtendedMapMode.java
Fri Jun 19 20:24:02 2020 -0700
@@ -25,6 +25,8 @@
package jdk.nio.mapmode;
+import java.nio.MappedByteBuffer;
+import java.nio.channels.FileChannel;
import java.nio.channels.FileChannel.MapMode;
/**
@@ -52,7 +54,7 @@
/**
* File mapping mode for a read-write mapping of a file backed by
- * non-volatile RAM. {@linkplain MappedByteBufefr#force force}
+ * non-volatile RAM. {@linkplain MappedByteBuffer#force force}
* operations on a buffer created with this mode will be performed
* using cache line writeback rather than proceeding via a file
* device flush.
diff -r 086c7f077fc6
src/jdk.sctp/share/classes/com/sun/nio/sctp/NotificationHandler.java
---
a/src/jdk.sctp/share/classes/com/sun/nio/sctp/NotificationHandler.java
Fri Jun 19 15:22:19 2020 -0400
+++
b/src/jdk.sctp/share/classes/com/sun/nio/sctp/NotificationHandler.java
Fri Jun 19 20:24:02 2020 -0700
@@ -45,7 +45,7 @@
* this handler interface as the type for parameters, return type,
etc. rather
* than the abstract class.
*
- * @param T The type of the object attached to the receive operation
+ * @param The type of the object attached to the receive operation
*
* @since 1.7
*/
