On Thu, 12 Oct 2023 20:42:50 GMT, Vicente Romero <vrom...@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
