Re: RFR: JDK-8227046: compiler implementation for sealed classes, JDK-8227047: Javadoc for sealed types and JDK-8227044: javax.lang.model for sealed classes
> 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 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".
That’s actually a bug in the spec (to be filed). I believe now the intent is to
treat non-sealed as a contextual keyword, which is what the compiler does.
Gavin
RFR: 8265130: Make ConstantDesc class hierarchy sealed
Hi all, Please review this patch to make the ConstantDesc hierarchy `sealed`, as was promised in its Javadoc, now that sealed classes are finalising in JDK 17. Thanks, Gavin - Commit messages: - 8265130: Make ConstantDesc class hierarchy sealed Changes: https://git.openjdk.java.net/jdk/pull/4135/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4135&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8265130 Stats: 25 lines in 6 files changed: 16 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/4135.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4135/head:pull/4135 PR: https://git.openjdk.java.net/jdk/pull/4135
Re: RFR: 8265130: Make ConstantDesc class hierarchy sealed [v2]
> Hi all, > > Please review this patch to make the ConstantDesc hierarchy `sealed`, as was > promised in its Javadoc, now that sealed classes are finalising in JDK 17. > > Thanks, > Gavin Gavin Bierman has updated the pull request incrementally with one additional commit since the last revision: Removing javadoc comments about future use of sealed - Changes: - all: https://git.openjdk.java.net/jdk/pull/4135/files - new: https://git.openjdk.java.net/jdk/pull/4135/files/8084e90b..1cd54624 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4135&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4135&range=00-01 Stats: 131 lines in 6 files changed: 102 ins; 29 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4135.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4135/head:pull/4135 PR: https://git.openjdk.java.net/jdk/pull/4135
Re: RFR: 8265130: Make ConstantDesc class hierarchy sealed [v3]
> Hi all, > > Please review this patch to make the ConstantDesc hierarchy `sealed`, as was > promised in its Javadoc, now that sealed classes are finalising in JDK 17. > > Thanks, > Gavin Gavin Bierman has updated the pull request incrementally with one additional commit since the last revision: Removing file constants.patch added by mistake - Changes: - all: https://git.openjdk.java.net/jdk/pull/4135/files - new: https://git.openjdk.java.net/jdk/pull/4135/files/1cd54624..c8f632f6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4135&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4135&range=01-02 Stats: 102 lines in 1 file changed: 0 ins; 102 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4135.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4135/head:pull/4135 PR: https://git.openjdk.java.net/jdk/pull/4135
Re: RFR: 8265130: Make ConstantDesc class hierarchy sealed [v2]
On Thu, 20 May 2021 21:32:08 GMT, Mandy Chung wrote: >> Gavin Bierman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Removing javadoc comments about future use of sealed > > src/java.base/share/classes/java/lang/constant/DynamicConstantDesc.java line > 59: > >> 57: * @since 12 >> 58: */ >> 59: non-sealed public abstract class DynamicConstantDesc > > can you explain why `DynamicConstantDesc` is non-sealed? It's a permitted subclass of ConstantDesc, so it must be either `final`, `sealed`, or `non-sealed`. Making it `non-sealed` means it can still be extended. - PR: https://git.openjdk.java.net/jdk/pull/4135
Re: RFR: 8265130: Make ConstantDesc class hierarchy sealed [v4]
> Hi all, > > Please review this patch to make the ConstantDesc hierarchy `sealed`, as was > promised in its Javadoc, now that sealed classes are finalising in JDK 17. > > Thanks, > Gavin Gavin Bierman has updated the pull request incrementally with one additional commit since the last revision: Reordering class modifiers - Changes: - all: https://git.openjdk.java.net/jdk/pull/4135/files - new: https://git.openjdk.java.net/jdk/pull/4135/files/c8f632f6..c36075d2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4135&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4135&range=02-03 Stats: 6 lines in 6 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/4135.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4135/head:pull/4135 PR: https://git.openjdk.java.net/jdk/pull/4135
Re: RFR: 8265130: Make ConstantDesc class hierarchy sealed [v3]
On Fri, 21 May 2021 03:26:49 GMT, liach wrote: >> Gavin Bierman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Removing file constants.patch added by mistake > > src/java.base/share/classes/java/lang/constant/ClassDesc.java line 56: > >> 54: * @since 12 >> 55: */ >> 56: sealed public interface ClassDesc > > Should move `sealed` behind `public` per the blessed modifier order from JLS. > https://docs.oracle.com/javase/specs/jls/se16/preview/specs/sealed-classes-jls.html#jls-8.1.1 > > Per http://openjdk.java.net/jeps/409 the finalized sealed classes have no > difference from that in 16, so the order suggested there should be valid. Thanks. Now changed. - PR: https://git.openjdk.java.net/jdk/pull/4135
Integrated: 8265130: Make ConstantDesc class hierarchy sealed
On Thu, 20 May 2021 21:07:04 GMT, Gavin Bierman wrote: > Hi all, > > Please review this patch to make the ConstantDesc hierarchy `sealed`, as was > promised in its Javadoc, now that sealed classes are finalising in JDK 17. > > Thanks, > Gavin This pull request has now been integrated. Changeset: 379376f0 Author:Gavin Bierman Committer: Vicente Romero URL: https://git.openjdk.java.net/jdk/commit/379376f0783facba93e1d11db9b184ef8183a13b Stats: 54 lines in 6 files changed: 16 ins; 29 del; 9 mod 8265130: Make ConstantDesc class hierarchy sealed Reviewed-by: mchung, jvernee, vromero - PR: https://git.openjdk.java.net/jdk/pull/4135
Re: RFR: 8262889: Compiler implementation for Record Patterns [v2]
On Fri, 6 May 2022 14:09:24 GMT, Jan Lahoda wrote:
>> 8262889: Compiler implementation for Record Patterns
>>
>> A first version of a patch that introduces record patterns into javac as a
>> preview feature. For the specification, please see:
>> http://cr.openjdk.java.net/~gbierman/jep427+405/jep427+405-20220426/specs/patterns-switch-record-patterns-jls.html
>>
>> There are two notable tricky parts:
>> -in the parser, it was necessary to improve the `analyzePattern` method to
>> handle nested/record patterns, while still keeping error recovery reasonable
>> -in the `TransPatterns`, the desugaring of the record patterns is very
>> straightforward - effectivelly the record patterns are desugared into
>> guards/conditions. This will likely be improved in some future
>> version/preview
>>
>> `MatchException` has been extended to cover additional cases related to
>> record patterns.
>
> Jan Lahoda has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Reflecting review feedback.
> From the JLS specdiff
>
> > If the type R names a generic record class then it is a compile-time error
> > if R is not a parameterized type.
>
> The following snippet raises a `MatchException`. Shouldn't it be a
> compile-time error?
>
> ```
> Box r = new Box<>(null);
>
> switch (r) {
> case Box(String s):
> System.out.println("match");
> }
> ```
>
> If this is Ok and my understanding is wrong, then why that raises an
> exception at all? I can make it work (as an unconditional) if I define the
> Box as `record Box` and now I am confused...
>
> ping @GavinBierman @lahodaj
A couple of issues here. (1) This should be a compile-time error. (2) upon
investigation I think there is a bug with the pattern matching code, because
the compiler is currently saying that the pattern match here: `Box bs =
new Box<>(null); if (bs instanceof Box(String s)) {
System.out.println("match!"); }` does not succeed. (It should do). The
`MatchException` you are seeing is that the exhaustive pattern switch has no
matching label (if you put in a default, you don't get the exception).
-
PR: https://git.openjdk.java.net/jdk/pull/8516
