8260517: implement Sealed Classes as a standard feature in Java #3526

vicente-romero-oracle · 2021-04-16T01:08:57Z

Please review this PR that intents to make sealed classes a final feature in Java. This PR contains compiler and VM changes. In line with similar PRs, which has made preview features final, this one is mostly removing preview related comments from APIs plus options in test cases. Please also review the related CSR

Thanks

note: this PR is related to PR-3528 and must be integrated after it.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8260517: implement Sealed Classes as a standard feature in Java

Reviewers

David Holmes (@dholmes-ora - Reviewer) ⚠️ Review applies to 6e2a99c
Maurizio Cimadamore (@mcimadamore - Reviewer) ⚠️ Review applies to 8ebe56f
Jan Lahoda (@lahodaj - Reviewer) ⚠️ Review applies to 304fd76

Contributors

Harold Seigel <hseigel@openjdk.org>
Vicente Romero <vromero@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/3526/head:pull/3526
$ git checkout pull/3526

Update a local copy of the PR:
$ git checkout pull/3526
$ git pull https://git.openjdk.java.net/jdk pull/3526/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 3526

View PR using the GUI difftool:
$ git pr show -t 3526

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/3526.diff

bridgekeeper · 2021-04-16T01:09:42Z

👋 Welcome back vromero! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2021-04-16T01:11:30Z

@vicente-romero-oracle The following labels will be automatically applied to this pull request:

build
compiler
core-libs
hotspot-runtime
serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

mlbridge · 2021-04-16T01:15:58Z

Webrevs

vicente-romero-oracle · 2021-04-16T01:29:52Z

/csr

openjdk · 2021-04-16T01:30:49Z

@vicente-romero-oracle this pull request will not be integrated until the CSR request JDK-8265090 for issue JDK-8260517 has been approved.

vicente-romero-oracle · 2021-04-16T02:07:33Z

/contributor add Harold Seigel hseigel@openjdk.org
/contributor add Vicente Romero vromero@openjdk.org

openjdk · 2021-04-16T02:08:09Z

@vicente-romero-oracle Could not parse Harold Seigel hseigel@openjdk.org as a valid contributor.
Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

/contributor add @openjdk-bot
/contributor add duke
/contributor add J. Duke <duke@openjdk.org>

openjdk · 2021-04-16T02:08:19Z

@vicente-romero-oracle Could not parse Vicente Romero vromero@openjdk.org as a valid contributor.
Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

/contributor add @openjdk-bot
/contributor add duke
/contributor add J. Duke <duke@openjdk.org>

dholmes-ora

Hi Vicente,

Hotspot and hotspot tests all look fine. One query: why was this test removed?

test/hotspot/jtreg/runtime/sealedClasses/AbstractSealedTest.java

is that functionality tested elsewhere? (The other deleted test seemed obviously trivial.)

Thanks,
David

dholmes-ora · 2021-04-16T02:01:23Z

src/hotspot/share/classfile/classFileParser.cpp

