RFR: JDK-8227046: compiler implementation for sealed classes, JDK-8227047: Javadoc for sealed types and JDK-8227044: javax.lang.model for sealed classes

[Jan Lahoda]

Hi Vicente,

javac changes look overall OK to me.

A few comments:
-the handling of non-sealed in JavacParser is fairly good, even though I 
wonder if handling it in the tokenizer would not be better. If I read 
the specification correctly, "non-sealed" is specified as a keyword, so 
the tokenizer would seem like a proper place to recognize it (when 
preview is enabled, etc.). So, I think this:
class NonSealed {
     {
         int non = 0;
         int sealed = 0;
         int c = non-sealed;
     }
}

should not compile, but it does. In any case, not sure if there are 
tests for broken/strange non-sealed, like "non -sealed", 
"non/**/-sealed", "non\u002Dsealed".

Also, the checks for 'non', '-', 'sealed' tokens is duplicated twice, it 
would be nicer to have this slightly complex code only once.

-parsing of permitted classes allows parameterized types, while it, I 
guess, should not. I.e. it appears javac compiles this without producing 
compile-time errors:
public sealed class Sealed1 permits I<String>  {
}
final class I<T> extends Sealed1 {}

-in ClassReader:
new AttributeReader(names.PermittedSubclasses, V58, CLASS_ATTRIBUTE)
that should use V59?

-I appreciate the error recovery for using "sealed/non-sealed" for local 
classes, but I wonder if there's something we could do when "sealed" is 
used without --enable-preview (for local and non-local 
classes/interfaces). Maybe if "sealed" is followed by another modifier, 
'@', 'class' or 'interface', we can produce a better errors? (But we can 
work on that later.)

-regarding TypeElement#getPermittedSubclasses: it currently returns 
List<? extends TypeMirror>; I wonder if returning List<? extends 
TypeElement> would be better?

-not sure if I prefer the "invalid permits clause" errors like:
---
DuplicateTypeInPermits.java:30: error: invalid permits clause
sealed class Sealed permits Sub, Sub {}
                             ^
   (must not contain duplicates: Sub)
---

something like:
---
DuplicateTypeInPermits.java:30: error: repeated permitted type
sealed class Sealed permits Sub, Sub {}
                                  ^
---

might work as well.

-nit: ClassTree#getPermitsClause() could use List.of(), instead of 
Collections.emptyList();

Thanks,
     Jan

On 19. 05. 20 0:42, Vicente Romero wrote:
> Hi,
> 
> Please review this patch for the compiler, javadoc and javax.lang.model 
> support for the JEP 360 Sealed Classes (Preview). The changes are 
> provided at [1], which implements the latest JLS for sealed types [2]. 
> The patch also include the needed changes to javadoc and 
> javax.lang.model to support sealed types. The CSR witht the changes in 
> the javax.lang.model spec is at [3]. The sealed types JEP is accessible 
> at [4]. There is an ongoing review for the VM and core-libs code of 
> sealed types [5] and that code hasn't been included in this webrev,
> 
> Thanks,
> Vicente
> 
> [1] http://cr.openjdk.java.net/~vromero/8227046/webrev.00/
> [2] 
> http://cr.openjdk.java.net/~gbierman/jep360/jep360-20200513/specs/sealed-classes-jls.html 
> 
> [3] https://bugs.openjdk.java.net/browse/JDK-8244367
> [4] https://openjdk.java.net/jeps/360
> [5] 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-May/066440.html