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

2020-06-02 Thread Vicente Romero
thanks to all the reviewers, the code for sealed classes has already been pushed to mainline. Vicente On 5/18/20 6:42 PM, 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

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

2020-05-26 Thread Maurizio Cimadamore
Looks good. For the diagnostic, longer term it would be nice to generalize those: # 0: token, 1: token 2168 compiler.err.expected2=\ 2169 {0} or {1} expected 2170 2171 # 0: token, 1: token, 2: token 2172 compiler.err.expected3=\ 2173 {0}, {1}, or {2} expected 2174 2175 # 0: token, 1:

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

2020-05-25 Thread Vicente Romero
Hi Maurizio, Right good catch I forgot to add the MONKEY_AT to the list of expected tokens. I have fixed that. I have published another iteration at [1]. Apart from the MONKEY_AT issue addressed at the parser this iteration also:  - adds another error key to compiler.properties, this new key

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

2020-05-25 Thread Maurizio Cimadamore
Hi, changes look good, but the parser changes do not convince me. Now that the logic is more clearly defined, I see some issues e.g. there seems to be a tight coupling by where "sealed" or "non-sealed" is, and the fact that the following token must be e.g. "class". This is unlike other

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

2020-05-22 Thread Vicente Romero
Hi, Thanks Jan and Maurizio for the reviews. I have created another iteration that I hope addresses all of the comments. I recognize that dealing with `sealed`, `non-sealed` is the most complicated part as there is no guide right now from the spec. So I have tried to make them contextual

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

2020-05-21 Thread Maurizio Cimadamore
Hi Vicente, looks very good. Some comments below. * the parser logic is clever in its use of position to apply context-dependent keyword detection; as Jan says, perhaps just share the code so that the position checks are not repeated. * I found one very edge-case quirk in the

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

2020-05-19 Thread Gavin Bierman
> On 19 May 2020, at 13:44, Jan Lahoda wrote: > > 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

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

2020-05-19 Thread 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

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

2020-05-18 Thread Vicente Romero
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