On Fri, 3 Nov 2023 14:19:17 GMT, Rémi Forax <fo...@openjdk.org> wrote:
>> Jan Lahoda has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Some more get->orElseThrow
>> - Reflecting review feedback.
>
> src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 397:
>
>> 395: cb.iload(1);
>> 396: Label dflt = cb.newLabel();
>> 397: record Element(Label target, Label next, Object
>> label) {}
>
> 'label' is not a Label, is there a better name to make the difference between
> the switch label and the bytecode label
Changed to `caseLabel`.
> src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 423:
>
>> 421: Label next = element.next();
>> 422: cb.labelBinding(element.target());
>> 423: if (element.label() instanceof Class<?>
>> classLabel) {
>
> It's too bad we can not use a switch on the label here instead of a bunch of
> instanceof :)
Well, we could, if we tweaked javac so that it would produce a
different/simplified code for java.base for pattern switches (i.e. hardcode an
if-else cascade). Which we may or may not do at some point.
> src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 425:
>
>> 423: if (element.label() instanceof Class<?>
>> classLabel) {
>> 424: cb.aload(0);
>> 425:
>> cb.instanceof_(classLabel.describeConstable().get());
>
> You should use orElseThrow() (see the javadoc) instead of get(). This code
> does not work because hidden class Class are not denotable. My first idea was
> that it was not a real problem because no code generated by javac will use an
> hidden class but this is not true. A type pattern can use the current class
> and this class can be loaded by a defineHiddenClass.
>
> A way to solve that issue is to store the non denotable classes inside a list
> and send that list as a class data when calling defineHiddenClass.
Done (both `orElseThrow()` and the hidden classes).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381873158
PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381879214
PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381879791
