RFR: 8303374: Compiler Implementation for Primitive types in patterns, instanceof, and switch (Preview) [v19]

[Aggelos Biboudis]

On Thu, 12 Oct 2023 20:42:50 GMT, Vicente Romero <vromero at openjdk.org> wrote:

>> Aggelos Biboudis has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix duplicate method name and add a new test in PrimitivePatternsSwitchErrors
>
> src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 366:
> 
>> 364:             (selectorType.equals(int.class) && targetType.equals(Integer.class)))
>> 365:             return true;
>> 366:         else if (selectorType.equals(targetType) || (selectorType.equals(byte.class) && !targetType.equals(char.class)  ||
> 
> this code reminds me of some tricks we did to simplify similar code in the past, see `com.sun.tools.javac.code.TypeTag` and related:  `com.sun.tools.javac.code.TypeTag.NumericClasses` and method `com.sun.tools.javac.code.TypeTag::isSubRangeOf` and how it is used to determine if a numerical type is subtype of another numerical type. These tricks could be reused here, but up to you. I could also be done in a future refactoring.

Thanks for the recommendation. I used them and ported the subrange functionality to Wrappers (added two extra parameters for the enum variants).

> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java line 5055:
> 
>> 5053:         return (source.isPrimitive() && target.isPrimitive()) &&
>> 5054:                 ((source.hasTag(BYTE) && !target.hasTag(CHAR) ||
>> 5055:                         (source.hasTag(SHORT) && (target.hasTag(INT) || target.hasTag(LONG) || target.hasTag(FLOAT) || target.hasTag(DOUBLE)))||
> 
> this code can also be simplified as suggested for similar code in SwitchBootstraps, actually here you have access to the infra already in TypeTag and can leverage it.

Done.

> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java line 2910:
> 
>> 2908:     }
>> 2909: 
>> 2910:     public void visitTypeTest(JCInstanceOf tree) {
> 
> would it make sense to document how the generated code should look like?

Done!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15638#discussion_r1361010084
PR Review Comment: https://git.openjdk.org/jdk/pull/15638#discussion_r1361013492
PR Review Comment: https://git.openjdk.org/jdk/pull/15638#discussion_r1361010752