RFR: 8357185: Redundant local variables with unconditionally matching primitive patterns [v2]

[Jan Lahoda]

On Thu, 3 Jul 2025 15:31:01 GMT, Chen Liang <liach 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
>
> Chen Liang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Further improvements and testing per suggestions

Overall, looks reasonable to me. A stylistic comment for consideration inline.

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java line 2834:

> 2832:     public void visitTypeTest(JCInstanceOf tree) {
> 2833:         if (tree.expr.type.isPrimitive() || tree.pattern.type.isPrimitive()) {
> 2834:             JCStatement sideEffect;

As a mostly stylistic comment: if I were doing this, I would create a variable like: `List<JCStatement> statements;` (or any other name), and then would assign either `List.nil()` or `List.of(...)` to it. That way there would be no `null` checks later.

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

PR Review: https://git.openjdk.org/jdk/pull/26107#pullrequestreview-2987974182
PR Review Comment: https://git.openjdk.org/jdk/pull/26107#discussion_r2185851259