----- Mail original ----- > De: "David Holmes" <david.hol...@oracle.com> > À: "Remi Forax" <fo...@univ-mlv.fr> > Cc: "Harold David Seigel" <harold.sei...@oracle.com>, "Vicente Romero" > <vrom...@openjdk.java.net>, "compiler-dev" > <compiler-...@openjdk.java.net>, "core-libs-dev" > <core-libs-dev@openjdk.java.net>, "hotspot-dev" > <hotspot-...@openjdk.java.net> > Envoyé: Mardi 24 Novembre 2020 13:16:39 > Objet: Re: RFR: 8246778: Compiler implementation for Sealed Classes (Second > Preview)
> Hi Remi, > > On 24/11/2020 7:45 pm, Remi Forax wrote: >> ----- Mail original ----- >>> De: "David Holmes" <david.hol...@oracle.com> >>> À: "Harold David Seigel" <harold.sei...@oracle.com>, "Vicente Romero" >>> <vrom...@openjdk.java.net>, "compiler-dev" >>> <compiler-...@openjdk.java.net>, "core-libs-dev" >>> <core-libs-dev@openjdk.java.net>, "hotspot-dev" >>> <hotspot-...@openjdk.java.net> >>> Envoyé: Mardi 24 Novembre 2020 02:04:55 >>> Objet: Re: RFR: 8246778: Compiler implementation for Sealed Classes (Second >>> Preview) >> >>> Hi Harold, >>> >>> On 24/11/2020 6:27 am, Harold Seigel wrote: >>>> Hi David, >>>> >>>> Thanks for looking at this. >>>> >>>> The intent was for method Class.permittedSubclasses() to be implemented >>>> similarly to Class.getNestMembers(). Are you suggesting that a security >>>> manager check be added to permittedSubclasses() similar to the security >>>> manager check in getNestMembers()? >>> >>> No I'm suggesting the change to the API is plain wrong. :) Please see >>> discussion in the CSR. >> >> Given that the CSR is closed, i will answer here. >> There are two issues with the previous implementation of permittedSubclasses, >> first it's the only method that using method desc which means that people has >> to be aware on another non trivial concept (object that describes constant >> pool >> constant) to understand how to use the method then i've tested this API with >> my >> students, all but one what able to correctly derives the Class from a >> ClassDesc, so instead of asking every users of permittedSubclasses to go >> through the oops of getting Class from a ClassDesc, i think we can agree that >> it's better to move the burden from the user to the JDK implementors. > > Why is the objective to get the Class objects? What purpose does that > serve? The whole idea of the reflection is to provide the runtime view of Java the language. Even if such thing does not exist. > The original API provided a descriptor for the contents of the > permittedSubclasses attribute. I find it totally bizarre to have an API > whose role is now to attempt to load all the subclasses of a sealed class. It's as bizarre as Class.getClasses() loading all the member classes. You are discovering that the reflection API is bizarre, and it is :) It's not the view of the JVM, it's the view of Java at runtime, whatever it means. > > YMMV. > > David Rémi [1] https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Class.html#getClasses() > >>> >>> Cheers, >>> David >> >> regards, >> Rémi >> >>> >>>> Thanks, Harold >>>> >>>> On 11/18/2020 12:31 AM, David Holmes wrote: >>>>> Hi Vincente, >>>>> >>>>> On 16/11/2020 11:36 pm, Vicente Romero wrote: >>>>>> Please review the code for the second iteration of sealed classes. In >>>>>> this iteration we are: >>>>>> >>>>>> - Enhancing narrowing reference conversion to allow for stricter >>>>>> checking of cast conversions with respect to sealed type hierarchies. >>>>>> - Also local classes are not considered when determining implicitly >>>>>> declared permitted direct subclasses of a sealed class or sealed >>>>>> interface >>>>> >>>>> The major change here seems to be that getPermittedSubclasses() now >>>>> returns actual Class objects instead of ClassDesc. My recollection >>>>> from earlier discussions here was that the use of ClassDesc was very >>>>> deliberate as the permitted subclasses may not actually exist and >>>>> there may be security concerns with returning them! >>>>> >>>>> Cheers, >>>>> David >>>>> ----- >>>>> >>>>>> ------------- >>>>>> >>>>>> Commit messages: >>>>>> - 8246778: Compiler implementation for Sealed Classes (Second Preview) >>>>>> >>>>>> Changes: https://git.openjdk.java.net/jdk/pull/1227/files >>>>>> Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1227&range=00 >>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8246778 >>>>>> Stats: 589 lines in 12 files changed: 508 ins; 18 del; 63 mod >>>>>> Patch: https://git.openjdk.java.net/jdk/pull/1227.diff >>>>>> Fetch: git fetch https://git.openjdk.java.net/jdk >>>>>> pull/1227/head:pull/1227 >>>>>> > >>>>> PR: https://git.openjdk.java.net/jdk/pull/1227