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

[Aggelos Biboudis]

On Thu, 3 Jul 2025 09:13:30 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> A commit in the implementation of JEP 455, https://github.com/openjdk/jdk/pull/15638/commits/ceee1e4c08457a0793fdfb556db99e057a947af1, added redundant synthetic local variable for trivial `instanceof int`-ish type conversion operations in the javac AST. Such conversions have been present since the introduction of record patterns, and previously they consistently lower to the part before `instanceof`. With this change, the introduced redundant variable is visible in the class file, as seen in the JBS issue.
>> 
>> Testing: langtools/tools/javac
>
> 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:


    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 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;
    }

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

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