RFR: 8357185: Redundant local variables with unconditionally matching primitive patterns

[Chen Liang]

On Thu, 3 Jul 2025 09:24:33 GMT, Aggelos Biboudis <abimpoudis at openjdk.org> wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java line 2846:
>> 
>>> 2844: 
>>> 2845:             if (types.isUnconditionallyExact(tree.expr.type, tree.pattern.type)) {
>>> 2846:                 var = make.Exec(instanceOfExpr);  // drop redundant variable
>> 
>> While this is sufficient to solve the issue, it might be good to go through the if cascade more thoroughly, and only introduce the variable if needed. I.e. in case the value is used more than once. If the value is used only once (like in the `else if (tree.expr.type.isPrimitive()) {`, it ought to be possible to use it directly. Unless there are ordering or other concerns, of course.
>
> On the flipside, in the tests we already have this (in `PrimitiveInstanceOfPatternOpWithTopLevelPatterns` but also as a type comparison op in `PrimitiveInstanceOfTypeComparisonOp`):
> 
> 
>     public static int meth() {return 42;}
>     public static boolean exprMethod() {
>         return meth() instanceof int ii;
>     }
> 
> 
> However this is not sufficient. Can we also add one (one in each file) that actually has side effects and a null check as well and confirm that i is incremented once?
> 
> 
>     static int i = 0;
>     public static Integer meth_sideEffect() {  i++; return 42;}
>     public static boolean exprMethod2() {
>         return meth_sideEffect() instanceof int ii;
>     }

Do I just paste this block into both tests and call them in main?

    static int sideEffect;
    public static Integer methSideEffect() { sideEffect++; return 42;}
    public static boolean exprMethodSideEffect() {
        sideEffect = 5;
        return methSideEffect() instanceof int ii && sideEffect == 6;
    }

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26107#discussion_r2182970222