> On Oct 27, 2020, at 10:56 AM, Chris Hegarty <chris.hega...@oracle.com> wrote: > > Gavin, > > Comments relating to the JVMS: > > (Many of my comments relate to the wellformedness of the PermittedSubclasses > Attribute, which may or may not be strictly specified, so take with a pinch > of salt! ;-) ) > > I find myself mapping the set of allowable values in the PermittedSubclasses > Attribute to the behaviour of the Core Reflection API, to discern; what is > enforced by the VM and what is possible to be observed reflective at run-time. > > The `number_of_classes` is not required to be a non-zero, but I expect that > it should. Or maybe this is implicit or specified/enforced elsewhere?
The language requires the 'permits' clause to be non-empty, but there's no such JVM restriction. Naturally, if you permit no subclasses, nobody can extend you. (This is sort of like declaring a private constructor.) > "Array items that do not attempt to directly extend or implement the current > class or interface are ignored.” - Ok, so such items can be from any runtime > module or package, right? Related to `number_of_classes`, the requirement > is really that there is at least one non-ignored item, no? ( if enforced at > all ) Like 'NestMembers', you can put whatever garbage names you want in here. They'll just have no effect. The only validation is when a class asks permission to extend. (Separately, reflection can do whatever it thinks is best.) > The recent change (proposed on this list) to Class::getPermittedSubclasses, > means that it will no longer be possible to reflectively return permitted > subclasses that are not loaded, or more specifically “loadable" - the classes > must exist somewhere. Currently, in JDK 15, permittedSubclasses will return > class descriptors for non-loadable classes. I think that this is ok, we just > need to ensure that it fits into the other rules here. Yep. I think this is a good change, and I think there's nothing wrong with the reflection API either ignoring some entries or reporting class loading errors. Best to do whatever the NestMembers query does. > "C does not have its ACC_PUBLIC flag set (4.1) and the superclass belongs to > a different run-time package than C.” - I get that there is a bidirectional > accessibility relationship between the superclass and the subclass, but this > seems at odds with JEP text: “In particular, a subclass may be less > accessible than the sealed class”. Why is this not that the superclass must > have ACC_PUBLIC, and not the subclass? The goal of this rule is to simulate resolution without actually asking to perform it, since the class hasn't even been created yet. Part of resolution is access checking, so we simulate it here. The JEP says "a permitted subclass and its sealed superclass must be accessible to each other. However, permitted subclasses need not have the same accessibility as each other, or as the sealed class." Keep in mind we're talking now about language-level accessibility, which doesn't always map to JVM accessibility. Anyway, some examples: public super, non-public sub, same package package-access super, private sub, same package public *and exported* super, public sub private super, public sub, same nest > Duplicate entries in the `classes` array are effectively ignored? They must > be elided by Class::getPermittedSubclasses, right? Or can duplicates be > propagated through this API point? No JVM restrictions on this. Again, reflection should do something reasonable, probably matching what NestMembers does. > "Changing a class that is declared non-sealed to no longer be declared > non-sealed does not break compatibility with pre-existing binaries.” - > doesn’t this depend on the whether or not the superclass/superinterface is > sealed? And whether it has subclasses or not ? Agree, this seems like a mistake. If it's not `non-sealed`, it's becoming either `final` or `sealed`. Either way, that risks binary incompatibilities.
