RFR: 8246778: Compiler implementation for Sealed Classes (Second Preview)

[Harold Seigel]

On Tue, 1 Dec 2020 23:16:45 GMT, Mandy Chung <mchung at openjdk.org> wrote:

>> This pull request replaces https://github.com/openjdk/jdk/pull/1227.
>> 
>> From the original PR:
>> 
>>> 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
>>> 
>>>     * renaming Class::permittedSubclasses to Class::getPermittedSubclasses, still in the same method, the return type has been changed to Class<?>[] instead of the previous ClassDesc[]
>>> 
>>>     * adding code to make sure that annotations can't be sealed
>>> 
>>>     * improving some tests
>>> 
>>> 
>>> TIA
>>> 
>>> Related specs:
>>> [Sealed Classes JSL](http://cr.openjdk.java.net/~gbierman/jep397/jep397-20201104/specs/sealed-classes-jls.html)
>>> [Sealed Classes JVMS](http://cr.openjdk.java.net/~gbierman/jep397/jep397-20201104/specs/sealed-classes-jvms.html)
>>> [Additional: Contextual Keywords](http://cr.openjdk.java.net/~gbierman/jep397/jep397-20201104/specs/contextual-keywords-jls.html)
>> 
>> This PR strives to reflect the review comments from 1227:
>>  * adjustments to javadoc of j.l.Class methods
>>  * package access checks in Class.getPermittedSubclasses()
>>  * fixed to the narrowing conversion/castability as pointed out by Maurizio
>
> From David's comment:
> 
>> These subtleties are what I was also getting at in the CSR discussion. If Foo lists Bar as a permitted subtype, but Bar does not extend Foo, is that an error? Should Bar not be returned by getPermittedSubclasses? The answer depends on exactly how you specify getPermittedSubclasses() and whether that aligns with reading the classfile attribute, or actually checking the runtime relationships. 
> 
> As I read the current spec of `Class::getPermittedSubclasses`, it intends to return the direct subclasses or subinterfaces permitted to extend or implement this class or interface if it's sealed.   If not, it should be clarified that it is the class file view.
> 
> `NestMembers` and `PermittedSubclasses` attributes are critical to correct interpretation of the class file by JVM.   Prior to these two attributes, the attributes inspected by core reflection APIs are all non-critical.  API like `Class::getDeclaringClass` reads `InnerClasses` attribute if present in order to determine its declaring class but the current spec does not specify the behavior on error cases (which I consider a spec bug - see JDK-8250226.)
> 
> IMO it is reasonable for `getPermittedSubclasses` (and `getNestMembers` and `getNestHost`) to return the runtime view as it returns `Class` objects since these attributes are critical to correct interpretation of the class file.  It would be confusing if it returns `Foo` that is not a permitted subtype at runtime.

Additional changes may be needed to Class.permittedSubclasses() and/or Class.isSealed() as part of fixing bug JDK-8256867.  The JVM is being changed to treat classes with empty PermittedSubclasses attributes as sealed classes that cannot be extended (or implemented).

Current thinking is that Class.permittedSubclasses() will return an empty array for both non-sealed classes and for sealed classes with empty PermittedSubclasses attributes.  And, Class.isSealed() will return False in the former case and True in the latter.  This will require changing the implementation of Class.isSealed() to call the JVM directly instead of calling Class.permittedSubclasses().

Does this seem like a reasonable way to handle this corner case?

-------------

PR: https://git.openjdk.java.net/jdk/pull/1483