8260593: javac can skip a temporary local variable when pattern matching over a local variable

Reviewed-by: vromero

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -26,13 +26,14 @@
 package com.sun.tools.javac.comp;
 
 import com.sun.tools.javac.code.Flags;
+import com.sun.tools.javac.code.Kinds;
+import com.sun.tools.javac.code.Kinds.Kind;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symbol.BindingSymbol;
 import com.sun.tools.javac.code.Symbol.VarSymbol;
 import com.sun.tools.javac.code.Symtab;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.Types;
-import com.sun.tools.javac.tree.JCTree;
 import com.sun.tools.javac.tree.JCTree.JCAssign;
 import com.sun.tools.javac.tree.JCTree.JCBinary;
 import com.sun.tools.javac.tree.JCTree.JCConditional;
@@ -49,7 +50,6 @@ import com.sun.tools.javac.tree.JCTree.JCWhileLoop;
 import com.sun.tools.javac.tree.JCTree.Tag;
 import com.sun.tools.javac.tree.TreeMaker;
 import com.sun.tools.javac.tree.TreeTranslator;
-import com.sun.tools.javac.util.Assert;
 import com.sun.tools.javac.util.Context;
 import com.sun.tools.javac.util.ListBuffer;
 import com.sun.tools.javac.util.Log;
@@ -68,6 +68,7 @@ import com.sun.tools.javac.tree.JCTree.JCDoWhileLoop;
 import com.sun.tools.javac.tree.JCTree.JCLambda;
 import com.sun.tools.javac.tree.JCTree.JCStatement;
 import com.sun.tools.javac.tree.JCTree.LetExpr;
+import com.sun.tools.javac.tree.TreeInfo;
 import com.sun.tools.javac.util.List;
 import java.util.HashMap;
 
@@ -152,15 +153,23 @@ public class TransPatterns extends TreeTranslator {
             //E instanceof T N
             //=>
             //(let T' N$temp = E; N$temp instanceof T && (N = (T) N$temp == (T) N$temp))
+            Symbol exprSym = TreeInfo.symbol(tree.expr);
             JCBindingPattern patt = (JCBindingPattern)tree.pattern;
             VarSymbol pattSym = patt.var.sym;
             Type tempType = tree.expr.type.hasTag(BOT) ?
                     syms.objectType
                     : tree.expr.type;
-            VarSymbol temp = new VarSymbol(pattSym.flags() | Flags.SYNTHETIC,
+            VarSymbol temp;
-                    names.fromString(pattSym.name.toString() + target.syntheticNameChar() + "temp"),
+            if (exprSym != null &&
-                    tempType,
+                exprSym.kind == Kind.VAR &&
-                    patt.var.sym.owner);
+                exprSym.owner.kind.matches(Kinds.KindSelector.VAL_MTH)) {
+                temp = (VarSymbol) exprSym;
+            } else {
+                temp = new VarSymbol(pattSym.flags() | Flags.SYNTHETIC,
+                        names.fromString(pattSym.name.toString() + target.syntheticNameChar() + "temp"),
+                        tempType,
+                        patt.var.sym.owner);
+            }
             JCExpression translatedExpr = translate(tree.expr);
             Type castTargetType = types.boxedTypeOrType(pattSym.erasure(types));
 
@@ -176,8 +185,10 @@ public class TransPatterns extends TreeTranslator {
                 nestedLE.setType(syms.booleanType);
                 result = makeBinary(Tag.AND, (JCExpression)result, nestedLE);
             }
-            result = make.at(tree.pos).LetExpr(make.VarDef(temp, translatedExpr), (JCExpression)result).setType(syms.booleanType);
+            if (temp != exprSym) {
-            ((LetExpr) result).needsCond = true;
+                result = make.at(tree.pos).LetExpr(make.VarDef(temp, translatedExpr), (JCExpression)result).setType(syms.booleanType);
+                ((LetExpr) result).needsCond = true;
+            }
         } else {
             super.visitTypeTest(tree);
         }

test/langtools/tools/javac/patterns/Annotations.java

