Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v5]

[Jonathan Gibbons]

On Fri, 10 Jun 2022 09:34:53 GMT, Magnus Ihse Bursie  wrote:

>> With project Skara, the ability to run a set of sanity build and test jobs 
>> on selected platforms was added. This functionality was driven by 
>> `.github/workflows/submit.yml`. This file unfortunately lacks any real 
>> structure, and contains a lot of code duplication and redundancy. This has 
>> made it hard to add functionality, and new platforms to test, and it has 
>> made it even harder to debug issues. (This is hard enough as it is, since we 
>> have no direct access to the platforms that GHA runs on.)
>> 
>> Since the GHA tests are important for a large subset of the community, we 
>> need to do better. 
>> 
>> ## GitHub Actions framework rewrite
>>  
>> This is a complete overhaul of the GHA testing framework. I started out 
>> trying to just tease the old `submit.yml` apart, trying to de-duplicate 
>> code, but I soon realized a much more thorough rework was needed.
>> 
>> ### Design description
>> 
>> The principle for the new design was to avoid code duplication, and to 
>> improve readability of the code. The latter is extra important since the GHA 
>> "language" is very limited, needs a lot of quirks and workarounds, and is 
>> probably not well known by many OpenJDK developers. I've strived to find 
>> useful layers of abstraction to make the expressions as clear as possible.
>> 
>> Unfortunately, the Workflow/Action YAML language is quite limited. There are 
>> two ways to avoid duplication, "local composite actions" and "callable 
>> workflows". They both have several limitations:
>> 
>>  * "Callable workflows" can only be used in a single redirection. They are 
>> (apparently) inlined into the "calling workflow" at run time, and as such, 
>> they are present without having to check out the source code. (Which is a 
>> lengthy process.)
>> 
>>  * "Local composite actions" can use other actions, but you must start by 
>> checking out the repo.
>> 
>> To use the strength of both kinds of sub-modules, I'm using "callable 
>> workflows" from `main.yml` to call `build-.yml` and `test.yml`. It 
>> is not allowed to mix "strategies" (that is, the method of automatically 
>> creating a test matrix) when calling callable workflows, so I needed to have 
>> some amount of duplication in `main.yml` that could have been avoided 
>> otherwise.
>> 
>> All the callable workflows need to check out the source code anyway, so 
>> there is no real additional cost of using "local composite actions" for 
>> abstraction of these workflows. (A bit of a lucky break.) I've created "high 
>> level" actions, corresponding to something like a function call. The goal 
>> here was both to avoid duplication, and to improve readability of the 
>> workflows.
>> 
>> The four `build-.yml` files are very similar. But in the end of 
>> the day, only like 50% of the source code is shared, and the platform 
>> specific changes permeate the files. So I decided to keep them separately, 
>> since mixing them all into one would have made a mess, due to the lack of 
>> proper abstraction mechanisms. But that also mean that if we change platform 
>> independent code in building, we need to remember to update it in all four 
>> places.
>> 
>> In the strictest sense, this is a "refactoring" in that the functionality 
>> should be equal to the old `submit.yml`. The same platforms should build, 
>> with the same arguments, and the same tests should run. When I look at the 
>> code now, I see lots of potential for improvement here, by rethinking what 
>> we do run. But let's save that discussion for the next PR.
>> 
>> There is one major change, though. Windows is no longer running on Cygwin, 
>> but on MSYS2. This was not really triggered by the recurring build issues on 
>> Cygwin (though that certainly did help me in thinking I made the right 
>> choice), but the sheer impossibility of getting Cygwin to behave as a normal 
>> unix shell on GHA Windows hosts. I spent countless hours trying to work out 
>> limitations, by setting `SHELLOPTS=igncr`, by running `set +x posix` to turn 
>> of the POSIX compliance mode that kept turning on by itself and made bash 
>> choke on several of our scripts, by playing tricks with the `PATH`, but in 
>> the end to no avail. There were no single combination of hacks and 
>> workarounds that could get us past the entire chain from configure, to 
>> build, to testing. (The old solution user PowerShell instead to get around 
>> these limitations.) I'm happy to report that I have had absolutely zero 
>> issues with MSYS2 since I made the switch (and understood how to set the 
>> PATH properly), and I'm seriously c
 onsidering switching stance to recommend using MSYS2 instead of Cygwin as the 
