On Fri, 16 Oct 2020 16:07:41 GMT, Maurizio Cimadamore <mcimadamore@openjdk.org> 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

On Fri, 16 Oct 2020 16:47:25 GMT, Erik Joelsson <erikj@openjdk.org> 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

Build changes look good.

I've updated the patch based on the comments: mostly updating the anchors in the javadoc, but also removing the updates to JavacParser, which are only loosely related to the primary focus of this patch, and should possibly be done separately. ------------- PR: https://git.openjdk.java.net/jdk/pull/703

On Mon, 19 Oct 2020 14:22:17 GMT, Hannes Wallnöfer <hannesw@openjdk.org> wrote:

...

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 2238:

...

2236: if (previewTree != null) { 2237: previewDiv.add(new HtmlTree(TagName.A).put(HtmlAttr.ID, "preview") 2238: .add(new RawHtml(utils.getPreviewTreeSummaryOrDetails(previewTree, false))));

The `id` attribute needs to be unique within the page, so in addition to make the value not a valid java identifier (as @jonathan-gibbons pointed out in a comment elsewhere) we need to support multiple preview ids per page. One way to do this would be to add the element name to the id value, e.g. `preview-<element name>`.

Of course the element name won't do for overloaded methods and constructors... `Links#getAnchor(ExecutableElement)` should be used for those.

Uh, originally, there was only preview section per file, and I didn't fully realize the JDK's javadoc may have multiple such section. I'll work on this. Thanks for the pointer! ------------- PR: https://git.openjdk.java.net/jdk/pull/703

On Tue, 20 Oct 2020 12:15:23 GMT, Jan Lahoda <jlahoda@openjdk.org> 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/jav... [3] http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.compiler... [4] http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/preview-list.... [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 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

javac changes look good ------------- Marked as reviewed by mcimadamore (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/703

On Wed, 21 Oct 2020 15:25:59 GMT, Hannes Wallnöfer <hannesw@openjdk.org> 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

The javadoc code changes look reasonable, and the preview bits in the generated documentation look good as well.

Apart from my inline comments which all address minor issues, there are a few things I don't like around `PreviewListWriter`: its code replication with `DeprecatedListWriter, and it adds things to HtmlDocletWriter and HtmlStyle that I don't think are strictly necessary. However, I wouldn't expect you to know how we like things done in javadoc, so maybe the simplest solution would be for one of us to go over the javadoc bits, either as part of this pull request or (if you prefer to get it integrated rather sooner) as a follow-up task.

I have update the patch based on Hannes' comments. One open question is whether we should have Utils.elementFlags, or just Utils.isPreviewElement. @jonathan-gibbons, any preference? Thanks! ------------- PR: https://git.openjdk.java.net/jdk/pull/703

On Fri, 23 Oct 2020 16:21:53 GMT, Jan Lahoda <jlahoda@openjdk.org> 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/jav... [3] http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.compiler... [4] http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/preview-list.... [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:

Removing unnecessary cast.

I have primarily gone through all the javadoc changes. There are three main areas of feedback: 1. The ElementFlag question. We have gone from one kind of element with special treatment (deprecated) to two (deprecated, preview), and there are signs of a third kind coming soon, maybe as early as next year, for work currently being discussed in the Panama project. As the saying goes, three would be one too many. So, I agree `ElementFlag` is underutilized today and could reasonably be removed, but it does seem worthwhile to keep, and it would even be worth increasing its usage soon, such as to combine methods and classes for deprecated elements and preview elements. I'm not sure I can go back into looking at files while typing this message, but if we are to keep `ElementFlag` we should at a minimum provide a better description of its purpose. For example, can/should it be used for all predicates on elements, or just the elements that get "special" handling when generating docs. 2. The use of strings containing HTML, and use of `RawHtml`, instead of working in terms of `Content` nodes such as `HtmlTree` and `StringContent`. 3. Track recent updates to the repo, regarding Conditional Pages. See how we set up conditional pages for the deprecated list, and do the same for preview items. This is probably a must-do item, once you merge with mainline. -- Finally, I realize this is a big chunk of work (well done!), across many areas, and that getting through a review is hard. If it is too hard to address some of the comments here, I'm OK if you file follow-up bugs of at least the same priority and Fix Version as for the main bug here: JDK-8250768. (That applies to #1, #2 above; not #3). src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/WorkArounds.java line 526:

524: return (sym.flags() & Flags.PREVIEW_REFLECTIVE) != 0; 525: } 526:

Generally, hacking your way into `Symbol` is undesirable (though accepted if there is no realistic alternative.) Adding new code into the `WorkArounds` class should be seen as a means of last resort when the required information cannot be obtained from public API ... which may require updating the public API as well. For example, should these methods be predicates on the Language Model `Elements` class? src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java line 89:

87: @Override 88: protected Content getDeprecatedOrPreviewLink(Element member) { 89: Content deprecatedLinkContent = new ContentBuilder();

name does not match usage. I suggest simplifying it to just "content". src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java line 88:

86: 87: @Override 88: protected Content getDeprecatedOrPreviewLink(Element member) {

This name is a side-effect of the `ElementFlag` question. We should either use explicit field and method names, or we should use `ElementFlag` more consistently. This method name works OK for now, but if if ever gets to have another `orFoo` component in the name, the signature should be parameterized with something like `ElementFlag` or its equivalent. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AllClassesIndexWriter.java line 172:

170: description.add(getDeprecatedPhrase(klass)); 171: List<? extends DocTree> 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 `<sup>` 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));

This is a deviation from existing practice to allow HTML in resource files. That doesn't seem like the sort of stuff that should be localizable. In other situations, (e.g. the Help page) we put the plain-text contents in the resource file and generate the markup in the code. Elsewhere, I said that `WorkArounds` is a means of last resort. The same is true for `RawHtml`. Use it if you must, but it's better to use other forms of `Content`, like `HtmlTree`. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 2322:

2320: feature.features 2321: .stream() 2322: .map(f -> "<code>" + f + "</code>")

Ugh for using string constants for HTML. Try and use `Content` instead, src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 2329:

2327: featureDisplayName, 2328: featureCodes); 2329: result.add(new RawHtml(text));

General ugh for many of the aforementioned reasons, all related to handling HTML in strings. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 2351:

2349: .map(this::toLink) 2350: .map(link -> getLink(link).toString()) 2351: .collect(Collectors.joining(", "))));

More of the same ... `RawHtml` and building HTML in strings src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/LinkInfoImpl.java line 64:

62: * Indicate that the link appears in member documentation on the Deprecated or Preview page. 63: */ 64: MEMBER_DEPRECATED_PREVIEW,

Will need to be generalized, eventually src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Navigation.java line 228:

226: addTreeLink(tree); 227: addDeprecatedLink(tree); 228: addPreviewLink(tree);

It's OK to put the link here, I guess, but it should also be on the INDEX page. See also recent updates for conditional pages. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Navigation.java line 888:

886: 887: private void addPreviewLink(Content tree) { 888: if (configuration.getIncludedModuleElements().stream().anyMatch(m -> m.getQualifiedName().contentEquals("java.base"))) {

This becomes simpler with recent support for conditional pages. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageWriterImpl.java line 39:

37: import com.sun.source.doctree.TextTree; 38: import com.sun.source.doctree.UnknownInlineTagTree; 39: import java.util.stream.Collectors;

why these imports? src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties line 288:

286: doclet.Declared_Using_Preview.SEALED_PERMITS=Sealed Classes 287: doclet.PreviewPlatformLeadingNote=<code>{0}</code> is a preview API of the Java platform. 288: doclet.ReflectivePreviewPlatformLeadingNote=<code>{0}</code> is a reflective preview API of the Java platform.

HTML in resource files is bad. It would be marginally better to move the HTML to the value being substituted (using strings from Content nodes) and even better to use a method that substitutes Content nodes into a resource string (Not sure if that method exists, but it could/should). src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/ConstructorWriter.java line 79:

77: * @param annotationDocTree content tree to which the preview information will be added 78: */ 79: void addPreview(ExecutableElement member, Content contentTree);

Note to javadoc-team: Consider using default methods to provide an empty implementation. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/builders/ConstructorBuilder.java line 28:

26: package jdk.javadoc.internal.doclets.toolkit.builders; 27: 28: import static java.lang.invoke.ConstantBootstraps.enumConstant;

Really? If it is required, it is in the wrong place. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/PreviewAPIListBuilder.java line 46:

44: * deletion without notice.</b> 45: */ 46: public class PreviewAPIListBuilder {

OK for now, but it might be worth eventually trying to merge this with `DeprecatedListBuilder` src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/PreviewAPIListBuilder.java line 79:

77: for (PreviewElementKind kind : PreviewElementKind.values()) { 78: previewMap.put(kind, 79: new TreeSet<>(utils.comparators.makeDeprecatedComparator()));

The use of `makeDeprecatedComparator` is not awful, but does smell a bit. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/PreviewAPIListBuilder.java line 86:

84: /** 85: * Build the sorted list of all the deprecated APIs in this run. 86: * Build separate lists for deprecated modules, packages, classes, constructors,

"d-word", twice src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/PreviewAPIListBuilder.java line 89:

87: * methods and fields. 88: */ 89: private void buildDeprecatedAPIInfo() {

D-word src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/PreviewAPIListBuilder.java line 133:

131: } 132: } 133: composeDeprecatedList(previewMap.get(PreviewElementKind.FIELD),

I suggest you grep this file for all uses of the d-word src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/PreviewAPIListBuilder.java line 158:

156: * @param members members to be added in the list. 157: */ 158: private void composeDeprecatedList(SortedSet<Element> sset, List<? extends Element> members) {

Last time d-word comment. Consider all such uses to be flagged. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java line 2848:

2846: UnknownInlineTagTree previewTag = (UnknownInlineTagTree) t; 2847: List<? extends DocTree> previewContent = previewTag.getContent(); 2848: String previewText = ((TextTree) previewContent.get(0)).getBody();

This looks unreasonably fragile. And, I thought the tag was going away... at least according to earlier files in this review. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java line 2984:

2982: SEALED(List.of("sealed")), 2983: SEALED_PERMITS(List.of("sealed", "permits")), 2984: RECORD(List.of("record"));

I'm guessing this is about to go away soon? src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java line 2980:

2978: } 2979: 2980: public enum DeclarationPreviewLanguageFeatures {

General thinking aloud question ... how does all this interact with source or release options for an earlier release? src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java line 3025:

3023: case MODULE, PACKAGE -> { 3024: } 3025: default -> throw new IllegalStateException("Unexpected: " + el.getKind());

Should be `IllegalArgumentException` not `IllegalStateException` src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java line 3145:

3143: * Checks whether the given Element should be marked as a preview API. 3144: * 3145: * Note that is a type is marked as a preview, its members are not.

probable typo: "is" -> "if" 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. ------------- PR: https://git.openjdk.java.net/jdk/pull/703

On Tue, 27 Oct 2020 16:11:18 GMT, Jan Lahoda <jlahoda@openjdk.org> wrote:

...

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

...

2978: } 2979: 2980: public enum DeclarationPreviewLanguageFeatures {

General thinking aloud question ... how does all this interact with source or release options for an earlier release?

I don't think there should be much interaction with -source <older-version>. 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. ------------- PR: https://git.openjdk.java.net/jdk/pull/703

On Tue, 27 Oct 2020 16:09:45 GMT, Jan Lahoda <jlahoda@openjdk.org> 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

On Thu, 29 Oct 2020 09:26:05 GMT, Jan Lahoda <jlahoda@openjdk.org> 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

On Fri, 23 Oct 2020 18:19:13 GMT, Jonathan Gibbons <jjg@openjdk.org> wrote:

...

Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision:

Removing unnecessary cast.

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

...

2846: UnknownInlineTagTree previewTag = (UnknownInlineTagTree) t; 2847: List<? extends DocTree> previewContent = previewTag.getContent(); 2848: String previewText = ((TextTree) previewContent.get(0)).getBody();

This looks unreasonably fragile. And, I thought the tag was going away... at least according to earlier files in this review.

This was intended to allow Preview APIs to provide their own text instead of the generic one. But, looking again at JEP 12, this shouldn't be supported, so removing. ------------- PR: https://git.openjdk.java.net/jdk/pull/703

On Mon, 2 Nov 2020 18:15:09 GMT, Jan Lahoda <jlahoda@openjdk.org> 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/jav... [3] http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.compiler... [4] http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/preview-list.... [5] http://cr.openjdk.java.net/~jlahoda/8250768/test.javadoc.00/ [6] https://bugs.openjdk.java.net/browse/JDK-8250769

Jan Lahoda has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 46 commits:

- Removing trailing whitespace. - Merging master into JDK-8250768. - Updating tests after records are a final feature. - Fixing tests. - Finalizing removal of record preview hooks. - Merging master into JDK-8250768 - Reflecting review comments. - Merge branch 'master' into JDK-8250768 - Removing unnecessary cast. - Using a more correct way to get URLs. - ... and 36 more: https://git.openjdk.java.net/jdk/compare/d93e3a7d...2e403900

I have read all the files. I have added a n umber of various minor non-blocking comments (no need for re-review( to fix these. But I have a couple of comments/questions before finally giving approval. There's a comment in `PreviewListWriter` about annotation members that needs too be addressed, and I wonder is RECORD and RECORD_COMPONENT need to be added into PreviewElementKind. src/java.base/share/classes/jdk/internal/javac/PreviewFeature.java line 75:

73: * A key for testing. 74: */ 75: TEST,

Slightly weird src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTaskPool.java line 257:

255: //when patching modules (esp. java.base), it may be impossible to 256: //clear the symbols read from the patch path: 257: polluted |= get(JavaFileManager.class).hasLocation(StandardLocation.PATCH_MODULE_PATH);

OK, but looks unrelated to primary work src/jdk.compiler/share/classes/com/sun/tools/javac/code/Preview.java line 218:

216: return Errors.PreviewFeatureDisabledClassfile(classfile, majorVersionToSource.get(majorVersion).name); 217: } 218:

Up above in isPreview, lines 185-188, I'm supervised it's not a `switch` statement. (Can't annotate those lines directly) src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java line 89:

87: @Override 88: protected Content getDeprecatedOrPreviewLink(Element member) { 89: Content content = new ContentBuilder();

Yeah the shorter name is good here and more in keeping with the code style src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java line 93:

91: if (!utils.isConstructor(member)) { 92: content.add("."); 93: content.add(member.getSimpleName());

this is OK, but generally FYI, `Content` is now set up to allow chained method calls. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassUseWriter.java line 436:

434: configuration, LinkInfoImpl.Kind.CLASS_USE_HEADER, typeElement) 435: .label(resources.getText("doclet.Class")) 436: .skipPreview(true));

@hns General comment, not directly related to this review: is it worth a cleanup to fold the sequence `getLink(new LinkInfoImpl(...))` into a hypothetical new `LinkBuilder` ? src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriterImpl.java line 221:

219: if (modifiers.endsWith(" ")) { 220: pre.add(" "); 221: }

Obligatory ugh that `modifiers` is a string and that it might end with a space. This is a possibility for future cleanup. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriterImpl.java line 283:

281: if (isFirst) { 282: pre.add(DocletConstants.NL); 283: pre.add("permits");

Note for javadoc-dev folk: we should have a better more consistent way of handling keywords src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HelpWriter.java line 254:

252: contentTree.add(section); 253: } 254:

👍 src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlConfiguration.java line 145:

143: // which performs a somewhat similar role 144: public enum ConditionalPage { 145: CONSTANT_VALUES, DEPRECATED, PREVIEW, SERIALIZED_FORM, SYSTEM_PROPERTIES

👍 src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/LinkFactoryImpl.java line 120:

118: title, 119: classLinkInfo.target)); 120: if (flags.contains(ElementFlag.PREVIEW)) {

I see a lot of `new HtmlTree(TagName.SUP).add(...)` ... enough to warrant a new factory method ``HtmlTree.SUP(Content)` or similar. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ModuleIndexWriter.java line 39:

37: import jdk.javadoc.internal.doclets.formats.html.markup.HtmlStyle; 38: import jdk.javadoc.internal.doclets.formats.html.markup.HtmlTree; 39: import jdk.javadoc.internal.doclets.formats.html.markup.RawHtml;

Are these imports still required? src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PreviewListWriter.java line 40:

38: 39: import com.sun.source.doctree.DocTree; 40: import javax.lang.model.element.ElementKind;

unordered imports src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PreviewListWriter.java line 223:

221: case CONSTRUCTOR -> new ConstructorWriterImpl(this); 222: case ENUM_CONSTANT -> new EnumConstantWriterImpl(this); 223: case ANNOTATION_TYPE_MEMBER -> new AnnotationTypeOptionalMemberWriterImpl(this, null);

Hmmm. Not sure about this one. For better or worse, javadoc current handles optional and required members differently (i.e. with different classes). That's arguably something that should be cleaned up at some point, but in the meantime, I'm surprised to not see the distinction here. I guess if we're not likely to see an interaction between preview-ness and annotation-members, this will not likely show up in practice. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PreviewListWriter.java line 67:

65: public class PreviewListWriter extends SubWriterHolderWriter { 66: 67: private String getAnchorName(PreviewElementKind kind) {

1. I'm mildly surprised to not see anything record-related here. 2. I'm mildly surprised you haven't used the new switch-expression syntax. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/PreviewAPIListBuilder.java line 67:

65: ENUM_CONSTANT, 66: ANNOTATION_TYPE_MEMBER // no ElementKind mapping 67: };

Should there be RECORD and RECORD_COMPONENT in this list? src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlTree.java line 932:

930: switch (tagName) { 931: case A: case BUTTON: case BR: case CODE: case EM: case I: case IMG: 932: case LABEL: case SMALL: case SPAN: case STRONG: case SUB: case SUP:

See comment elsewhere about possibly adding a factory method for `SUP` trees. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties line 290:

288: doclet.Declared_Using_Preview.SEALED_PERMITS=Sealed Classes 289: doclet.PreviewPlatformLeadingNote={0} is a preview API of the Java platform. 290: doclet.ReflectivePreviewPlatformLeadingNote={0} is a reflective preview API of the Java platform.

Question: is the following "inconsistency" deliberate: 1. Java platform 2. Java language 3. Java SE (in javac diagnostics) src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/FieldWriter.java line 75:

73: 74: /** 75: * Add the preview output for the given member.

(minor) Should be "Adds" src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/MethodWriter.java line 75:

73: 74: /** 75: * Add the preview output for the given member.

(minor) should be "Adds" src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/builders/AnnotationTypeRequiredMemberBuilder.java line 156:

154: buildSignature(annotationDocTree); 155: buildDeprecationInfo(annotationDocTree); 156: buildPreviewInfo(annotationDocTree);

(Just checking) I presume this behavior is inherited into `AnnotationTypeOptionalMemberBuilder` so no changes required there. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/builders/EnumConstantBuilder.java line 166:

164: writer.addPreview(currentElement, enumConstantsTree); 165: } 166:

Note to self: I regret that this duplication of adding this method everywhere seems to be necessary. This would be good to clean up at some point. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/doclets.properties line 213:

211: doclet.Preview=Preview. 212: doclet.Properties=Properties 213: doclet.constructors=constructors

Is the period after `Preview` intentional? It seems inconsistent. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Comparators.java line 131:

129: /** 130: * Returns a Comparator for deprecated items listed on deprecated list page, by comparing the 131: * fully qualified names, and if those are equal the names of the enclosing modules.

Comment does not match method name src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java line 132:

130: import static com.sun.source.doctree.DocTree.Kind.*; 131: import javax.lang.model.AnnotatedConstruct; 132: import javax.lang.model.util.SimpleAnnotationValueVisitor14;

unordered imports test/langtools/jdk/javadoc/doclet/testPreview/TestPreview.java line 75:

73: </div>"""; 74: String expected = MessageFormat.format(expectedTemplate, zero, one, two, three); 75: checkOutput("m/pkg/TestPreviewDeclaration.html", true, expected);

Interesting technique... Future: it might be interesting to consider folding the bundle access on line 59 into a utility call in JavadocTester test/langtools/jdk/javadoc/doclet/testPreview/api/preview/Core.java line 28:

26: import jdk.internal.javac.PreviewFeature.Feature; 27: 28: @PreviewFeature(feature=Feature.TEST)

Yeah, I remember `Feature.TEST` from earlier. I guess it's OK for now, as a workaround for a testing a feature which is inherently, by design, a moving target across releases. These days, javadoc tests are trending towards generating small sample test programs, instead of having small static side-files dominated by a legal header. I wonder if there is a possibility of having a "generator class" in the `javadoc.tester` package that can generate sample code using one or more of the current set of preview features, as a way of reducing the need for the TEST feature. test/langtools/tools/javac/patterns/BindingsTest2.out line 54:

52: - compiler.note.preview.filename: BindingsTest2.java, DEFAULT 53: - compiler.note.preview.recompile 54: 51 errors

(issue unrelated to review) it seems wrong that we encourage/allow the use of files that do not end in newline. test/langtools/tools/javac/preview/PreviewAutoSuppress.java line 25:

23: 24: /* 25: * @test

no `@bug`? test/langtools/tools/javac/preview/PreviewErrors.java line 38:

36: * @build toolbox.ToolBox toolbox.JavacTask 37: * @build combo.ComboTestHelper 38: * @compile PreviewErrors.java

do you need `@compile` here: is `@build` not good enough? ------------- PR: https://git.openjdk.java.net/jdk/pull/703

On Mon, 2 Nov 2020 19:37:39 GMT, Jonathan Gibbons <jjg@openjdk.org> 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.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/builders/AnnotationTypeRequiredMemberBuilder.java line 156:

...

154: buildSignature(annotationDocTree); 155: buildDeprecationInfo(annotationDocTree); 156: buildPreviewInfo(annotationDocTree);

(Just checking) I presume this behavior is inherited into `AnnotationTypeOptionalMemberBuilder` so no changes required there.

Right, I think so. ------------- PR: https://git.openjdk.java.net/jdk/pull/703

On Mon, 2 Nov 2020 19:59:10 GMT, Jonathan Gibbons <jjg@openjdk.org> 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

test/langtools/jdk/javadoc/doclet/testPreview/api/preview/Core.java line 28:

...

26: import jdk.internal.javac.PreviewFeature.Feature; 27: 28: @PreviewFeature(feature=Feature.TEST)

Yeah, I remember `Feature.TEST` from earlier. I guess it's OK for now, as a workaround for a testing a feature which is inherently, by design, a moving target across releases.

These days, javadoc tests are trending towards generating small sample test programs, instead of having small static side-files dominated by a legal header. I wonder if there is a possibility of having a "generator class" in the `javadoc.tester` package that can generate sample code using one or more of the current set of preview features, as a way of reducing the need for the TEST feature.

I have intentionally added Feature.TEST to improve testability. Before, tests were using one of the constants (typically whatever was the first constant), but that seems somewhat problematic - what if (at some point, transiently) we have no preview features? ------------- PR: https://git.openjdk.java.net/jdk/pull/703

On Wed, 4 Nov 2020 19:40:33 GMT, Jan Lahoda <jlahoda@openjdk.org> wrote:

...

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.

Thanks @jonathan-gibbons for your comments! I've tried to update the code based on them, mostly in https://github.com/lahodaj/jdk/commit/743f516c660b577035cdda4510a0bb97937fd9... and https://github.com/lahodaj/jdk/commit/e4b02827998fc2e8f19f983aabfb3d720b03d1...

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... ------------- PR: https://git.openjdk.java.net/jdk/pull/703

On 5 Nov 2020, at 18:11, Alex Buckley <alex.buckley@oracle.com> wrote:

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).

Very cool! ( I didn’t notice this until you pointed it out ;-) ) http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.01/api/preview-list.... <http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.01/api/preview-list.html> There appears to be a very minor bug; when I click on the PREVIEW link in the banner, the page that lists the preview API points loads, but the banner does not “highlight” that PREVIEW is “selected”, but rather it “highlights” DEPRECATED. -Chris.

On Fri, 6 Nov 2020 15:32:16 GMT, Jonathan Gibbons <jjg@openjdk.org> 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

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...

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.buckley@oracle.com> on kulla-dev <mailto:kulla-dev@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$>.

Thanks for noticing Chris and @jonathan-gibbons. Fixed in 61264fe, updated javadoc: http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.02/api/preview-list.... ------------- PR: https://git.openjdk.java.net/jdk/pull/703

On Fri, 6 Nov 2020 18:41:15 GMT, Jan Lahoda <jlahoda@openjdk.org> 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/jav... [3] http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.compiler... [4] http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/preview-list.... [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

On Tue, 8 Dec 2020 19:35:03 GMT, Jan Lahoda <jlahoda@openjdk.org> 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/jav... [3] http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.compiler... [4] http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/preview-list.... [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 55 commits:

- 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 - Reflecting review comments. - Removing trailing whitespace. - ... and 45 more: https://git.openjdk.java.net/jdk/compare/044616bd...0c1c4d57

Build changes still good. ------------- Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/703

On Fri, 8 Jan 2021 01:58:07 GMT, Jonathan Gibbons <jjg@openjdk.org> 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