@@ -3919,8 +3913,8 @@ void ClassFileParser::parse_classfile_attributes(const ClassFileStream* const cf
            parsed_record_attribute = true;
            record_attribute_start = cfs->current();
            record_attribute_length = attribute_length;
-          } else if (tag == vmSymbols::tag_permitted_subclasses()) {
-            if (supports_sealed_types()) {
+          } else if (_major_version >= JAVA_17_VERSION) {


Can you update the comment at L3932 to say JAVA_17_VERSION please.

dholmes-ora · 2021-04-16T02:01:55Z

src/hotspot/share/classfile/classFileParser.hpp

@@ -341,9 +341,6 @@ class ClassFileParser {
                                      const u1* const record_attribute_start,
                                      TRAPS);

-  bool supports_sealed_types();
-  bool supports_records();


Good catch!

vicente-romero-oracle · 2021-04-16T03:23:12Z

Hi Vicente,

Hotspot and hotspot tests all look fine. One query: why was this test removed?

test/hotspot/jtreg/runtime/sealedClasses/AbstractSealedTest.java

is that functionality tested elsewhere? (The other deleted test seemed obviously trivial.)

Thanks,
David

Hi David, thanks for your comments, yes regarding test test/hotspot/jtreg/runtime/sealedClasses/AbstractSealedTest.java, it was removed because the functionality is tested in test/langtools/tools/javac/sealed/SealedCompilationTests.java

vicente-romero-oracle · 2021-04-16T03:28:22Z

/contributor add Harold Seigel hseigel@openjdk.org
/contributor add Vicente Romero vromero@openjdk.org

openjdk · 2021-04-16T03:28:31Z

@vicente-romero-oracle
Contributor Harold Seigel <hseigel@openjdk.org> successfully added.

openjdk · 2021-04-16T03:30:18Z

@vicente-romero-oracle
Contributor Vicente Romero <vromero@openjdk.org> successfully added.

mcimadamore

Compiler changes look good (I have not checked SymbolGenerator).

Why were some tests removed?

vicente-romero-oracle · 2021-04-21T15:10:52Z

Compiler changes look good (I have not checked SymbolGenerator).

Why were some tests removed?

thanks for the review. The removed tests were already covered in langtools regression tests, so I only removed duplicated tests

…t is needed by the build

lahodaj

javac changes look good to me.

openjdk · 2021-05-13T21:52:27Z

@vicente-romero-oracle This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8260517: implement Sealed Classes as a standard feature in Java

Co-authored-by: Harold Seigel <hseigel@openjdk.org>
Co-authored-by: Vicente Romero <vromero@openjdk.org>
Reviewed-by: dholmes, mcimadamore, jlahoda

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

vicente-romero-oracle · 2021-05-20T09:10:00Z

/integrate

openjdk · 2021-05-20T09:11:35Z

@vicente-romero-oracle Since your change was applied there have been 3 commits pushed to the master branch:

31b98e1: 8265319: implement Sealed Classes as a standard feature in Java, javax.lang.model changes
726785b: 8267155: runtime/os/TestTracePageSizes times out
8e3549f: 8266332: Adler32 intrinsic for x86 64-bit platforms

Your commit was automatically rebased without conflicts.

Pushed as commit 0fa9223.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

vicente-romero-oracle added 5 commits April 13, 2021 12:14

8260517: Compiler implementation for Sealed Classes

696b2a5

JVM related changes

53e5026

fixing failing regression tests

987859f

Merge branch 'master' into JDK-8260517

36234de

Merge branch 'master' into JDK-8260517

6e2a99c

openjdk bot added the rfr Pull request is ready for review label Apr 16, 2021

openjdk bot added serviceability serviceability-dev@openjdk.org hotspot-runtime hotspot-runtime-dev@openjdk.org build build-dev@openjdk.org core-libs core-libs-dev@openjdk.org compiler compiler-dev@openjdk.org labels Apr 16, 2021

vicente-romero-oracle changed the title ~~8260517: implement Sealed Classes as a standard feature~~ 8260517: implement Sealed Classes as a standard feature in Java Apr 16, 2021

openjdk bot added the csr Pull request needs approved CSR before integration label Apr 16, 2021

vicente-romero-oracle mentioned this pull request Apr 16, 2021

8265319: implement Sealed Classes as a standard feature in Java, javax.lang.model changes #3528

Closed

3 tasks

removing javax.lang.model changes

5aef510

dholmes-ora approved these changes Apr 16, 2021

View reviewed changes

updating comment after review feedback

8ebe56f

mcimadamore approved these changes Apr 21, 2021

View reviewed changes

vicente-romero-oracle added 3 commits April 22, 2021 22:10

Merge branch 'master' into JDK-8260517

43e9cb5

Merge branch 'master' into JDK-8260517

2744f61

restoring jdk.internal.javac.PreviewFeature.Feature.SEALED_CLASSES, i…

304fd76

…t is needed by the build

lahodaj approved these changes May 3, 2021

View reviewed changes

Merge branch 'master' into JDK-8260517

0208dcf

openjdk bot removed the csr Pull request needs approved CSR before integration label May 13, 2021

openjdk bot added the ready Pull request is ready to be integrated label May 13, 2021

Merge branch 'master' into JDK-8260517

d220bc2

openjdk bot closed this May 20, 2021

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels May 20, 2021

This was referenced May 20, 2021

Create a Java 17 distribution jOOQ/jOOQ#11729

Closed

scala.tools.nsc.javac.JavaParsers can't parse JDK17 "Sealed Classes" scala/bug#12159

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8260517: implement Sealed Classes as a standard feature in Java #3526

8260517: implement Sealed Classes as a standard feature in Java #3526

vicente-romero-oracle commented Apr 16, 2021 •

edited by openjdk bot

Loading

bridgekeeper bot commented Apr 16, 2021

openjdk bot commented Apr 16, 2021

mlbridge bot commented Apr 16, 2021 •

edited

Loading

vicente-romero-oracle commented Apr 16, 2021

openjdk bot commented Apr 16, 2021

vicente-romero-oracle commented Apr 16, 2021 •

edited

Loading

openjdk bot commented Apr 16, 2021

openjdk bot commented Apr 16, 2021

dholmes-ora left a comment •

edited

Loading

dholmes-ora Apr 16, 2021 •

edited

Loading

vicente-romero-oracle Apr 16, 2021

dholmes-ora Apr 16, 2021

vicente-romero-oracle commented Apr 16, 2021

vicente-romero-oracle commented Apr 16, 2021

openjdk bot commented Apr 16, 2021

openjdk bot commented Apr 16, 2021

mcimadamore left a comment

vicente-romero-oracle commented Apr 21, 2021

lahodaj left a comment

openjdk bot commented May 13, 2021 •

edited

Loading

vicente-romero-oracle commented May 20, 2021

openjdk bot commented May 20, 2021

8260517: implement Sealed Classes as a standard feature in Java #3526

8260517: implement Sealed Classes as a standard feature in Java #3526

Conversation

vicente-romero-oracle commented Apr 16, 2021 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Contributors

Reviewing

bridgekeeper bot commented Apr 16, 2021

openjdk bot commented Apr 16, 2021

mlbridge bot commented Apr 16, 2021 • edited Loading

Webrevs

vicente-romero-oracle commented Apr 16, 2021

openjdk bot commented Apr 16, 2021

vicente-romero-oracle commented Apr 16, 2021 • edited Loading

openjdk bot commented Apr 16, 2021

openjdk bot commented Apr 16, 2021

dholmes-ora left a comment • edited Loading

Choose a reason for hiding this comment

dholmes-ora Apr 16, 2021 • edited Loading

Choose a reason for hiding this comment

vicente-romero-oracle Apr 16, 2021

Choose a reason for hiding this comment

dholmes-ora Apr 16, 2021

Choose a reason for hiding this comment

vicente-romero-oracle commented Apr 16, 2021

vicente-romero-oracle commented Apr 16, 2021

openjdk bot commented Apr 16, 2021

openjdk bot commented Apr 16, 2021

mcimadamore left a comment

Choose a reason for hiding this comment

vicente-romero-oracle commented Apr 21, 2021

lahodaj left a comment

Choose a reason for hiding this comment

openjdk bot commented May 13, 2021 • edited Loading

vicente-romero-oracle commented May 20, 2021

openjdk bot commented May 20, 2021

vicente-romero-oracle commented Apr 16, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Apr 16, 2021 •

edited

Loading

vicente-romero-oracle commented Apr 16, 2021 •

edited

Loading

dholmes-ora left a comment •

edited

Loading

dholmes-ora Apr 16, 2021 •

edited

Loading

openjdk bot commented May 13, 2021 •

edited

Loading