primary winenv for building the JDK.
>> 
>> ### Example run
>> 
>> A good example on how a run looks like with the new GHA system is [the run 
>> for this PR](https://github.com/magicus/jdk/actions/runs/2454577164).
>> 
>> ### New features
>> 
>> 

Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests

[Jonathan Gibbons]

On Tue, 7 Jun 2022 20:05:26 GMT, Jonathan Gibbons  wrote:

>> With project Skara, the ability to run a set of sanity build and test jobs 
>> on selected platforms was added. This functionality was driven by 
>> `.github/workflows/submit.yml`. This file unfortunately lacks any real 
>> structure, and contains a lot of code duplication and redundancy. This has 
>> made it hard to add functionality, and new platforms to test, and it has 
>> made it even harder to debug issues. (This is hard enough as it is, since we 
>> have no direct access to the platforms that GHA runs on.)
>> 
>> Since the GHA tests are important for a large subset of the community, we 
>> need to do better. 
>> 
>> ## GitHub Actions framework rewrite
>>  
>> This is a complete overhaul of the GHA testing framework. I started out 
>> trying to just tease the old `submit.yml` apart, trying to de-duplicate 
>> code, but I soon realized a much more thorough rework was needed.
>> 
>> ### Design description
>> 
>> The principle for the new design was to avoid code duplication, and to 
>> improve readability of the code. The latter is extra important since the GHA 
>> "language" is very limited, needs a lot of quirks and workarounds, and is 
>> probably not well known by many OpenJDK developers. I've strived to find 
>> useful layers of abstraction to make the expressions as clear as possible.
>> 
>> Unfortunately, the Workflow/Action YAML language is quite limited. There are 
>> two ways to avoid duplication, "local composite actions" and "callable 
>> workflows". They both have several limitations:
>> 
>>  * "Callable workflows" can only be used in a single redirection. They are 
>> (apparently) inlined into the "calling workflow" at run time, and as such, 
>> they are present without having to check out the source code. (Which is a 
>> lengthy process.)
>> 
>>  * "Local composite actions" can use other actions, but you must start by 
>> checking out the repo.
>> 
>> To use the strength of both kinds of sub-modules, I'm using "callable 
>> workflows" from `main.yml` to call `build-.yml` and `test.yml`. It 
>> is not allowed to mix "strategies" (that is, the method of automatically 
>> creating a test matrix) when calling callable workflows, so I needed to have 
>> some amount of duplication in `main.yml` that could have been avoided 
>> otherwise.
>> 
>> All the callable workflows need to check out the source code anyway, so 
>> there is no real additional cost of using "local composite actions" for 
>> abstraction of these workflows. (A bit of a lucky break.) I've created "high 
>> level" actions, corresponding to something like a function call. The goal 
>> here was both to avoid duplication, and to improve readability of the 
>> workflows.
>> 
>> The four `build-.yml` files are very similar. But in the end of 
>> the day, only like 50% of the source code is shared, and the platform 
>> specific changes permeate the files. So I decided to keep them separately, 
>> since mixing them all into one would have made a mess, due to the lack of 
>> proper abstraction mechanisms. But that also mean that if we change platform 
>> independent code in building, we need to remember to update it in all four 
>> places.
>> 
>> In the strictest sense, this is a "refactoring" in that the functionality 
>> should be equal to the old `submit.yml`. The same platforms should build, 
>> with the same arguments, and the same tests should run. When I look at the 
>> code now, I see lots of potential for improvement here, by rethinking what 
>> we do run. But let's save that discussion for the next PR.
>> 
>> There is one major change, though. Windows is no longer running on Cygwin, 
>> but on MSYS2. This was not really triggered by the recurring build issues on 
>> Cygwin (though that certainly did help me in thinking I made the right 
>> choice), but the sheer impossibility of getting Cygwin to behave as a normal 
>> unix shell on GHA Windows hosts. I spent countless hours trying to work out 
>> limitations, by setting `SHELLOPTS=igncr`, by running `set +x posix` to turn 
>> of the POSIX compliance mode that kept turning on by itself and made bash 
>> choke on several of our scripts, by playing tricks with the `PATH`, but in 
>> the end to no avail. There were no single combination of hacks and 
>> workarounds that could get us past the entire chain from configure, to 
>> build, to testing. (The old soluti

Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests

[Jonathan Gibbons]

On Tue, 7 Jun 2022 13:05:49 GMT, Magnus Ihse Bursie  wrote:

> With project Skara, the ability to run a set of sanity build and test jobs on 
> selected platforms was added. This functionality was driven by 
> `.github/workflows/submit.yml`. This file unfortunately lacks any real 
> structure, and contains a lot of code duplication and redundancy. This has 
> made it hard to add functionality, and new platforms to test, and it has made 
> it even harder to debug issues. (This is hard enough as it is, since we have 
> no direct access to the platforms that GHA runs on.)
> 
> Since the GHA tests are important for a large subset of the community, we 
> need to do better. 
> 
> ## GitHub Actions framework rewrite
>  
> This is a complete overhaul of the GHA testing framework. I started out 
> trying to just tease the old `submit.yml` apart, trying to de-duplicate code, 
> but I soon realized a much more thorough rework was needed.
> 
> ### Design description
> 
> The principle for the new design was to avoid code duplication, and to 
> improve readability of the code. The latter is extra important since the GHA 
> "language" is very limited, needs a lot of quirks and workarounds, and is 
> probably not well known by many OpenJDK developers. I've strived to find 
> useful layers of abstraction to make the expressions as clear as possible.
> 
> Unfortunately, the Workflow/Action YAML language is quite limited. There are 
> two ways to avoid duplication, "local composite actions" and "callable 
> workflows". They both have several limitations:
> 
>  * "Callable workflows" can only be used in a single redirection. They are 
> (apparently) inlined into the "calling workflow" at run time, and as such, 
> they are present without having to check out the source code. (Which is a 
> lengthy process.)
> 
>  * "Local composite actions" can use other actions, but you must start by 
> checking out the repo.
> 
> To use the strength of both kinds of sub-modules, I'm using "callable 
> workflows" from `main.yml` to call `build-.yml` and `test.yml`. It 
> is not allowed to mix "strategies" (that is, the method of automatically 
> creating a test matrix) when calling callable workflows, so I needed to have 
> some amount of duplication in `main.yml` that could have been avoided 
> otherwise.
> 
> All the callable workflows need to check out the source code anyway, so there 
> is no real additional cost of using "local composite actions" for abstraction 
> of these workflows. (A bit of a lucky break.) I've created "high level" 
> actions, corresponding to something like a function call. The goal here was 
> both to avoid duplication, and to improve readability of the workflows.
> 
> The four `build-.yml` files are very similar. But in the end of the 
> day, only like 50% of the source code is shared, and the platform specific 
> changes permeate the files. So I decided to keep them separately, since 
> mixing them all into one would have made a mess, due to the lack of proper 
> abstraction mechanisms. But that also mean that if we change platform 
> independent code in building, we need to remember to update it in all four 
> places.
> 
> In the strictest sense, this is a "refactoring" in that the functionality 
> should be equal to the old `submit.yml`. The same platforms should build, 
> with the same arguments, and the same tests should run. When I look at the 
> code now, I see lots of potential for improvement here, by rethinking what we 
> do run. But let's save that discussion for the next PR.
> 
> There is one major change, though. Windows is no longer running on Cygwin, 
> but on MSYS2. This was not really triggered by the recurring build issues on 
> Cygwin (though that certainly did help me in thinking I made the right 
> choice), but the sheer impossibility of getting Cygwin to behave as a normal 
> unix shell on GHA Windows hosts. I spent countless hours trying to work out 
> limitations, by setting `SHELLOPTS=igncr`, by running `set +x posix` to turn 
> of the POSIX compliance mode that kept turning on by itself and made bash 
> choke on several of our scripts, by playing tricks with the `PATH`, but in 
> the end to no avail. There were no single combination of hacks and 
> workarounds that could get us past the entire chain from configure, to build, 
> to testing. (The old solution user PowerShell instead to get around these 
> limitations.) I'm happy to report that I have had absolutely zero issues with 
> MSYS2 since I made the switch (and understood how to set the PATH properly), 
> and I'm seriously co
 nsidering switching stance to recommend using MSYS2 instead of Cygwin as the 
primary winenv for building the JDK.
> 
> ### Example run
> 
> A good example on how a run looks like with the new GHA system is [the run 
> for this PR](https://github.com/magicus/jdk/actions/runs/2454577164).
> 
> ### New features
> 
> While the primary focus was to convert the old system to a new framework, 
> more accommodating 

Integrated: JDK-8252717: Integrate/merge legacy standard doclet diagnostics and doclint

[Jonathan Gibbons]

On Thu, 2 Jun 2022 20:59:26 GMT, Jonathan Gibbons  wrote:

> Please review a moderate change to enable (most) doclet warnings even if 
> doclint is enabled.
> 
> Since the introduction of doclint, there was some (small) overlap between the 
> small set of warnings generated by the doclet and the new larger set of 
> diagnostics that could be generated by doclint.  The solution, until now, has 
> been to disable doclet warnings when doclint is enabled. But, neither set 
> contains the other, and the policy has inappropriately suppressed some 
> warnings and inhibited writing new code to generate new warnings by the 
> doclet that could only be done by the doclet, and not doclint.
> 
> One notable group of warnings that has been inappropriately suppressed is the 
> warnings generated by using the `-serial warn` option.
> 
> The fundamental core of the change is to remove the conditional checks in the 
> doclet `Messages.java`, which would suppress messages when doclint was 
> enabled.  A consequence is that some specific messages have to disabled if 
> they are duplicate checks if doclint is enabled. (The messages cannot be 
> removed altogether because doclint might _not_ be enabled.)   A test is added 
> to verify that either one message or the other is generated, but never both.
> 
> As previously noted, an issue with the earlier policy is that warnings 
> generated by using the `-serial warn` option were inappropriately suppressed, 
> and this change fixes that ... revealing a number of latent warnings in the 
> JDK API that need to be addressed.  The short term fix here is to temporarily 
> remove the use of the `-serialwarn` option. See 
> [JDK-8287749](https://bugs.openjdk.java.net/browse/JDK-8287749).

This pull request has now been integrated.

Changeset: 59e9700c
Author:Jonathan Gibbons 
URL:   
https://git.openjdk.java.net/jdk/commit/59e9700c4e0ae892f15607bcaa267e5868eb0512
Stats: 267 lines in 11 files changed: 228 ins; 21 del; 18 mod

8252717: Integrate/merge legacy standard doclet diagnostics and doclint

Reviewed-by: erikj, prappo

-

PR: https://git.openjdk.java.net/jdk/pull/9006

Re: RFR: JDK-8252717: Integrate/merge legacy standard doclet diagnostics and doclint [v2]

[Jonathan Gibbons]

On Thu, 2 Jun 2022 23:33:13 GMT, Pavel Rappo  wrote:

>> Jonathan Gibbons has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   fix whitespace
>
> test/langtools/jdk/javadoc/doclet/testDoclintDocletMessages/TestDocLintDocletMessages.java
>  line 190:
> 
>> 188: checkExit(expect.exit);
>> 189: 
>> 190: checkOutput(Output.OUT, true,expect.message);
> 
> Add whitespace after `true,`

✅

-

PR: https://git.openjdk.java.net/jdk/pull/9006

Re: RFR: JDK-8252717: Integrate/merge legacy standard doclet diagnostics and doclint [v2]

[Jonathan Gibbons]

> Please review a moderate change to enable (most) doclet warnings even if 
> doclint is enabled.
> 
> Since the introduction of doclint, there was some (small) overlap between the 
> small set of warnings generated by the doclet and the new larger set of 
> diagnostics that could be generated by doclint.  The solution, until now, has 
> been to disable doclet warnings when doclint is enabled. But, neither set 
> contains the other, and the policy has inappropriately suppressed some 
> warnings and inhibited writing new code to generate new warnings by the 
> doclet that could only be done by the doclet, and not doclint.
> 
> One notable group of warnings that has been inappropriately suppressed is the 
> warnings generated by using the `-serial warn` option.
> 
> The fundamental core of the change is to remove the conditional checks in the 
> doclet `Messages.java`, which would suppress messages when doclint was 
> enabled.  A consequence is that some specific messages have to disabled if 
> they are duplicate checks if doclint is enabled. (The messages cannot be 
> removed altogether because doclint might _not_ be enabled.)   A test is added 
> to verify that either one message or the other is generated, but never both.
> 
> As previously noted, an issue with the earlier policy is that warnings 
> generated by using the `-serial warn` option were inappropriately suppressed, 
> and this change fixes that ... revealing a number of latent warnings in the 
> JDK API that need to be addressed.  The short term fix here is to temporarily 
> remove the use of the `-serialwarn` option. See 
> [JDK-8287749](https://bugs.openjdk.java.net/browse/JDK-8287749).

Jonathan Gibbons has updated the pull request incrementally with one additional 
commit since the last revision:

  fix whitespace

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/9006/files
  - new: https://git.openjdk.java.net/jdk/pull/9006/files/2f4bc7fb..55b5576f

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=9006=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9006=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/9006.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/9006/head:pull/9006

PR: https://git.openjdk.java.net/jdk/pull/9006

Re: RFR: JDK-8252717: Integrate/merge legacy standard doclet diagnostics and doclint

[Jonathan Gibbons]

On Fri, 3 Jun 2022 15:59:04 GMT, Pavel Rappo  wrote:

> > I did a visual inspection over the Checker class.
> 
> > The test is set up to be a framework to accommodate new cases if any should 
> > arise.
> 
> This is good. It would be even better if we had a mechanism to reduce risk of 
> diagnostic duplication in the future. I don't have any concrete proposal, but 
> unique internal error codes shared between doclint and doclet come to mind.

I would also like to have a policy to align the user-facing content of the 
diagnostic message. This includes the severity (warning or error) as well as 
the position and message text, including substituted values.

-

PR: https://git.openjdk.java.net/jdk/pull/9006

Re: RFR: JDK-8252717: Integrate/merge legacy standard doclet diagnostics and doclint

[Jonathan Gibbons]

On Fri, 3 Jun 2022 07:48:15 GMT, Pavel Rappo  wrote:

> > Since the introduction of doclint, there was some (small) overlap between 
> > the small set ... and the new larger set ... But, the sets do not overlap
> 
> Do or do they not overlap? :)

Ooops. Wrong word. I was meaning that neither set contains the other.

> 
> > the policy has inappropriately suppressed some warnings and inhibited the 
> > generation of new warnings
> 
> What's the difference between "suppressed" and "inhibited" here?

I was using "suppressed" with respect to existing doclet warnings, which have 
not been displayed if doclint was enabled, and was using "inhibited" with 
respect to writing code to generate new warnings, meaning that there was not 
much benefit to adding new warnings if they were typically going to be 
suppressed when doclint was enabled.


> 
> Generally, how can we be sure that we haven't missed any double-reported 
> cases?

I did a visual inspection over the Checker class. Generally, Checker does more 
in the area of "missing" comments and HTML issues, with less focus on 
"reference" issues, while the doclet does not report missing comments and does 
not even attempt to detect HTML issues. The overlap is "syntax" issues and 
"reference" issues, which are covered here.

The test is set up to be a framework to accommodate new cases if any should 
arise.

-

PR: https://git.openjdk.java.net/jdk/pull/9006

RFR: JDK-8252717: Integrate/merge legacy standard doclet diagnostics and doclint

[Jonathan Gibbons]

Please review a moderate change to enable (most) doclet warnings even if 
doclint is enabled.

Since the introduction of doclint, there was some (small) overlap between the 
small set of warnings generated by the doclet and the new larger set of 
diagnostics that could be generated by doclint.  The solution, until now, has 
been to disable doclet warnings when doclint is enabled. But, the sets do not 
overlap, and the policy has inappropriately suppressed some warnings and 
inhibited the generation of new warnings by the doclet that could only be done 
by the doclet, and not doclint.

One notable group of warnings that has been inappropriately suppressed is the 
warnings generated by using the `-serial warn` option.

The fundamental core of the change is to remove the conditional checks in the 
doclet `Messages.java`, which would suppress messages when doclint was enabled. 
 A consequence is that some specific messages have to disabled if they are 
duplicate checks if doclint is enabled. (The messages cannot be removed 
altogether because doclint might _not_ be enabled.)   A test is added to verify 
that either one message or the other is generated, but never both.

As previously noted, an issue with the earlier policy is that warnings 
generated by using the `-serial warn` option were inappropriately suppressed, 
and this change fixes that ... revealing a number of latent warnings in the JDK 
API that need to be addressed.  The short term fix here is to temporarily 
remove the use of the `-serialwarn` option. See 
[JDK-8287749](https://bugs.openjdk.java.net/browse/JDK-8287749).

-

Commit messages:
 - JDK-8252717: Integrate/merge legacy standard doclet diagnostics and doclint

Changes: https://git.openjdk.java.net/jdk/pull/9006/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9006=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8252717
  Stats: 267 lines in 11 files changed: 228 ins; 21 del; 18 mod
  Patch: https://git.openjdk.java.net/jdk/pull/9006.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/9006/head:pull/9006

PR: https://git.openjdk.java.net/jdk/pull/9006

Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v3]

[Jonathan Gibbons]

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: 8284209: Replace remaining usages of 'a the' in source code

[Jonathan Gibbons]

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

Re: RFR: 8257733: Move module-specific data from make to respective module [v13]

[Jonathan Gibbons]

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: 8278275: Initial nroff manpage generation for JDK 19

[Jonathan Gibbons]

On Mon, 13 Dec 2021 05:51:37 GMT, David Holmes  wrote:

> Trivial update to change the version to 19-ea, and update the single 
> reference to the "current release".
> 
> Content changes for 19 will follow.
> 
> Thanks,
> David

Marked as reviewed by jjg (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/6811

Re: RFR: JDK-8272945: Use snippets in java.compiler documentation [v2]

[Jonathan Gibbons]

> Please review a patch to use snippets in the `java.compiler` documentation, 
> instead of a mix of raw HTML and/or `{@code ...}`.  The change is just about 
> the presentation of the code fragments; there are no changes to the normative 
> specifications for the module.
> 
> One of the examples went to extraordinary lengths to include the character 
> sequence `*/` within the example. That example has been replaced by an 
> external snippet in a separate source file, which does not have any 
> restriction on the use of `*/`. However, it does require that the file be 
> excluded from standard compilation, and that is done by setting `EXCLUDES`, 
> once for the "interim" compiler, and once again for the "product" compiler.   
>  Going forward, a better solution might be to modify the 
> `SetupJavaCompilation` macro to ignore all directories whose name is not a 
> valid Java identifier (or, if easier, contains a `-`, such as `doc-files` or 
> `snippet-files`.)

Jonathan Gibbons has updated the pull request incrementally with one additional 
commit since the last revision:

  Address review comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6686/files
  - new: https://git.openjdk.java.net/jdk/pull/6686/files/97cedc9a..27467bf2

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=6686=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6686=00-01

  Stats: 3 lines in 2 files changed: 0 ins; 1 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6686.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6686/head:pull/6686

PR: https://git.openjdk.java.net/jdk/pull/6686

Re: RFR: JDK-8272945: Use snippets in java.compiler documentation

[Jonathan Gibbons]

On Fri, 3 Dec 2021 00:40:03 GMT, Magnus Ihse Bursie  wrote:

>> Please review a patch to use snippets in the `java.compiler` documentation, 
>> instead of a mix of raw HTML and/or `{@code ...}`.  The change is just about 
>> the presentation of the code fragments; there are no changes to the 
>> normative specifications for the module.
>> 
>> One of the examples went to extraordinary lengths to include the character 
>> sequence `*/` within the example. That example has been replaced by an 
>> external snippet in a separate source file, which does not have any 
>> restriction on the use of `*/`. However, it does require that the file be 
>> excluded from standard compilation, and that is done by setting `EXCLUDES`, 
>> once for the "interim" compiler, and once again for the "product" compiler.  
>>   Going forward, a better solution might be to modify the 
>> `SetupJavaCompilation` macro to ignore all directories whose name is not a 
>> valid Java identifier (or, if easier, contains a `-`, such as `doc-files` or 
>> `snippet-files`.)
>
> src/java.compiler/share/classes/javax/tools/snippet-files/JavaSourceFromString.java
>  line 29:
> 
>> 27: return code;
>> 28: }
>> 29: }
> 
> Missing newline..?

Fixed

-

PR: https://git.openjdk.java.net/jdk/pull/6686

Re: RFR: JDK-8272945: Use snippets in java.compiler documentation

[Jonathan Gibbons]

On Fri, 3 Dec 2021 13:47:05 GMT, Erik Joelsson  wrote:

>> make/modules/java.compiler/Java.gmk line 30:
>> 
>>> 28: 
>>> 29: EXCLUDES += \
>>> 30: javax/tools/snippet-files \
>> 
>> You can put this just on a single line :-). 
>> 
>> And I'm frankly not sure if make is happy about having a trailing backslash 
>> but no additional line...
>
> As long as the next line is empty, it works, but it's not a good idea. That's 
> why we came up with the terminating # in our 1 element per line lists. In 
> this case, I would prefer a single line assignment without any backslashes.

Fixed to single line

-

PR: https://git.openjdk.java.net/jdk/pull/6686

Re: RFR: JDK-8278175: Enable all doclint warnings for build of java.desktop

[Jonathan Gibbons]

On Fri, 3 Dec 2021 01:18:20 GMT, Joe Darcy  wrote:

> In JDK 18, JDK-8189591 added the ability to suppress doclint warnings. 
> Therefore, it is now possible to enable the full doclint checks for the 
> java.desktop module if the instances of warnings are suppressed. This patch 
> does this; it would be preferable to address the doc warnings directly, but 
> that is beyond what I'm attempting to do here.

Looks OK to me, although I don't consider myself a Reviewer for this code.

-

PR: https://git.openjdk.java.net/jdk/pull/6687

RFR: JDK-8272945: Use snippets in java.compiler documentation

[Jonathan Gibbons]

Please review a patch to use snippets in the `java.compiler` documentation, 
instead of a mix of raw HTML and/or `{@code ...}`.  The change is just about 
the presentation of the code fragments; there are no changes to the normative 
specifications for the module.

One of the examples went to extraordinary lengths to include the character 
sequence `*/` within the example. That example has been replaced by an external 
snippet in a separate source file, which does not have any restriction on the 
use of `*/`. However, it does require that the file be excluded from standard 
compilation, and that is done by setting `EXCLUDES`, once for the "interim" 
compiler, and once again for the "product" compiler.Going forward, a better 
solution might be to modify the `SetupJavaCompilation` macro to ignore all 
directories whose name is not a valid Java identifier (or, if easier, contains 
a `-`, such as `doc-files` or `snippet-files`.)

-

Commit messages:
 - JDK-8272945: Use snippets in java.compiler documentation

Changes: https://git.openjdk.java.net/jdk/pull/6686/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6686=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8272945
  Stats: 123 lines in 7 files changed: 51 ins; 25 del; 47 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6686.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6686/head:pull/6686

PR: https://git.openjdk.java.net/jdk/pull/6686

Re: RFR: 8277847: Support toolGuide tag in class-level documentation

[Jonathan Gibbons]

On Thu, 25 Nov 2021 15:58:58 GMT, Julia Boes  wrote:

> This change adds support for the `@toolGuide` tag in class-level API 
> documentation. 
> 
> (A use case is the jwebserver tool, where the 
> com.sun.net.httpserver.SimpleFileServer class provides API points for 
> extension and customization of the underlying server and its components. 
> Linking to the tool's man page in the class-level documentation would be 
> beneficial.)

Marked as reviewed by jjg (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/6566

Re: RFR: 8275745: Reproducible copyright headers [v3]

[Jonathan Gibbons]

Emmanual,

If you're interested, please contact the javadoc-...@openjdk.java.net 
list regarding the javadoc fix. While the diff looks promising, it would 
require a corresponding regression test.


-- Jon

On 11/19/21 6:24 AM, Emmanuel Bourg wrote:

On Thu, 11 Nov 2021 15:37:09 GMT, Emmanuel Bourg  wrote:

[snip]

https://salsa.debian.org/openjdk-team/openjdk/-/blob/openjdk-17/debian/patches/reproducible-javadoc-timestamp.diff

[snip]


-

PR: https://git.openjdk.java.net/jdk/pull/1498

Re: RFR: 8276654: element-list order is non deterministic [v3]

[Jonathan Gibbons]

On Fri, 5 Nov 2021 19:47:04 GMT, Andrew Leonard  wrote:

>> Fixes: https://bugs.openjdk.java.net/browse/JDK-8276654
>> 
>> A intermittent problem with the make dependencies means the jdk.javadoc 
>> element-list-.txt generation can remove the src defined 
>> element|package-list-<7,8,9,10>.txt files.
>> Recreatable by using --with-jobs=1 causing jdk.javadoc "gendata" to always 
>> occur after "java" module build dependency.
>> 
>> Signed-off-by: Andrew Leonard 
>
> Andrew Leonard has refreshed the contents of this pull request, and previous 
> commits have been removed. The incremental views will show differences 
> compared to the previous content of the PR.

Can someone please verify that the contents of these files after the fix is the 
same as the contents of the files for "good" builds before the fix?

-

PR: https://git.openjdk.java.net/jdk/pull/6278

Re: Running jdk's tests to produce coverage report

[Jonathan Gibbons]

I'll update the jtreg build script, and then you'll be able to build 
jtreg yourself, and get the jcov from there.


-- Jon


On 11/3/21 9:55 AM, Chaliasos, Stefanos wrote:

Do you know where I can download this version?

In GitHub there are releases until jcov3.0-b07

Stefanos

From: Alan Bateman 
Sent: 03 November 2021 16:39
To: Chaliasos, Stefanos ; build-dev@openjdk.java.net 

Subject: Re: Running jdk's tests to produce coverage report


This email from alan.bate...@oracle.com originates from outside Imperial. Do not 
click on links and attachments unless you recognise the sender. If you trust the 
sender, add them to your safe senders 
list to disable email stamping 
for this address.




On 03/11/2021 16:34, Chaliasos, Stefanos wrote:
Thanks Alan,

I used the one that jtreg uses. The complete configuration of JCOV for jtreg is:

```
DEFAULT_JCOV_SRC_TAG=jcov3.0-b07
DEFAULT_JCOV_SRC_ARCHIVE_CHECKSUM=c5c26085750628d58de275b3f50a7409300c0497

DEFAULT_ANT_VERSION=1.10.8
DEFAULT_ANT_ARCHIVE_CHECKSUM=dbe187ce2963f9df8a67de8aaff3b0a437d06978

DEFAULT_ASM_VERSION=8.0
DEFAULT_ASM_JAR_CHECKSUM=d1a17d07c60e9e82c8b31b1d8f9ca98726418db4
DEFAULT_ASM_TREE_JAR_CHECKSUM=7b31ca94da9f57334a5aed79b40f2b88c5ee9f4f
DEFAULT_ASM_UTIL_JAR_CHECKSUM=b21996293fd49851ed9017cfde3191e49f77fbd0

DEFAULT_JTHARNESS_SRC_TAG=jt6.0-b13
DEFAULT_JTHARNESS_SRC_ARCHIVE_CHECKSUM=43936b2616476fcac8ee4bd0132e73c015119337
```

Could you please share from where I can download the JCOV version that you are 
referring to?

Here's the JBS issue that upgraded jcov to support v62 class files. So I think 
you need jcov 3.0.9

https://bugs.openjdk.java.net/browse/CODETOOLS-7902988

-Alan

Re: RFR: JDK-8275406: Add copy-to-clipboard feature to snippet UI [v2]

[Jonathan Gibbons]

On Wed, 20 Oct 2021 13:46:43 GMT, Hannes Wallnöfer  wrote:

>> Please review a change to add a copy-to-clipboard feature to snippets. I 
>> took special care to make the feature usable on mobile devices. Sample 
>> output can be viewed and tested here:
>> 
>> http://cr.openjdk.java.net/~hannesw/8275406/api.00/java.base/java/lang/Throwable.html#printStackTrace()
>
> Hannes Wallnöfer has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update copyright years

Marked as reviewed by jjg (Reviewer).

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java
 line 396:

> 394: .put(HtmlAttr.DATA_COPIED, copiedText));
> 395: HtmlTree pre = new HtmlTree(TagName.PRE)
> 396: .setStyle(HtmlStyle.snippet);

This should be updated (later) to follow the HTML recommendations.

-

PR: https://git.openjdk.java.net/jdk/pull/6011

Re: RFR: JDK-8275406: Add copy-to-clipboard feature to snippet UI [v2]

[Jonathan Gibbons]

On Fri, 22 Oct 2021 09:06:49 GMT, Pavel Rappo  wrote:

>> I've traced the execution of a local jtreg run with a debugger and examined 
>> the respective `jtr` file. Here is what I saw. The locale that the javadoc 
>> under test uses is that of returned by java.util.Locale.getDefault(). This 
>> happens because neither of the three pieces -- jtreg, JavadocTester, or 
>> TestSnippetTag -- passes `-locale` to javadoc.
>> 
>> Now I begin to wonder: how come we don't see failures on CI builds? Are we 
>> lucky and all the machines on which javadoc tests run use variants of 
>> English locale? Do we not have other tests where we hard-code English string 
>> variants? Is there something that I don't see?
>
> You don't have to do anything about locales in this PR; this part is fine as 
> it is. I filed a bug, JDK-8275727, to address this issue at mass. It looks 
> like almost half of javadoc tests have a similar issue.

I'm not very worried about the locale issue (JDK-8275727) since it is primarily 
just a problem that affects us ... i.e. it is at most a build/CI/test issue, 
not an end-user  issue.

-

PR: https://git.openjdk.java.net/jdk/pull/6011

Re: RFR: JDK-8275406: Add copy-to-clipboard feature to snippet UI [v2]

[Jonathan Gibbons]

On Wed, 20 Oct 2021 14:41:43 GMT, Pavel Rappo  wrote:

>> I think this should be ok for the general case. Linking to "#" (i.e. empty 
>> fragment) is common practice for links that trigger script functions.
>
> What I mean is that the amended test will now pass for a URL which has an 
> empty fragment but is not a "button".

I think this change is OK for now. We can make it more restrictive later if it 
becomes a problem.  I tried to come up with a short explanatory comment, but 
couldn't find one I was satisfied with.

-

PR: https://git.openjdk.java.net/jdk/pull/6011

Re: RFR: JDK-8275406: Add copy-to-clipboard feature to snippet UI [v2]

[Jonathan Gibbons]

On Wed, 27 Oct 2021 16:59:57 GMT, Jonathan Gibbons  wrote:

>> What I mean is that the amended test will now pass for a URL which has an 
>> empty fragment but is not a "button".
>
> I think this change is OK for now. We can make it more restrictive later if 
> it becomes a problem.  I tried to come up with a short explanatory comment, 
> but couldn't find one I was satisfied with.

While maybe uncommon and maybe not necessary, it is not bad HTML (is it?) to 
have an empty fragment.

-

PR: https://git.openjdk.java.net/jdk/pull/6011

Re: RFR: 8275512: Upgrade required version of jtreg to 6.1 [v2]

[Jonathan Gibbons]

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

Integrated: JDK-8272667: substandard error messages from the docs build

[Jonathan Gibbons]

On Wed, 18 Aug 2021 22:06:13 GMT, Jonathan Gibbons  wrote:

> Please review a small (delete one character) change to improve the error 
> messages reported when bad HTML is detected when post-processing the output 
> from pandoc to generate the docs.
> 
> The change is just to pass the filename as an argument to the command, 
> instead of just piping the input to stdin. As a result, the name of any file 
> containing bad input is reported in the message, instead of the message 
> simply referring to ``

This pull request has now been integrated.

Changeset: 6d3d4795
Author:Jonathan Gibbons 
URL:   
https://git.openjdk.java.net/jdk/commit/6d3d47957ef03c90ed3b1cb7a48902366cd1bc27
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8272667: substandard error messages from the docs build

Reviewed-by: darcy, iris

-

PR: https://git.openjdk.java.net/jdk/pull/5175

RFR: JDK-8272667: substandard error messages from the docs build

[Jonathan Gibbons]

Please review a small (delete one character) change to improve the error 
messages reported when bad HTML is detected when post-processing the output 
from pandoc to generate the docs.

The change is just to pass the filename as an argument to the command, instead 
of just piping the input to stdin. As a result, the name of any file containing 
bad input is reported in the message, instead of the message simply referring 
to ``

-

Commit messages:
 - JDK-8272667: substandard error messages from the docs build

Changes: https://git.openjdk.java.net/jdk/pull/5175/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5175=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8272667
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5175.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5175/head:pull/5175

PR: https://git.openjdk.java.net/jdk/pull/5175

Integrated: JDK-8272374: doclint should report missing "body" comments

[Jonathan Gibbons]

On Fri, 13 Aug 2021 03:51:23 GMT, Jonathan Gibbons  wrote:

> Please review a relatively simple update to have doclnt check for empty 
> "descriptions" -- the body of a doc comment, before the block tags.
> 
> It is already the case that doclint checks for missing/empty descriptions in 
> block tags, like @param, @return, etc.
> 
> There are three cases to consider:
> 
> * The comment itself is missing: this was already checked and reported as 
> "missing comment".
> * The comment is present, but is empty ... this seems relatively unlikely, 
> but is nevertheless checked and reported as "empty comment".
> * The comment is present but only has block tags. This is not always a 
> problem, since the description may be inherited, for example, in an 
> overriding method, but when it is an issue, it is reported as "no initial 
> description". 
> 
> No diagnostic is reported if the description is missing but the first tag is 
> `@deprecated`, because in this case, javadoc will use the body of the 
> `@deprecated` tag for the summary. See 
> [`Character.UnicodeBlock#SURROGATES_AREA`](https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Character.UnicodeBlock.html#SURROGATES_AREA)
>  and the corresponding entry in the summary table to see an example of this 
> situation.
> 
> Diagnostics are reported if the declaration is not an overriding method and 
> does not begin with `@deprecated`.  This occurs in a number of places in the 
> `java.desktop` module, often where the doc comment is of the form `/** 
> @return _description_ */`.  To suppress those warnings for now, the 
> `-missing` option is added to `DOCLINT_OPTIONS` for the `java.desktop` 
> module.  To see the effects of this anti-pattern, look at the empty 
> descriptions for 
> [`javax.swing.text.html.parser.AttributeList`](https://docs.oracle.com/en/java/javase/15/docs/api/java.desktop/javax/swing/text/html/parser/AttributeList.html#method.summary)
> 
> Many of the doclint tests needed to be updated, because of their 
> over-simplistic minimal comments. It was not possible, in general, to avoid 
> updating the source code while preserving line numbers, so in many cases, the 
> golden `*.out` files had to be updated as well.
> 
> A new test is added, focussing on the different forms of empty/missing 
> descriptions, as described above.

This pull request has now been integrated.

Changeset: ae45592d
Author:Jonathan Gibbons 
URL:   
https://git.openjdk.java.net/jdk/commit/ae45592d3304f50aa9e8e114416a41e7899fe37b
Stats: 280 lines in 37 files changed: 151 ins; 10 del; 119 mod

8272374: doclint should report missing "body" comments

Reviewed-by: kcr, hannesw

-

PR: https://git.openjdk.java.net/jdk/pull/5106

Re: RFR: JDK-8272374: doclint should report missing "body" comments [v2]

[Jonathan Gibbons]

> Please review a relatively simple update to have doclnt check for empty 
> "descriptions" -- the body of a doc comment, before the block tags.
> 
> It is already the case that doclint checks for missing/empty descriptions in 
> block tags, like @param, @return, etc.
> 
> There are three cases to consider:
> 
> * The comment itself is missing: this was already checked and reported as 
> "missing comment".
> * The comment is present, but is empty ... this seems relatively unlikely, 
> but is nevertheless checked and reported as "empty comment".
> * The comment is present but only has block tags. This is not always a 
> problem, since the description may be inherited, for example, in an 
> overriding method, but when it is an issue, it is reported as "no initial 
> description". 
> 
> No diagnostic is reported if the description is missing but the first tag is 
> `@deprecated`, because in this case, javadoc will use the body of the 
> `@deprecated` tag for the summary. See 
> [`Character.UnicodeBlock#SURROGATES_AREA`](https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Character.UnicodeBlock.html#SURROGATES_AREA)
>  and the corresponding entry in the summary table to see an example of this 
> situation.
> 
> Diagnostics are reported if the declaration is not an overriding method and 
> does not begin with `@deprecated`.  This occurs in a number of places in the 
> `java.desktop` module, often where the doc comment is of the form `/** 
> @return _description_ */`.  To suppress those warnings for now, the 
> `-missing` option is added to `DOCLINT_OPTIONS` for the `java.desktop` 
> module.  To see the effects of this anti-pattern, look at the empty 
> descriptions for 
> [`javax.swing.text.html.parser.AttributeList`](https://docs.oracle.com/en/java/javase/15/docs/api/java.desktop/javax/swing/text/html/parser/AttributeList.html#method.summary)
> 
> Many of the doclint tests needed to be updated, because of their 
> over-simplistic minimal comments. It was not possible, in general, to avoid 
> updating the source code while preserving line numbers, so in many cases, the 
> golden `*.out` files had to be updated as well.
> 
> A new test is added, focussing on the different forms of empty/missing 
> descriptions, as described above.

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/jdk/pull/5106/files
  - new: https://git.openjdk.java.net/jdk/pull/5106/files/60c6f569..6c875688

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5106=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5106=00-01

  Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5106.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5106/head:pull/5106

PR: https://git.openjdk.java.net/jdk/pull/5106

Re: RFR: JDK-8272374: doclint should report missing "body" comments

[Jonathan Gibbons]

On Mon, 16 Aug 2021 17:08:13 GMT, Jonathan Gibbons  wrote:

>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java line 
>> 203:
>> 
>>> 201: // Don't report an empty description if the 
>>> comment begins with @deprecated,
>>> 202: // because javadoc will use the content of that 
>>> tag in summary tables.
>>> 203: if (firstTag.getKind() != DocTree.Kind.DEPRECATED) 
>>> {
>> 
>> Why do we only check the first block tag here?
>
> Various reasons, 
> 1. It seems the convention is to simply prefix an existing comment with 
> `@deprecated` or to replace the existing body description with `@deprecated 
> reason-why-deprecated`
> 2. This is only for the case when there is no body description; it seems hard 
> to imagine that someone would start a comment with `@see` `@param` `@return` 
> etc and then have the `@deprecated` tag.
> 
> That being said, it would be easy enough to change the code to check for any 
> instance of `@deprecated`.

Given that the downstream code does not impose any ordering restrictions on the 
tags, it is probably with doing the same here.

-

PR: https://git.openjdk.java.net/jdk/pull/5106

Re: RFR: JDK-8272374: doclint should report missing "body" comments

[Jonathan Gibbons]

On Fri, 13 Aug 2021 09:21:40 GMT, Hannes Wallnöfer  wrote:

>> Please review a relatively simple update to have doclnt check for empty 
>> "descriptions" -- the body of a doc comment, before the block tags.
>> 
>> It is already the case that doclint checks for missing/empty descriptions in 
>> block tags, like @param, @return, etc.
>> 
>> There are three cases to consider:
>> 
>> * The comment itself is missing: this was already checked and reported as 
>> "missing comment".
>> * The comment is present, but is empty ... this seems relatively unlikely, 
>> but is nevertheless checked and reported as "empty comment".
>> * The comment is present but only has block tags. This is not always a 
>> problem, since the description may be inherited, for example, in an 
>> overriding method, but when it is an issue, it is reported as "no initial 
>> description". 
>> 
>> No diagnostic is reported if the description is missing but the first tag is 
>> `@deprecated`, because in this case, javadoc will use the body of the 
>> `@deprecated` tag for the summary. See 
>> [`Character.UnicodeBlock#SURROGATES_AREA`](https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Character.UnicodeBlock.html#SURROGATES_AREA)
>>  and the corresponding entry in the summary table to see an example of this 
>> situation.
>> 
>> Diagnostics are reported if the declaration is not an overriding method and 
>> does not begin with `@deprecated`.  This occurs in a number of places in the 
>> `java.desktop` module, often where the doc comment is of the form `/** 
>> @return _description_ */`.  To suppress those warnings for now, the 
>> `-missing` option is added to `DOCLINT_OPTIONS` for the `java.desktop` 
>> module.  To see the effects of this anti-pattern, look at the empty 
>> descriptions for 
>> [`javax.swing.text.html.parser.AttributeList`](https://docs.oracle.com/en/java/javase/15/docs/api/java.desktop/javax/swing/text/html/parser/AttributeList.html#method.summary)
>> 
>> Many of the doclint tests needed to be updated, because of their 
>> over-simplistic minimal comments. It was not possible, in general, to avoid 
>> updating the source code while preserving line numbers, so in many cases, 
>> the golden `*.out` files had to be updated as well.
>> 
>> A new test is added, focussing on the different forms of empty/missing 
>> descriptions, as described above.
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java line 
> 204:
> 
>> 202: // because javadoc will use the content of that tag 
>> in summary tables.
>> 203: if (firstTag.getKind() != DocTree.Kind.DEPRECATED) {
>> 204: env.messages.report(MISSING, Kind.WARNING, 
>> tree, "dc.empty.description");
> 
> Is there a reason to not use `reportMissing` here?

It doesn't have the right signature. `reportMissing` reports a missing comment 
on an element; here, we're reporting a missing description within a `DocTree`.  
There's a similar occurrence at line 1214.

-

PR: https://git.openjdk.java.net/jdk/pull/5106

Re: RFR: JDK-8272374: doclint should report missing "body" comments

[Jonathan Gibbons]

On Fri, 13 Aug 2021 09:20:19 GMT, Hannes Wallnöfer  wrote:

>> Please review a relatively simple update to have doclnt check for empty 
>> "descriptions" -- the body of a doc comment, before the block tags.
>> 
>> It is already the case that doclint checks for missing/empty descriptions in 
>> block tags, like @param, @return, etc.
>> 
>> There are three cases to consider:
>> 
>> * The comment itself is missing: this was already checked and reported as 
>> "missing comment".
>> * The comment is present, but is empty ... this seems relatively unlikely, 
>> but is nevertheless checked and reported as "empty comment".
>> * The comment is present but only has block tags. This is not always a 
>> problem, since the description may be inherited, for example, in an 
>> overriding method, but when it is an issue, it is reported as "no initial 
>> description". 
>> 
>> No diagnostic is reported if the description is missing but the first tag is 
>> `@deprecated`, because in this case, javadoc will use the body of the 
>> `@deprecated` tag for the summary. See 
>> [`Character.UnicodeBlock#SURROGATES_AREA`](https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Character.UnicodeBlock.html#SURROGATES_AREA)
>>  and the corresponding entry in the summary table to see an example of this 
>> situation.
>> 
>> Diagnostics are reported if the declaration is not an overriding method and 
>> does not begin with `@deprecated`.  This occurs in a number of places in the 
>> `java.desktop` module, often where the doc comment is of the form `/** 
>> @return _description_ */`.  To suppress those warnings for now, the 
>> `-missing` option is added to `DOCLINT_OPTIONS` for the `java.desktop` 
>> module.  To see the effects of this anti-pattern, look at the empty 
>> descriptions for 
>> [`javax.swing.text.html.parser.AttributeList`](https://docs.oracle.com/en/java/javase/15/docs/api/java.desktop/javax/swing/text/html/parser/AttributeList.html#method.summary)
>> 
>> Many of the doclint tests needed to be updated, because of their 
>> over-simplistic minimal comments. It was not possible, in general, to avoid 
>> updating the source code while preserving line numbers, so in many cases, 
>> the golden `*.out` files had to be updated as well.
>> 
>> A new test is added, focussing on the different forms of empty/missing 
>> descriptions, as described above.
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java line 
> 203:
> 
>> 201: // Don't report an empty description if the comment 
>> begins with @deprecated,
>> 202: // because javadoc will use the content of that tag 
>> in summary tables.
>> 203: if (firstTag.getKind() != DocTree.Kind.DEPRECATED) {
> 
> Why do we only check the first block tag here?

Various reasons, 
1. It seems the convention is to simply prefix an existing comment with 
`@deprecated` or to replace the existing body description with `@deprecated 
reason-why-deprecated`
2. This is only for the case when there is no body description; it seems hard 
to imagine that someone would start a comment with `@see` `@param` `@return` 
etc and then have the `@deprecated` tag.

That being said, it would be easy enough to change the code to check for any 
instance of `@deprecated`.

-

PR: https://git.openjdk.java.net/jdk/pull/5106

RFR: JDK-8272374: doclint should report missing "body" comments

[Jonathan Gibbons]

Please review a relatively simple update to have doclnt check for empty 
"descriptions" -- the body of a doc comment, before the block tags.

It is already the case that doclint checks for missing/empty descriptions in 
block tags, like @param, @return, etc.

There are three cases to consider:

* The comment itself is missing: this was already checked and reported as 
"missing comment".
* The comment is present, but is empty ... this seems relatively unlikely, but 
is nevertheless checked and reported as "empty comment".
* The comment is present but only has block tags. This is not always a problem, 
since the description may be inherited, for example, in an overriding method, 
but when it is an issue, it is reported as "no initial description". 

No diagnostic is reported if the description is missing but the first tag is 
`@deprecated`, because in this case, javadoc will use the body of the 
`@deprecated` tag for the summary. See 
[`Character.UnicodeBlock#SURROGATES_AREA`](https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Character.UnicodeBlock.html#SURROGATES_AREA)
 and the corresponding entry in the summary table to see an example of this 
situation.

Diagnostics are reported if the declaration is not an overriding method and 
does not begin with `@deprecated`.  This occurs in a number of places in the 
`java.desktop` module, often where the doc comment is of the form `/** @return 
_description_ */`.  To suppress those warnings for now, the `-missing` option 
is added to `DOCLINT_OPTIONS` for the `java.desktop` module.  To see the 
effects of this anti-pattern, look at the empty descriptions for 
[`javax.swing.text.html.parser.AttributeList`](https://docs.oracle.com/en/java/javase/15/docs/api/java.desktop/javax/swing/text/html/parser/AttributeList.html#method.summary)

Many of the doclint tests needed to be updated, because of their 
over-simplistic minimal comments. It was not possible, in general, to avoid 
updating the source code while preserving line numbers, so in many cases, the 
golden `*.out` files had to be updated as well.

A new test is added, focussing on the different forms of empty/missing 
descriptions, as described above.

-

Commit messages:
 - JDK-8272374: doclint should report missing "body" comments

Changes: https://git.openjdk.java.net/jdk/pull/5106/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5106=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8272374
  Stats: 281 lines in 37 files changed: 152 ins; 10 del; 119 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5106.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5106/head:pull/5106

PR: https://git.openjdk.java.net/jdk/pull/5106

Re: [jdk17] RFR: 8259848: Interim javadoc build does not support platform links

[Jonathan Gibbons]

On Wed, 7 Jul 2021 19:02:04 GMT, Erik Joelsson  wrote:

> This patch adds copying of element-list files generated for jdk.javadoc to 
> the interim langtools version of jdk.javadoc. I'm also refactoring the 
> jdk.javadoc/Gendata.gmk file a bit to better adhere to current build infra 
> standards as this was missed in the original review of this file. Rebuilding 
> should now work better with the various clean targets and any problems with 
> the generation tool will be captured in separate log files, along with the 
> command lines.
> 
> @hns Can you verify that this solves the problem you reported?

Generally, looks good.

Thanks for the general cleanup to prevailing standards.

The work looks OK, but I don't consider myself a build-Reviewer.

Note to self: it might be interesting to figure out exactly how we might 
routinely test this feature.

make/modules/jdk.javadoc/Gendata.gmk line 86:

> 84:   11 \
> 85:   )
> 86: # Generate element-list file for the current JDK version

Is this indentation as intended?

-

Marked as reviewed by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk17/pull/227

Re: RFR: 8268768: idea.sh has been updated in surprising and incompatible ways

[Jonathan Gibbons]

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

[Jonathan Gibbons]

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

Re: RFR: 8267634: Update --release 17 symbol information for JDK 17 build 26

[Jonathan Gibbons]

On Fri, 11 Jun 2021 22:44:03 GMT, Joe Darcy  wrote:

> Update symbol files in JDK 18 for changes in JDK 17 b26.

Marked as reviewed by jjg (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/4477

Re: RFR: JDK-8263468: New page for "recent" new API [v4]

[Jonathan Gibbons]

On Mon, 7 Jun 2021 19:48:39 GMT, Hannes Wallnöfer  wrote:

>> This adds a new kind of summary list for new API added in specific releases, 
>> and adds information to the deprecated API list about elements that were 
>> deprecated in the given releases.
>> 
>> The changes to the code are relatively minor thanks to the existing 
>> infrastructure for summary list pages, which was extended by adding the 
>> `getTableCaption` and `addTableTabs` methods to `SummaryListWriter.java` in 
>> order to generate tabbed tables. 
>> 
>> One important area that needs to be reviewed is the addition of resources in 
>> `standard.properties`. A relatively big share of discussion and effort went 
>> into shaping the UI messages.
>> 
>> The build system change adds options to generate API changes for all 
>> releases after JDK 11, with "New API since JDK 11" as page title for the new 
>> API page. I uploaded the generated documentation here:
>> 
>> http://cr.openjdk.java.net/~hannesw/8263468/api-pr.00/new-list.html
>> http://cr.openjdk.java.net/~hannesw/8263468/api-pr.00/deprecated-list.html
>
> Hannes Wallnöfer has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   JDK-8263468: review comments

One future suggestion.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
 line 113:

> 111: deprecated elements, regardless of the releases in which they were 
> deprecated. \
> 112: Each of the other tabs "Deprecated in ..." indicates the elements 
> deprecated \
> 113: in a specific release.)

This is OK for now, but a noreg-doc follow-up might be to improve the 
punctuation in this text, possibly after consulting with docs folk.

It's the use of the quoted strings that seems weird, without any commas or 
parentheses too set them off. On the other hand, I realize we sometimes go for 
brevity in places like this.

-

Marked as reviewed by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4209

Re: RFR: 8268299: jvms tag produces incorrect URL

[Jonathan Gibbons]

On Sun, 6 Jun 2021 22:03:46 GMT, Joe Darcy  wrote:

> The @jls and @jvms taglet share most of their functionality.  A  JLS URL 
> looks like
> 
> https://docs.oracle.com/javase/specs/jls/se16/html/**jls**-8.html#jls-8.1
> 
> and a JVMS URL looks like
> 
> 
> https://docs.oracle.com/javase/specs/jvms/se16/html/**jvms**-4.html#jvms-4.3.2
> 
> The current taglet incorrectly uses "jls" in from the chapter for both JLS 
> and JVMS URLs. The patch corrects this to use "jvms" for JVMS URLs.

Marked as reviewed by jjg (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/4381

Re: RFR: JDK-8263468: New page for "recent" new API [v3]

[Jonathan Gibbons]

On Mon, 7 Jun 2021 15:46:45 GMT, Jonathan Gibbons  wrote:

>> Hannes Wallnöfer has updated the pull request with a new target base due to 
>> a merge or a rebase. The pull request now contains 16 commits:
>> 
>>  - Merge branch 'master' into JDK-8263468
>>  - JDK-8263468: automate build integration
>>  - JDK-8263468: make constant static
>>  - JDK-8263468: Remove unused DocPaths methods
>>  - JDK-8263468: Cleanup
>>  - JDK-8263468: Add tests
>>  - JDK-8263468: Update to new Table methods
>>  - Merge branch 'master' into JDK-8263468
>>
>># Conflicts:
>># 
>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/stylesheet.css
>>  - JDK-8263468: Fix tests
>>  - JDK-8263468: Update to latest CSR
>>  - ... and 6 more: 
>> https://git.openjdk.java.net/jdk/compare/3396b69f...3b13ae32
>
> test/langtools/jdk/javadoc/doclet/testNewApiList/mdl/module-info.java line 30:
> 
>> 28: module mdl {
>> 29: exports pkg;
>> 30: }
> 
> final newline

For all these "sample API" source files, they are OK, but another time, 
consider the use of possibly-custom builders, to generate these files 
dynamically. and to make for more concise reading.

-

PR: https://git.openjdk.java.net/jdk/pull/4209

Re: RFR: JDK-8263468: New page for "recent" new API [v3]

[Jonathan Gibbons]

On Mon, 7 Jun 2021 15:00:43 GMT, Hannes Wallnöfer  wrote:

>> This adds a new kind of summary list for new API added in specific releases, 
>> and adds information to the deprecated API list about elements that were 
>> deprecated in the given releases.
>> 
>> The changes to the code are relatively minor thanks to the existing 
>> infrastructure for summary list pages, which was extended by adding the 
>> `getTableCaption` and `addTableTabs` methods to `SummaryListWriter.java` in 
>> order to generate tabbed tables. 
>> 
>> One important area that needs to be reviewed is the addition of resources in 
>> `standard.properties`. A relatively big share of discussion and effort went 
>> into shaping the UI messages.
>> 
>> The build system change adds options to generate API changes for all 
>> releases after JDK 11, with "New API since JDK 11" as page title for the new 
>> API page. I uploaded the generated documentation here:
>> 
>> http://cr.openjdk.java.net/~hannesw/8263468/api-pr.00/new-list.html
>> http://cr.openjdk.java.net/~hannesw/8263468/api-pr.00/deprecated-list.html
>
> Hannes Wallnöfer has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 16 commits:
> 
>  - Merge branch 'master' into JDK-8263468
>  - JDK-8263468: automate build integration
>  - JDK-8263468: make constant static
>  - JDK-8263468: Remove unused DocPaths methods
>  - JDK-8263468: Cleanup
>  - JDK-8263468: Add tests
>  - JDK-8263468: Update to new Table methods
>  - Merge branch 'master' into JDK-8263468
>
># Conflicts:
>#  
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/stylesheet.css
>  - JDK-8263468: Fix tests
>  - JDK-8263468: Update to latest CSR
>  - ... and 6 more: 
> https://git.openjdk.java.net/jdk/compare/3396b69f...3b13ae32

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/NewAPIListWriter.java
 line 49:

> 47: /**
> 48:  * Generate File to list all the new API elements with the
> 49:  * appropriate links.

(minor) not standard form of comment, but it's "only" an internal class, so 
could be fixed up later

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/NewAPIListWriter.java
 line 72:

> 70: /**
> 71:  * Get list of all the new elements.
> 72:  * Then instantiate NewAPIListWriter and generate File.

Comment. Looks like it may have been copied from elsewhere, I guess

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java
 line 726:

> 724:  */
> 725: deprecatedInReleasePage,
> 726: 

Note to self ... affects new "Output Generated " document

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/DocPaths.java
 line 145:

> 143: public static final DocPath PACKAGE_USE = 
> DocPath.create("package-use.html");
> 144: 
> 145: /** The name of the fie for preview elements. */

typo: "fie"

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/DocPaths.java
 line 148:

> 146: public static final DocPath PREVIEW_LIST = 
> DocPath.create("preview-list.html");
> 147: 
> 148: /** The name of the fie for new elements. */

typo "fie"

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java
 line 1526:

> 1524: }
> 1525: 
> 1526: // Returns the Deprecated annotation element value of the given 
> element, or null.

Use `/**...*/`

test/langtools/jdk/javadoc/doclet/testNewApiList/mdl/module-info.java line 30:

> 28: module mdl {
> 29: exports pkg;
> 30: }

final newline

-

PR: https://git.openjdk.java.net/jdk/pull/4209

Re: RFR: JDK-8263468: New page for "recent" new API [v2]

[Jonathan Gibbons]

On Fri, 28 May 2021 08:19:33 GMT, Hannes Wallnöfer  wrote:

>> This adds a new kind of summary list for new API added in specific releases, 
>> and adds information to the deprecated API list about elements that were 
>> deprecated in the given releases.
>> 
>> The changes to the code are relatively minor thanks to the existing 
>> infrastructure for summary list pages, which was extended by adding the 
>> `getTableCaption` and `addTableTabs` methods to `SummaryListWriter.java` in 
>> order to generate tabbed tables. 
>> 
>> One important area that needs to be reviewed is the addition of resources in 
>> `standard.properties`. A relatively big share of discussion and effort went 
>> into shaping the UI messages.
>> 
>> The build system change adds options to generate API changes for all 
>> releases after JDK 11, with "New API since JDK 11" as page title for the new 
>> API page. I uploaded the generated documentation here:
>> 
>> http://cr.openjdk.java.net/~hannesw/8263468/api-pr.00/new-list.html
>> http://cr.openjdk.java.net/~hannesw/8263468/api-pr.00/deprecated-list.html
>
> Hannes Wallnöfer has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   JDK-8263468: automate build integration

Some minor suggestions for your consideration

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
 line 112:

> 110: doclet.Deprecated_Tabs_Intro=(The leftmost tab "Deprecated ..." 
> indicates all the \
> 111: deprecated elements, regardless of the releases in which they were 
> deprecated. \
> 112: Each of the righthand tabs "Deprecated in ..." indicates the 
> elements deprecated \

"Each of the righthand" doesn't read well.   Would "Each of the other" be 
better?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
 line 119:

> 117: doclet.New_Label=New
> 118: doclet.New_Tabs_Intro=(The leftmost tab "New ..." indicates all the new 
> elements, \
> 119: regardless of the releases in which they were added. Each of the 
> righthand \

ditto

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
 line 268:

> 266: doclet.help.new.body=\
> 267: The {0} page lists APIs that have been added in recent releases. \
> 268: The content of this page is based on information provided by Javadoc 
> @since tags.

Either change to "JavaDoc" or (preferably?) just delete this word, or even the 
sentence.

Is there any interaction with `-nosince`? Should there be?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java
 line 337:

> 335:  */
> 336: public int getSourceVersionNumber() {
> 337: return configuration.docEnv.getSourceVersion().ordinal();

As a general comment, I believe Joe does not encourage use of `ordinal`

-

PR: https://git.openjdk.java.net/jdk/pull/4209

Re: RFR: 8268267: Remove -Djavatest.security.noSecurityManager=true from jtreg runs

[Jonathan Gibbons]

On Fri, 4 Jun 2021 15:53:42 GMT, Weijun Wang  wrote:

> Now that the default behavior of JDK 17 is still 
> `-Djava.security.manager=allow`, we can remove the 
> `-Djavatest.security.noSecurityManager=true` option from the jtreg command 
> line inside `RunTests.gmk`. Three problem-listed langtools tests can also be 
> liberated.

Marked as reviewed by jjg (Reviewer).

test/langtools/ProblemList.txt line 51:

> 49: tools/javac/warnings/suppress/TypeAnnotations.java
>   8057683generic-allimprove ordering of errors with type 
> annotations
> 50: tools/javac/modules/SourceInSymlinkTest.java  
>   8180263windows-allfails when run on a subst drive
> 51: tools/javac/processing/options/XprintRepeatingAnnotations.java
>   8265611generic-all@compile/ref comparison fails when 
> noSecurityManager=true

Maybe close out JDK-8265611?

-

PR: https://git.openjdk.java.net/jdk/pull/4364

Integrated: JDK-8268185: Update GitHub Actions for jtreg 6

[Jonathan Gibbons]

On Thu, 3 Jun 2021 19:05:35 GMT, Jonathan Gibbons  wrote:

> Please review changes to the GitHub Actions for the repo, to use jtreg 6.
> 
> There are three small parts to the change:
> 
> 1. The version info is updated in `make/conf/test-dependencies`
> 2. The new `make/build.sh` script is used to build `jtreg`, instead of the 
> earlier `build-all.sh` script
> 3. The format of the tag in the jtreg repo is changed to follow OpenJDK 
> conventions.
> 
> Tested implicitly with GitHub Actions on my fork, seen here:
> https://github.com/jonathan-gibbons/jdk/actions/runs/903574304

This pull request has now been integrated.

Changeset: e27c4d46
Author:Jonathan Gibbons 
URL:   
https://git.openjdk.java.net/jdk/commit/e27c4d463d920994b79b8163f063ad74f6ee5d59
Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod

8268185: Update GitHub Actions for jtreg 6

Reviewed-by: erikj

-

PR: https://git.openjdk.java.net/jdk/pull/4343

RFR: JDK-8268185: Update GitHub Actions for jtreg 6

[Jonathan Gibbons]

Please review changes to the GitHub Actions for the repo, to use jtreg 6.

There are three small parts to the change:

1. The version info is updated in `make/conf/test-dependencies`
2. The new `make/build.sh` script is used to build `jtreg`, instead of the 
earlier `build-all.sh` script
3. The format of the tag in the jtreg repo is changed to follow OpenJDK 
conventions.

Tested implicitly with GitHub Actions on my fork, seen here:
https://github.com/jonathan-gibbons/jdk/actions/runs/903574304

-

Commit messages:
 - use bash for build.sh
 - JDK-8268185: Update GitHub Actions for jtreg 6

Changes: https://git.openjdk.java.net/jdk/pull/4343/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4343=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8268185
  Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4343.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4343/head:pull/4343

PR: https://git.openjdk.java.net/jdk/pull/4343

Re: RFR: JDK-8266254: Update to use jtreg 6 [v2]

[Jonathan Gibbons]

> 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=4315=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4315=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]

[Jonathan Gibbons]

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

[Jonathan Gibbons]

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=4315=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]

[Jonathan Gibbons]

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]

[Jonathan Gibbons]

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]

[Jonathan Gibbons]

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

Integrated: JDK-8265483: All-caps “JAVA” in the top navigation bar

[Jonathan Gibbons]

On Tue, 20 Apr 2021 22:36:28 GMT, Jonathan Gibbons  wrote:

> Please review a moderately simple change to `make/Docs.gmk` to move the link 
> for "Other Versions" from a "hidden" link in the top nav bar to an explicit 
> link in the "bottom" text. The link is placed to appear after the sentence 
> beginning "For more information..." and before all the legal text (i.e. 
> trademark, copyright, license, etc)
> 
> A side effect of moving the link is that the top text reverts to its intended 
> appearance of "Java ...", without all-caps.
> 
> As before, the presence of the link is optional, and depends on the specific 
> docs target. It is set up to just appear for the main JDK/JavaSE 
> documentation, and not the other docs bundles, such as docs-reference.

This pull request has now been integrated.

Changeset: 3de0dcba
Author:Jonathan Gibbons 
URL:   https://git.openjdk.java.net/jdk/commit/3de0dcba
Stats: 16 lines in 1 file changed: 6 ins; 6 del; 4 mod

8265483: All-caps “JAVA” in the top navigation bar

Reviewed-by: iris, erikj

-

PR: https://git.openjdk.java.net/jdk/pull/3594

RFR: JDK-8265483: All-caps “JAVA” in the top navigation bar

[Jonathan Gibbons]

Please review a moderately simple change to `make/Docs.gmk` to move the link 
for "Other Versions" from a "hidden" link in the top nav bar to an explicit 
link in the "bottom" text. The link is placed to appear after the sentence 
beginning "For more information..." and before all the legal text (i.e. 
trademark, copyright, license, etc)

A side effect of moving the link is that the top text reverts to its intended 
appearance of "Java ...", without all-caps.

As before, the presence of the link is optional, and depends on the specific 
docs target. It is set up to just appear for the main JDK/JavaSE documentation, 
and not the other docs bundles, such as docs-reference.

-

Commit messages:
 - JDK-8265483: All-caps “JAVA” in the top navigation bar

Changes: https://git.openjdk.java.net/jdk/pull/3594/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3594=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8265483
  Stats: 16 lines in 1 file changed: 6 ins; 6 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3594.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3594/head:pull/3594

PR: https://git.openjdk.java.net/jdk/pull/3594

Integrated: JDK-8251210: Link JDK api docs to other versions

[Jonathan Gibbons]

On Sat, 6 Mar 2021 02:21:00 GMT, Jonathan Gibbons  wrote:

> Please review a small change to Docs.gmk, such that the short version string 
> in the upper right corner of the main API docs bundle is linked to a page 
> that links to other versions of the documentation.  While it is not so useful 
> in this release, to be able to access _older_ versions, going forward, this 
> will simplify the ability to get to _newer_ versions when search engines take 
> you to the docs for an older release.

This pull request has now been integrated.

Changeset: b1cc864a
Author:    Jonathan Gibbons 
URL:   https://git.openjdk.java.net/jdk/commit/b1cc864a
Stats: 10 lines in 2 files changed: 9 ins; 0 del; 1 mod

8251210: Link JDK api docs to other versions

Reviewed-by: iris, erikj

-

PR: https://git.openjdk.java.net/jdk/pull/2854

Re: RFR: JDK-8251210: Link JDK api docs to other versions

[Jonathan Gibbons]

On Sat, 6 Mar 2021 13:13:02 GMT, liach  
wrote:

> Does this change add the "other versions" text under the current version 
> info? I don't see where the `Other Versions` text (proposed in the original 
> issue) is added. Without that text, imo this utility would be significantly 
> less accessible, as not many people would know this trick by looking at this 
> pull request, the original JDK issue, or by inspecting the html.

No, after offline discussions, we decided to just link the existing text for 
the short version string, to minimize the disruption to the header. This opens 
up the possibility of applying a similar change to modify existing published 
docs in a similar manner, which is the more interesting case.

-

PR: https://git.openjdk.java.net/jdk/pull/2854

RFR: JDK-8251210: Link JDK api docs to other versions

[Jonathan Gibbons]

Please review a small change to Docs.gmk, such that the short version string in 
the upper right corner of the main API docs bundle is linked to a page that 
links to other versions of the documentation.  While it is not so useful in 
this release, to be able to access _older_ versions, going forward, this will 
simplify the ability to get to _newer_ versions when search engines take you to 
the docs for an older release.

-

Commit messages:
 - JDK-8251210: Link JDK api docs to other versions

Changes: https://git.openjdk.java.net/jdk/pull/2854/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2854=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8251210
  Stats: 10 lines in 2 files changed: 9 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2854.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2854/head:pull/2854

PR: https://git.openjdk.java.net/jdk/pull/2854

Re: RFR: 8262893: Enable more doclint checks in javadoc build

[Jonathan Gibbons]

On Tue, 2 Mar 2021 22:41:38 GMT, Joe Darcy  wrote:

> With recent cleanups, several doclint categories have been cleared across the 
> JDK and should be enabled in the javadoc build to make sure they stay clean.
> 
> Local docs build completes with this stricter set of checks.



-

Marked as reviewed by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2799

Integrated: JDK-8262315: missing ';' in generated entities

[Jonathan Gibbons]

On Wed, 24 Feb 2021 17:45:19 GMT, Jonathan Gibbons  wrote:

> Fix  typo-by-omission.  A `;` is missing after ``.
> 
> This is in the build tools, in the code for the `@jls` taglet, used to 
> generate docs. No change to JDK product code.

This pull request has now been integrated.

Changeset: 65492129
Author:    Jonathan Gibbons 
URL:   https://git.openjdk.java.net/jdk/commit/65492129
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8262315: missing ';' in generated entities

Reviewed-by: lancea

-

PR: https://git.openjdk.java.net/jdk/pull/2709

RFR: JDK-8262315: missing ';' in generated entities

[Jonathan Gibbons]

Fix  typo-by-omission.  A `;` is missing after ``.

This is in the build tools, in the code for the `@jls` taglet, used to generate 
docs. No change to JDK product code.

-

Commit messages:
 - JDK-8262315: missing ';' in generated entities

Changes: https://git.openjdk.java.net/jdk/pull/2709/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2709=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8262315
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2709.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2709/head:pull/2709

PR: https://git.openjdk.java.net/jdk/pull/2709

Re: RFR: 8259512: Update --release 16 symbol information for JDK 16 build 31

[Jonathan Gibbons]

On Fri, 8 Jan 2021 22:36:01 GMT, Joe Darcy  wrote:

> Using the symbol updating script, update the symbol files for API changes in 
> JDK 16 build 31.

Marked as reviewed by jjg (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/2010

Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v13]

[Jonathan Gibbons]

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]

[Jonathan Gibbons]

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: 8247957: remove doclint support for HTML 4 [v7]

[Jonathan Gibbons]

On Fri, 25 Dec 2020 02:17:16 GMT, Yoshiki Sato  wrote:

>> HTML4 is no longer supported in javadoc.
>> 
>> doclint needs to drop HTML4 support as well.
>> The changes consist of:
>> * Removing -Xhtmlversion option from doclint and --doclint-format from javac.
>> * Removing jdk.javadoc.internal.doclint.HtmlVersion and its references.
>> * Updating makefile not to use removed option.
>> * Sorting out supported tags and attributes in HTML5 (including fix 
>> incorrect permission of valign in TH, TR, TD, THEAD and TBODY)
>> * Fixing incorrect value checks for the id attribute.
>> * Modifying test code and expected outputs to be checked in HTML5
>
> Yoshiki Sato has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains ten commits:
> 
>  - Merge master
>  - 7th: Some fixes and additional changes for 8247957
>  - 5th: 8258460: Remove --doclint-format option from javac
>  - 5th: 8257204 and 8256313
>8257204: Remove usage of -Xhtmlversion option from javac
>8256313: JavaCompilation.gmk needs to be updated not to use 
> --doclint-format html5 option
>  - 8257204 and 8256313
>8257204: Remove usage of -Xhtmlversion option from javac
>8256313: JavaCompilation.gmk needs to be updated not to use 
> --doclint-format html5 option
>  - 3rd: 8247957 and 8256312
>  - Merge branch 'master' into JDK-8247957_2
>  - 8247957: remove doclint support for HTML 4
>8256312: Valid anchor 'id' value not allowed
>  - 8247957: remove doclint support for HTML 4

Marked as reviewed by jjg (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/893

Re: RFR: 8247957: remove doclint support for HTML 4 [v6]

[Jonathan Gibbons]

On Thu, 24 Dec 2020 02:54:17 GMT, Yoshiki Sato  wrote:

>> HTML4 is no longer supported in javadoc.
>> 
>> doclint needs to drop HTML4 support as well.
>> The changes consist of:
>> * Removing jdk.javadoc.internal.doclint.HtmlVersion and its references.
>> * Sorting out supported tags and attributes in HTML5 (including fix 
>> incorrect permission of valign in TH, TR, TD, THEAD and TBODY)
>> * Modifying test code and expected outputs to be checked in HTML5
>
> Yoshiki Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   7th: Some fixes and additional changes for 8247957

Marked as reviewed by jjg (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/893

Re: RFR: 8247957: remove doclint support for HTML 4 [v5]

[Jonathan Gibbons]

On Wed, 23 Dec 2020 17:08:55 GMT, Jonathan Gibbons  wrote:

>> \s was introduced as a valid escape character in JDK 15 as part of the 
>> support for Text Blocks.
>> 
>> https://docs.oracle.com/javase/specs/jls/se15/html/jls-3.html#jls-EscapeSequence
>> https://docs.oracle.com/javase/specs/jls/se14/preview/specs/text-blocks-jls.html#jls-3.10.7
>> 
>> FWIW, the escape sequence showed up with red squiggly lines in my IDE.
>> 
>> -- Jon
>> 
>> On 12/22/20 9:38 PM, Yoshiki SATO wrote:
>>>
>>> *@satoyoshiki* commented on this pull request.
>>>
>>> 
>>>
>>> In 
>>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java 
>>> <https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/893*discussion_r547668049__;Iw!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZiYojsWrqg$>:
>>>
>>> > @@ -765,8 +732,8 @@ private Element getEnclosingPackageOrClass(Element e) 
>>> > {
>>>   return e;
>>>   }
>>>   
>>> -//http://www.w3.org/TR/html401/types.html#type-name  
>>> <https://urldefense.com/v3/__http://www.w3.org/TR/html401/types.html*type-name__;Iw!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZibVYnjqnQ$>
>>> -private static final Pattern validName = 
>>> Pattern.compile("[A-Za-z][A-Za-z0-9-_:.]*");
>>> +//https://html.spec.whatwg.org/#the-id-attribute  
>>> <https://urldefense.com/v3/__https://html.spec.whatwg.org/*the-id-attribute__;Iw!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZib5vw0eRg$>
>>> +private static final Pattern validId = Pattern.compile("[^\s]+");
>>>
>>> Correct. Thanks a lot for finding this error.
>>> Now that I have doubts why this line could have been compiled without 
>>> error. This line should cause a compiler error.
>>>
>>> Let me review all anchor tests again because the logic should be 
>>> checked there.
>>>
>>> —
>>> 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/893*discussion_r547668049__;Iw!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZiYojsWrqg$>,
>>>  
>>> or unsubscribe 
>>> <https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AOUXBRXCJBCNZR4ZUBZCEBTSWF66TANCNFSM4TBXTH2Q__;!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZia6kQL08w$>.
>>>
>
> On 12/22/20 10:42 PM, Yoshiki SATO wrote:
>>
>> *@satoyoshiki* commented on this pull request.
>>
>> 
>>
>> In 
>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java 
>> <https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/893*discussion_r547716025__;Iw!!GqivPVa7Brio!Oae1uD8yALBU6ReCJTwKccZjdEsGyXddDLOOeF2SJLi8x7SoU5jr-Jr0KVl2EVajBRHM-g$>:
>>
>> > -}
>> -
>> -private void validateHtml5Attrs(AttributeTree tree, Name name, AttrKind 
>> k) {
>> -switch (k) {
>> -case ALL:
>> -case HTML5:
>> -break;
>> -
>> -case INVALID:
>> -case OBSOLETE:
>> -case USE_CSS:
>> -case HTML4:
>> -env.messages.error(HTML, tree, 
>> "dc.attr.not.supported.html5", name);
>> -break;
>> -}
>> -}
>>   
>>   private boolean checkAnchor(String name) {
>>
>> I understand. But is it really no problem to be done in part of the 
>> cleanup of doclint?
>> Looking at the classes in jdk/javadoc/internal/doclint, the term 
>> |(anchor|Anchor)| looks like only used in Checker.java and resource 
>> files. But a lot of other files, for instance in 
>> jdk/javadoc/internal/doclets, use this term to refer to the |id| or 
>> |name| attribute. I would be fine if it is supposed to be done in each 
>> cleanup in the future.
>>
>> —
>> 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/893*discussion_r547716025__;Iw!!GqivPVa7Brio!Oae1uD8yALBU6ReCJTwKccZjdEsGyXddDLOOeF2SJLi8x7SoU5jr-Jr0KV

Re: RFR: 8247957: remove doclint support for HTML 4 [v5]

[Jonathan Gibbons]

On Wed, 23 Dec 2020 15:53:00 GMT, Jonathan Gibbons  wrote:

>> One must-fix item (bad pattern constant.)
>
> \s was introduced as a valid escape character in JDK 15 as part of the 
> support for Text Blocks.
> 
> https://docs.oracle.com/javase/specs/jls/se15/html/jls-3.html#jls-EscapeSequence
> https://docs.oracle.com/javase/specs/jls/se14/preview/specs/text-blocks-jls.html#jls-3.10.7
> 
> FWIW, the escape sequence showed up with red squiggly lines in my IDE.
> 
> -- Jon
> 
> On 12/22/20 9:38 PM, Yoshiki SATO wrote:
>>
>> *@satoyoshiki* commented on this pull request.
>>
>> 
>>
>> In 
>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java 
>> <https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/893*discussion_r547668049__;Iw!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZiYojsWrqg$>:
>>
>> > @@ -765,8 +732,8 @@ private Element getEnclosingPackageOrClass(Element e) {
>>   return e;
>>   }
>>   
>> -//http://www.w3.org/TR/html401/types.html#type-name  
>> <https://urldefense.com/v3/__http://www.w3.org/TR/html401/types.html*type-name__;Iw!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZibVYnjqnQ$>
>> -private static final Pattern validName = 
>> Pattern.compile("[A-Za-z][A-Za-z0-9-_:.]*");
>> +//https://html.spec.whatwg.org/#the-id-attribute  
>> <https://urldefense.com/v3/__https://html.spec.whatwg.org/*the-id-attribute__;Iw!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZib5vw0eRg$>
>> +private static final Pattern validId = Pattern.compile("[^\s]+");
>>
>> Correct. Thanks a lot for finding this error.
>> Now that I have doubts why this line could have been compiled without 
>> error. This line should cause a compiler error.
>>
>> Let me review all anchor tests again because the logic should be 
>> checked there.
>>
>> —
>> 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/893*discussion_r547668049__;Iw!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZiYojsWrqg$>,
>>  
>> or unsubscribe 
>> <https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AOUXBRXCJBCNZR4ZUBZCEBTSWF66TANCNFSM4TBXTH2Q__;!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZia6kQL08w$>.
>>

On 12/22/20 10:42 PM, Yoshiki SATO wrote:
>
> *@satoyoshiki* commented on this pull request.
>
> 
>
> In 
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java 
> <https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/893*discussion_r547716025__;Iw!!GqivPVa7Brio!Oae1uD8yALBU6ReCJTwKccZjdEsGyXddDLOOeF2SJLi8x7SoU5jr-Jr0KVl2EVajBRHM-g$>:
>
> > -}
> -
> -private void validateHtml5Attrs(AttributeTree tree, Name name, AttrKind 
> k) {
> -switch (k) {
> -case ALL:
> -case HTML5:
> -break;
> -
> -case INVALID:
> -case OBSOLETE:
> -case USE_CSS:
> -case HTML4:
> -env.messages.error(HTML, tree, 
> "dc.attr.not.supported.html5", name);
> -break;
> -}
> -}
>   
>   private boolean checkAnchor(String name) {
>
> I understand. But is it really no problem to be done in part of the 
> cleanup of doclint?
> Looking at the classes in jdk/javadoc/internal/doclint, the term 
> |(anchor|Anchor)| looks like only used in Checker.java and resource 
> files. But a lot of other files, for instance in 
> jdk/javadoc/internal/doclets, use this term to refer to the |id| or 
> |name| attribute. I would be fine if it is supposed to be done in each 
> cleanup in the future.
>
> —
> 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/893*discussion_r547716025__;Iw!!GqivPVa7Brio!Oae1uD8yALBU6ReCJTwKccZjdEsGyXddDLOOeF2SJLi8x7SoU5jr-Jr0KVl2EVajBRHM-g$>,
>  
> or unsubscribe 
> <https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AOUXBRQS4FULCERHKMAZ5OLSWGGNVANCNFSM4TBXTH2Q__;!!GqivPVa7Brio!Oae1uD8yALBU6ReCJTwKccZjdEsGyXddDLOOeF2SJLi8x7SoU5jr-Jr0KVl2EVYEkdTG2A$>.
>

If you were to cleanup the "anchor" terminology, it should only be done 
in doclint, meaning Checker.java and resource files. Cleaning up 
terminology in the rest of javadoc is another project for another day. 
But, it's OK to skip this terminology cleanup in doclint at this time, 
since it is only peripherally related to the primary goal to remove 
HTML4 support.

-- Jon

>

-

PR: https://git.openjdk.java.net/jdk/pull/893

Re: RFR: 8247957: remove doclint support for HTML 4 [v5]

[Jonathan Gibbons]

On Wed, 23 Dec 2020 05:10:31 GMT, Jonathan Gibbons  wrote:

>> Yoshiki Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   5th: 8258460: Remove --doclint-format option from javac
>
> One must-fix item (bad pattern constant.)

\s was introduced as a valid escape character in JDK 15 as part of the 
support for Text Blocks.

https://docs.oracle.com/javase/specs/jls/se15/html/jls-3.html#jls-EscapeSequence
https://docs.oracle.com/javase/specs/jls/se14/preview/specs/text-blocks-jls.html#jls-3.10.7

FWIW, the escape sequence showed up with red squiggly lines in my IDE.

-- Jon

On 12/22/20 9:38 PM, Yoshiki SATO wrote:
>
> *@satoyoshiki* commented on this pull request.
>
> 
>
> In 
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java 
> <https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/893*discussion_r547668049__;Iw!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZiYojsWrqg$>:
>
> > @@ -765,8 +732,8 @@ private Element getEnclosingPackageOrClass(Element e) {
>   return e;
>   }
>   
> -//http://www.w3.org/TR/html401/types.html#type-name  
> <https://urldefense.com/v3/__http://www.w3.org/TR/html401/types.html*type-name__;Iw!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZibVYnjqnQ$>
> -private static final Pattern validName = 
> Pattern.compile("[A-Za-z][A-Za-z0-9-_:.]*");
> +//https://html.spec.whatwg.org/#the-id-attribute  
> <https://urldefense.com/v3/__https://html.spec.whatwg.org/*the-id-attribute__;Iw!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZib5vw0eRg$>
> +private static final Pattern validId = Pattern.compile("[^\s]+");
>
> Correct. Thanks a lot for finding this error.
> Now that I have doubts why this line could have been compiled without 
> error. This line should cause a compiler error.
>
> Let me review all anchor tests again because the logic should be 
> checked there.
>
> —
> 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/893*discussion_r547668049__;Iw!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZiYojsWrqg$>,
>  
> or unsubscribe 
> <https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AOUXBRXCJBCNZR4ZUBZCEBTSWF66TANCNFSM4TBXTH2Q__;!!GqivPVa7Brio!OUEFldgNdIV8aqoReM0lNXfIYO3IoXGR1RUbcYmVdEHRRdA3oy1Zaa3h2DevZia6kQL08w$>.
>

-

PR: https://git.openjdk.java.net/jdk/pull/893

Re: RFR: 8247957: remove doclint support for HTML 4 [v5]

[Jonathan Gibbons]

On Fri, 18 Dec 2020 02:34:16 GMT, Yoshiki Sato  wrote:

>> HTML4 is no longer supported in javadoc.
>> 
>> doclint needs to drop HTML4 support as well.
>> The changes consist of:
>> * Removing jdk.javadoc.internal.doclint.HtmlVersion and its references.
>> * Sorting out supported tags and attributes in HTML5 (including fix 
>> incorrect permission of valign in TH, TR, TD, THEAD and TBODY)
>> * Modifying test code and expected outputs to be checked in HTML5
>
> Yoshiki Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   5th: 8258460: Remove --doclint-format option from javac

One must-fix item (bad pattern constant.)

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java line 
736:

> 734: 
> 735: // https://html.spec.whatwg.org/#the-id-attribute
> 736: private static final Pattern validId = Pattern.compile("[^\s]+");

The regular expression is invalid and needs to be fixed. It should be 
`Pattern.compile("[^\\s]+")`
Note the extra `` character. This is because you need to escape the `` 
character in the string constant, so that the `` is seen in the pattern as part 
of `\s`.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java line 
710:

> 708: 
> 709: 
> 710: private boolean checkAnchor(String name) {

I was going to let it slide for this round of cleanup, but if you're editing 
this file again (see comment on line 736) it might be worth changing the use of 
`anchor` to `id`.  `anchor` is a term that was more appropriate in the days 
before the `id` attribute, when we used ``.  This is an optional 
suggestion.  It might equally be worth focussing on the must-fix items, and 
postpone this cleanup for later.

-

Changes requested by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/893

Re: RFR: 8247957: remove doclint support for HTML 4 [v4]

[Jonathan Gibbons]

On Fri, 18 Dec 2020 01:34:15 GMT, Yoshiki Sato  wrote:

>> HTML4 is no longer supported in javadoc.
>> 
>> doclint needs to drop HTML4 support as well.
>> The changes consist of:
>> * Removing jdk.javadoc.internal.doclint.HtmlVersion and its references.
>> * Sorting out supported tags and attributes in HTML5 (including fix 
>> incorrect permission of valign in TH, TR, TD, THEAD and TBODY)
>> * Modifying test code and expected outputs to be checked in HTML5
>
> Yoshiki Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   5th: 8257204 and 8256313
>   8257204: Remove usage of -Xhtmlversion option from javac
>   8256313: JavaCompilation.gmk needs to be updated not to use 
> --doclint-format html5 option

src/jdk.compiler/share/man/javac.1 line 638:

> 636: .RS
> 637: .RE
> 638: .TP

In general, you should not edit files like this; these files are generated from 
upstream Markdown files.

-

PR: https://git.openjdk.java.net/jdk/pull/893

Re: RFR: 8247957: remove doclint support for HTML 4 [v5]

[Jonathan Gibbons]

On Fri, 18 Dec 2020 02:34:16 GMT, Yoshiki Sato  wrote:

>> HTML4 is no longer supported in javadoc.
>> 
>> doclint needs to drop HTML4 support as well.
>> The changes consist of:
>> * Removing jdk.javadoc.internal.doclint.HtmlVersion and its references.
>> * Sorting out supported tags and attributes in HTML5 (including fix 
>> incorrect permission of valign in TH, TR, TD, THEAD and TBODY)
>> * Modifying test code and expected outputs to be checked in HTML5
>
> Yoshiki Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   5th: 8258460: Remove --doclint-format option from javac

Looks good.
I'll pull down a copy, for review in an IDE and to build and run tests, and if 
that goes OK, I will approve it.

-

PR: https://git.openjdk.java.net/jdk/pull/893

[jdk16] Integrated: JDK-8247994: Localize javadoc search

[Jonathan Gibbons]

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]

[Jonathan Gibbons]

> 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=16=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk16=16=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]

[Jonathan Gibbons]

> 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=16=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk16=16=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: RFR: 8247957: remove doclint support for HTML 4 [v3]

[Jonathan Gibbons]

On Thu, 17 Dec 2020 01:40:14 GMT, Yoshiki Sato  wrote:

>> HTML4 is no longer supported in javadoc.
>> 
>> doclint needs to drop HTML4 support as well.
>> The changes consist of:
>> * Removing jdk.javadoc.internal.doclint.HtmlVersion and its references.
>> * Sorting out supported tags and attributes in HTML5 (including fix 
>> incorrect permission of valign in TH, TR, TD, THEAD and TBODY)
>> * Modifying test code and expected outputs to be checked in HTML5
>
> Yoshiki Sato has refreshed the contents of this pull request, and previous 
> commits have been removed. The incremental views will show differences 
> compared to the previous content of the PR. The pull request contains one new 
> commit since the last revision:
> 
>   8257204 and 8256313
>   8257204: Remove usage of -Xhtmlversion option from javac
>   8256313: JavaCompilation.gmk needs to be updated not to use 
> --doclint-format html5 option

Mostly OK; some minor suggestions

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/resources/doclint.properties
 line 36:

> 34: dc.attr.img.border.not.number = attribute "border" for img is not a number
> 35: dc.attr.table.border.not.valid = attribute "border" for table only 
> accepts "" or "1", use CSS instead: {0}
> 36: dc.attr.table.border.not.number = attribute "border" for table is not a 
> number

suggest dropping "use CSS instead"

-

PR: https://git.openjdk.java.net/jdk/pull/893

Re: RFR: 8247957: remove doclint support for HTML 4 [v3]

[Jonathan Gibbons]

On Thu, 17 Dec 2020 04:50:59 GMT, Jonathan Gibbons  wrote:

>> Yoshiki Sato has refreshed the contents of this pull request, and previous 
>> commits have been removed. The incremental views will show differences 
>> compared to the previous content of the PR. The pull request contains one 
>> new commit since the last revision:
>> 
>>   8257204 and 8256313
>>   8257204: Remove usage of -Xhtmlversion option from javac
>>   8256313: JavaCompilation.gmk needs to be updated not to use 
>> --doclint-format html5 option
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/resources/doclint.properties
>  line 36:
> 
>> 34: dc.attr.img.border.not.number = attribute "border" for img is not a 
>> number
>> 35: dc.attr.table.border.not.valid = attribute "border" for table only 
>> accepts "" or "1", use CSS instead: {0}
>> 36: dc.attr.table.border.not.number = attribute "border" for table is not a 
>> number
> 
> suggest dropping "use CSS instead"

The wording of `attribute "border" for img is not a number` seems strange.
Suggest something like `invalid value for attribute "border": {0}`

-

PR: https://git.openjdk.java.net/jdk/pull/893

Re: RFR: 8247957: remove doclint support for HTML 4 [v3]

[Jonathan Gibbons]

On Thu, 12 Nov 2020 03:10:01 GMT, Yoshiki Sato  wrote:

>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/HtmlTag.java line 
>> 410:
>> 
>>> 408: OBSOLETE,
>>> 409: UNSUPPORTED
>>> 410: }
>> 
>> On one hand, I don't think we need this level of detail, but on the other, I 
>> see it closely matches `AttrKind`, so OK.
>> 
>> Is there are useful distinction between INVALID / OBSOLETE / UNSUPPORTED ?
>
> OK: valid
> OBSOLETE: obsolete, deprecated, but still supported (valid)
> UNSUPPORTED: ever supported but no longer supported (invalid)
> INVALID: the rest of others (invalid)
> 
> UNSUPPORTED can be used if we would like to choose a friendly message instead 
> of saying "unknown tag" only.
> OBSOLETE is not used anywhere in this commit.  Although HTML5 has some 
> obsolete features, 
> [JDK-8215577](https://bugs.openjdk.java.net/browse/JDK-8215577) didn't define 
> them as valid features if my understanding is correct.  So I chose not to 
> allow obsolete features in order to avoid inconsistency.

For both `ElemKind` and `AttrKind` there only seem to be two kinds:
* valid
* previously valid

For these two cases, `OK` is obviously reasonable for `valid`, but `OBSOLETE` 
seems a better fit than `UNSUPPORTED`, but you could also use `HTML4` or 
`OLD_HTML4` or something like that to indicate why we're keeping the name 
around for better messages.  Or, stay with `UNSUPPORTED` but add a doc comment 
explaining that it was previously supported but no longer supported

-

PR: https://git.openjdk.java.net/jdk/pull/893

Re: [jdk16] RFR: JDK-8247994: Localize javadoc search [v2]

[Jonathan Gibbons]

> 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=16=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk16=16=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

[Jonathan Gibbons]

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

Re: RFR: JDK-6251738: Want a top-level summary page that itemizes all spec documents referenced from javadocs (OEM spec) [v2]

[Jonathan Gibbons]

On Thu, 5 Nov 2020 16:16:44 GMT, Pavel Rappo  wrote:

>> Jonathan Gibbons has updated the pull request with a new target base due to 
>> a merge or a rebase. The pull request now contains 11 commits:
>> 
>>  - Fix merge issues; review feedback
>>  - Merge with master
>>  - allow rich content in createAnchorAndSearchIndex
>>  - update Docs.gmk to stop disabling spec tag
>>  - fix TestSpecTag.testEncodedURI
>>  - fix tests
>>  - remove support to workaround legacy @spec tag
>>  - Merge remote-tracking branch 'upstream/master' into new-spec-tag
>>  - fix trailing whitespace in test
>>  - temporarily allow existing legacy usage `@spec JPMS` `@spec jsr-51`
>>  - ... and 1 more: 
>> https://git.openjdk.java.net/jdk/compare/804bd725...ed5512d9
>
> 1. Thanks for incorporating my previous offline feedback.
> 2. Since Hannes and Erik seem to have looked at everything else, I looked 
> mostly at changes in `src/jdk.compiler/share/classes/com/sun/source/**`, 
> which are good!
> 3. There should be a corresponding but separate change to "Documentation 
> Comment Specification for the Standard Doclet".
> 4. Can we use this new `@since` tag to refer to the spec at 
> `com/sun/tools/javac/parser/DocCommentParser.java:1116`?
> 5. Should we specify in `com.sun.source.doctree.SpecTree` that both `url` and 
> `label` parts are mandatory?
> 6. `DCSpec extends DCEndPosTree`, sigh... Although that is not a public API, 
> this design suggests we could improve that abstraction sometime later.

Closing pull request, until we better decide the contents of the spec page

-

PR: https://git.openjdk.java.net/jdk/pull/790

Withdrawn: JDK-6251738: Want a top-level summary page that itemizes all spec documents referenced from javadocs (OEM spec)

[Jonathan Gibbons]

On Thu, 22 Oct 2020 02:40:44 GMT, Jonathan Gibbons  wrote:

> This introduces support for a new `@spec` tag that can be used as either an 
> inline tag or as a block tag. It is used to identify references to external 
> specifications, in such a way that the references can be collected together 
> on a new summary page, called "Other Specifications", available from either 
> the static INDEX page or the interactive search box.
> 
> As an inline tag, the format is `{@spec url label}`, which is roughly 
> translated to `label` in the generated docs.
> 
> As a block tag, the format is simply
> 
> @spec url label
> 
> which is handled in a manner analogous to
> 
> @see label
> 
> The tag is notable for being the first standard/supported tag that can be 
> used as either an inline or block tag. (We have had support for bimodal 
> non-standard/custom tags for a while, such as `{@jls}` and `{@jvms}`.) To 
> support bimodal standard tags, some changes to `DocCommentParser` are 
> incorporated here.
> 
> This change is only the _support_ for the new tag;  it does _not_ include any 
> changes to API docs to _use_ the new tag.

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.java.net/jdk/pull/790

[jdk16] RFR: JDK-8247994: Localize javadoc search

[Jonathan Gibbons]

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=16=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

[Jonathan Gibbons]

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]

[Jonathan Gibbons]

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: 8257450: Start of release updates for JDK 17 [v2]

[Jonathan Gibbons]

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]

[Jonathan Gibbons]

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-6251738: Want a top-level summary page that itemizes all spec documents referenced from javadocs (OEM spec) [v3]

[Jonathan Gibbons]

> This introduces support for a new `@spec` tag that can be used as either an 
> inline tag or as a block tag. It is used to identify references to external 
> specifications, in such a way that the references can be collected together 
> on a new summary page, called "Other Specifications", available from either 
> the static INDEX page or the interactive search box.
> 
> As an inline tag, the format is `{@spec url label}`, which is roughly 
> translated to `label` in the generated docs.
> 
> As a block tag, the format is simply
> 
> @spec url label
> 
> which is handled in a manner analogous to
> 
> @see label
> 
> The tag is notable for being the first standard/supported tag that can be 
> used as either an inline or block tag. (We have had support for bimodal 
> non-standard/custom tags for a while, such as `{@jls}` and `{@jvms}`.) To 
> support bimodal standard tags, some changes to `DocCommentParser` are 
> incorporated here.
> 
> This change is only the _support_ for the new tag;  it does _not_ include any 
> changes to API docs to _use_ the new tag.

Jonathan Gibbons has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 12 commits:

 - Merge master
 - Fix merge issues; review feedback
 - Merge with master
 - allow rich content in createAnchorAndSearchIndex
 - update Docs.gmk to stop disabling spec tag
 - fix TestSpecTag.testEncodedURI
 - fix tests
 - remove support to workaround legacy @spec tag
 - Merge remote-tracking branch 'upstream/master' into new-spec-tag
 - fix trailing whitespace in test
 - ... and 2 more: https://git.openjdk.java.net/jdk/compare/300cbaa6...69595e04

-

Changes: https://git.openjdk.java.net/jdk/pull/790/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=790=02
  Stats: 1374 lines in 42 files changed: 1337 ins; 14 del; 23 mod
  Patch: https://git.openjdk.java.net/jdk/pull/790.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/790/head:pull/790

PR: https://git.openjdk.java.net/jdk/pull/790

Re: RFR: 8256308: Send arguments to javac server in a config file [v6]

[Jonathan Gibbons]

On Mon, 16 Nov 2020 12:44:08 GMT, Magnus Ihse Bursie  wrote:

>> Currently, to use the javac server, a horrendously long command line option 
>> is created, looking like this: `--server:portfile=> portfile>:sjavac=`, where the sjavac command has 
>> had all spaces replaced by %20. Since Project Jigsaw, the set of module 
>> arguments needed is huge to begin with, making this command line 
>> incomprehensible after mangling.
>> 
>> Apart from making java command lines hard to read (and copy/paste!) by 
>> developers, it also makes it hard for scripts to parse. The upcoming winenv 
>> rewrite is dependent on being able to differentiate between path names and 
>> other arguments, which is not possible in this mess.
>> 
>> So, instead, let's write it to a file, without any escaping, and just pass 
>> the configuration file name to the server.
>> 
>> Note that this will change the behavior of the javac server, but as the 
>> source code states this is not a documented or externally supported API no 
>> CSR is needed. 
>> 
>> I also cleaned up some code in SjavacClient, in particular code relating to 
>> the passing of arguments. (We never change poolsize or keepalive when we 
>> call it.)
>
> Magnus Ihse Bursie has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Fix whitespace issues

There seems to be much more here than there needs to be.

If the issue is just long command lines, then the obvious/conventional solution 
would be to use @files, which would be a tiny change to the sjavac source code, 
to insert a single call to `CommandLine.parse` to expand any @file arguments on 
the command line.

So, before reading all the various details in this proposed change, is there 
any reason why the simple @file solution cannot be used?

-

PR: https://git.openjdk.java.net/jdk/pull/1195

Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v11]

[Jonathan Gibbons]

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: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v6]

[Jonathan Gibbons]

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-6251738: Want a top-level summary page that itemizes all spec documents referenced from javadocs (OEM spec) [v2]

[Jonathan Gibbons]

> This introduces support for a new `@spec` tag that can be used as either an 
> inline tag or as a block tag. It is used to identify references to external 
> specifications, in such a way that the references can be collected together 
> on a new summary page, called "Other Specifications", available from either 
> the static INDEX page or the interactive search box.
> 
> As an inline tag, the format is `{@spec url label}`, which is roughly 
> translated to `label` in the generated docs.
> 
> As a block tag, the format is simply
> 
> @spec url label
> 
> which is handled in a manner analogous to
> 
> @see label
> 
> The tag is notable for being the first standard/supported tag that can be 
> used as either an inline or block tag. (We have had support for bimodal 
> non-standard/custom tags for a while, such as `{@jls}` and `{@jvms}`.) To 
> support bimodal standard tags, some changes to `DocCommentParser` are 
> incorporated here.
> 
> This change is only the _support_ for the new tag;  it does _not_ include any 
> changes to API docs to _use_ the new tag.

Jonathan Gibbons has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 11 commits:

 - Fix merge issues; review feedback
 - Merge with master
 - allow rich content in createAnchorAndSearchIndex
 - update Docs.gmk to stop disabling spec tag
 - fix TestSpecTag.testEncodedURI
 - fix tests
 - remove support to workaround legacy @spec tag
 - Merge remote-tracking branch 'upstream/master' into new-spec-tag
 - fix trailing whitespace in test
 - temporarily allow existing legacy usage `@spec JPMS` `@spec jsr-51`
 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/804bd725...ed5512d9

-

Changes: https://git.openjdk.java.net/jdk/pull/790/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=790=01
  Stats: 1374 lines in 42 files changed: 1337 ins; 14 del; 23 mod
  Patch: https://git.openjdk.java.net/jdk/pull/790.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/790/head:pull/790

PR: https://git.openjdk.java.net/jdk/pull/790

Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v6]

[Jonathan Gibbons]

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.

Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v4]

[Jonathan Gibbons]

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]

[Jonathan Gibbons]

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 [v4]

[Jonathan Gibbons]

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]

[Jonathan Gibbons]

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]

[Jonathan Gibbons]

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: name

Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v2]

[Jonathan Gibbons]

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

RFR: JDK-6251738: Want a top-level summary page that itemizes all spec documents referenced from javadocs (OEM spec)

[Jonathan Gibbons]

This introduces support for a new `@spec` tag that can be used as either an 
inline tag or as a block tag. It is used to identify references to external 
specifications, in such a way that the references can be collected together on 
a new summary page, called "Other Specifications", available from either the 
static INDEX page or the interactive search box.

As an inline tag, the format is `{@spec url label}`, which is roughly 
translated to `label` in the generated docs.

As a block tag, the format is simply

@spec url label

which is handled in a manner analogous to

@see label

The tag is notable for being the first standard/supported tag that can be used 
as either an inline or block tag. (We have had support for bimodal 
non-standard/custom tags for a while, such as `{@jls}` and `{@jvms}`.) To 
support bimodal standard tags, some changes to `DocCommentParser` are 
incorporated here.

This change is only the _support_ for the new tag;  it does _not_ include any 
changes to API docs to _use_ the new tag.

-

Commit messages:
 - fix TestSpecTag.testEncodedURI
 - fix tests
 - remove support to workaround legacy @spec tag
 - Merge remote-tracking branch 'upstream/master' into new-spec-tag
 - fix trailing whitespace in test
 - temporarily allow existing legacy usage `@spec JPMS` `@spec jsr-51`
 - JDK-6251738: Want a top-level summary page that itemizes all spec documents 
referenced from javadocs (OEM spec)

Changes: https://git.openjdk.java.net/jdk/pull/790/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=790=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-6251738
  Stats: 1376 lines in 42 files changed: 1345 ins; 12 del; 19 mod
  Patch: https://git.openjdk.java.net/jdk/pull/790.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/790/head:pull/790

PR: https://git.openjdk.java.net/jdk/pull/790