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

2021-01-08 Thread Jan Lahoda 
On Fri, 8 Jan 2021 01:51:52 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
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/doclets.properties
>  line 154:
> 
>> 152: doclet.Errors=Errors
>> 153: doclet.Classes=Classes
>> 154: doclet.Records=Records
> 
> I guess I'm mildly surprised to see all these items being removed...

These were used from DeprecatedListWriter.getSummaryKey which is removed by 
this patch (and is unused in the current mainline, as far as I know).

-

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

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

2021-01-07 Thread 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 
> ,
>  
> or unsubscribe 
> .
>

-

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

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

2021-01-07 Thread 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: JDK-8250768: javac should be adapted to changes in JEP 12 [v13]

2021-01-07 Thread Jan Lahoda 
> 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

-

Changes: https://git.openjdk.java.net/jdk/pull/703/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=703&range=12
  Stats: 3719 lines in 134 files changed: 2724 ins; 695 del; 300 mod
  Patch: https://git.openjdk.java.net/jdk/pull/703.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/703/head:pull/703

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