Hi Remi,

Thank you for reviewing the ASM changes.

Harold

On 5/19/2020 4:41 AM, Remi Forax wrote:

----- Mail original -----
De: "David Holmes" <david.hol...@oracle.com>
À: "Harold David Seigel" <harold.sei...@oracle.com>, "hotspot-runtime-dev" 
<hotspot-runtime-...@openjdk.java.net>,
"amber-dev" <amber-...@openjdk.java.net>, "core-libs-dev" 
<core-libs-...@openjdk.java.net>, "serviceability-dev"
<serviceability-dev@openjdk.java.net>
Envoyé: Mardi 19 Mai 2020 07:26:38
Objet: Re: RFR: JDK-8225056 VM support for sealed classes
Hi Harold,

Adding serviceability-dev for the serviceability related changes.

Nit: "VM support for sealed classes"

This RFR covers the VM, core-libs, serviceability and even some
langtools tests. AFAICS only the javac changes are not included here so
when and where will they be reviewed and under what bug id? Ideally
there will be a single JBS issue for "Implementation of JEP 360: Sealed
types". It's okay to break up the RFRs across different areas, but it
should be done under one bug id.

Overall this looks good. I've looked at all files and mainly have some
style nits in various places. But there are some more significant items
below.

On 14/05/2020 7:09 am, Harold Seigel wrote:
Hi,

Please review this patch for JVM and Core-libs support for the JEP 360
Sealed Classes preview feature.  Code changes include the following:

   * Processing of the new PermittedSubclasses attribute to enforce that
     a class or interface, whose super is a sealed class or interface,
     must be listed in the super's PermittedSubclasses attribute.
   * Disallow redefinition of a sealed class or interface if its
     redefinition would change its PermittedSubclasses attribute.
   * Support API's to determine if a class or interface is sealed and, if
     it's sealed, return a list of its permitted subclasses.
   * asm support for the PermittedSubclasses attribute
I assume Remi is providing the upstream support in ASM? :) But also see
below ...

Currently the version of ASM used JDK already supports sealed classes but with 
the attribute named PermittedSubtypes instead of PermittedSubclasses.
This patch renames the attribute which is the right thing to do.
Then when JDK 15 will be released, we will release ASM 9 with full support of 
PermittedSubclasses, with the right method names, docs, etc, that will be then 
integrated in JDK 16.

So with my ASM hat, the changes to the 3 ASM classes are good.

Rémi

Reply via email to