@@ -70,8 +70,8 @@ public class Annotations extends JavacTestingAbstractProcessor {
                         codeAttr.attributes.map.get(Attribute.RuntimeInvisibleTypeAnnotations);
                 RuntimeInvisibleTypeAnnotations_attribute annotations =
                         (RuntimeInvisibleTypeAnnotations_attribute) annoAttr;
-                String expected = "[@Annotations$DTA; pos: [LOCAL_VARIABLE, {start_pc = 35, length = 7, index = 1}, pos = -1], " +
+                String expected = "[@Annotations$DTA; pos: [LOCAL_VARIABLE, {start_pc = 31, length = 7, index = 1}, pos = -1], " +
-                                  "@Annotations$TA; pos: [LOCAL_VARIABLE, {start_pc = 56, length = 7, index = 1}, pos = -1]]";
+                                  "@Annotations$TA; pos: [LOCAL_VARIABLE, {start_pc = 50, length = 7, index = 1}, pos = -1]]";
                 String actual = Arrays.toString(annotations.annotations);
                 if (!expected.equals(actual)) {
                     throw new AssertionError("Unexpected type annotations: " +

test/langtools/tools/javac/patterns/BindingsTest1.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -231,6 +231,11 @@ public class BindingsTest1 {
             throw new AssertionError();
         }
 
+        //binding in an anonymous class:
+        if (!(invokeOnce("") instanceof String s)) {
+            throw new AssertionError();
+        }
+
         System.out.println("BindingsTest1 complete");
     }
 
@@ -240,4 +245,13 @@ public class BindingsTest1 {
     static boolean id(boolean b) {
         return b;
     }
+    private static boolean invoked;
+    static Object invokeOnce(Object val) {
+        if (invoked) {
+            throw new IllegalStateException();
+        } else {
+            invoked = true;
+            return val;
+        }
+    }
 }

test/langtools/tools/javac/patterns/LocalVariableReuse.java

@@ -0,0 +1,182 @@
+/*
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8260593
+ * @summary Verify that a temporary storage variable is or is not used as needed when pattern matching.
+ * @library /tools/lib /tools/javac/lib
+ * @modules
+ *      jdk.compiler/com.sun.tools.javac.api
+ *      jdk.compiler/com.sun.tools.javac.file
+ *      jdk.compiler/com.sun.tools.javac.main
+ *      jdk.compiler/com.sun.tools.javac.util
+ * @build toolbox.ToolBox toolbox.JavacTask
+ * @build combo.ComboTestHelper
+ * @compile LocalVariableReuse.java
+ * @run main LocalVariableReuse
+ */
+
+import combo.ComboInstance;
+import combo.ComboParameter;
+import combo.ComboTask;
+import combo.ComboTestHelper;
+import java.io.IOException;
+import javax.tools.JavaFileObject;
+import toolbox.ToolBox;
+
+public class LocalVariableReuse extends ComboInstance<LocalVariableReuse> {
+    protected ToolBox tb;
+
+    LocalVariableReuse() {
+        super();
+        tb = new ToolBox();
+    }
+
+    public static void main(String... args) throws Exception {
+        new ComboTestHelper<LocalVariableReuse>()
+                .withDimension("CODE", (x, code) -> x.code = code, Code.values())
+                .run(LocalVariableReuse::new);
+    }
+
+    private Code code;
+
+    private static final String MAIN_TEMPLATE =
+            """
+            public class Test {
+                #{CODE}
+            }
+            """;
+
+    @Override
+    protected void doWork() throws Throwable {
+        ComboTask task = newCompilationTask()
+                .withSourceFromTemplate(MAIN_TEMPLATE, pname -> switch (pname) {
+                        case "CODE" -> code;
+                        default -> throw new UnsupportedOperationException(pname);
+                    });
+
+        task.withOption("-printsource");
+        task.generate(result -> {
+            for (JavaFileObject out : result.get()) {
+                try {
+                    String actualDesugared = out.getCharContent(false).toString();
+                    boolean hasTempVar = actualDesugared.contains("$temp");
+                    if (hasTempVar != code.useTemporaryVariable) {
+                        throw new AssertionError("Expected temporary variable: " + code.useTemporaryVariable +
+                                                  ", but got: " + actualDesugared);
+                    }
+                } catch (IOException ex) {
+                    throw new AssertionError(ex);
+                }
+            }
+        });
+    }
+
+    public enum Code implements ComboParameter {
+        LOCAL_VARIABLE(
+                """
+                private boolean test() {
+                    Object o = null;
+                    return o instanceof String s && s.length() > 0;
+                }
+                """, false),
+        PARAMETER(
+                """
+                private boolean test(Object o) {
+                    return o instanceof String s && s.length() > 0;
+                }
+                """, false),
+        LAMBDA_PARAMETER(
+                """
+                private void test(Object o) {
+                    I i = o -> o instanceof String s && s.length() > 0;
+                interface I {
+                    public boolean run(Object o);
+                }
+                """, false),
+        EXCEPTION(
+                """
+                private boolean test() {
+                    try {
+                        throw new Exception();
+                    } catch (Exception o) {
+                        return o instanceof RuntimeException re && re.getMessage() != null;
+                    }
+                }
+                """, false),
+        RESOURCE(
+                """
+                private boolean test() throws Exception {
+                    try (AutoCloseable r = null) {
+                        return r instanceof java.io.InputStream in && in.read() != (-1);
+                    } catch (Exception o) {
+                    }
+                }
+                """, false),
+        FIELD("""
+              private Object o;
+              private boolean test() {
+                  return o instanceof String s && s.length() > 0;
+              }
+              """,
+              true),
+        FINAL_FIELD("""
+              private final Object o;
+              private boolean test() {
+                  return o instanceof String s && s.length() > 0;
+              }
+              """,
+              true),
+        ARRAY_ACCESS("""
+              private boolean test() {
+                  Object[] o = null;
+                  return o[0] instanceof String s && s.length() > 0;
+              }
+              """,
+              true),
+        METHOD_INVOCATION("""
+              private boolean test() {
+                  return get() instanceof String s && s.length() > 0;
+              }
+              private Object get() {
+                  return null;
+              }
+              """,
+              true),
+        ;
+        private final String body;
+        private final boolean useTemporaryVariable;
+
+        private Code(String body, boolean useTemporaryVariable) {
+            this.body = body;
+            this.useTemporaryVariable = useTemporaryVariable;
+        }
+
+        @Override
+        public String expand(String optParameter) {
+            return body;
+        }
+    }
+
+}

test/langtools/tools/javac/patterns/NoUnnecessaryCast.java

@@ -67,11 +67,9 @@ public class NoUnnecessaryCast {
                               .get();
         String expectedInstructions = """
                                       aload_1
-                                      astore_3
-                                      aload_3
                                       instanceof
                                       ifeq
-                                      aload_3
+                                      aload_1
                                       checkcast
                                       astore_2
                                       aload_2