changeset 52358:9eb968074157 patterns-stage-1

The scope of a binding variable may extend beyond the statement that introduces it.
author jlahoda
date Tue, 11 Sep 2018 12:42:39 +0200
parents 79c8fbd899fe
children 17179e37aa12
files src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java src/jdk.compiler/share/classes/com/sun/tools/javac/comp/MatchBindingsComputer.java src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java test/langtools/tools/javac/patterns/BindingsTest1.java test/langtools/tools/javac/patterns/scope/ScopeTest.java test/langtools/tools/javac/patterns/scope/TEST.properties
diffstat 8 files changed, 338 insertions(+), 7 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java	Tue Sep 11 12:40:55 2018 +0200
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java	Tue Sep 11 12:42:39 2018 +0200
@@ -324,6 +324,11 @@
      */
     public static final long MATCH_BINDING = 1L<<58;
 
+    /**
+     * A flag to indicate a match binding variable whose scope extends after the current statement.
+     */
+    public static final long MATCH_BINDING_TO_OUTER = 1L<<59;
+
     /** Modifier masks.
      */
     public static final int
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java	Tue Sep 11 12:40:55 2018 +0200
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java	Tue Sep 11 12:42:39 2018 +0200
@@ -76,6 +76,8 @@
 import static com.sun.tools.javac.code.TypeTag.*;
 import static com.sun.tools.javac.code.TypeTag.WILDCARD;
 import com.sun.tools.javac.comp.Analyzer.AnalyzerMode;
+import com.sun.tools.javac.comp.MatchBindingsComputer.BindingSymbol;
+import com.sun.tools.javac.tree.JCTree.JCBreak;
 import static com.sun.tools.javac.tree.JCTree.Tag.*;
 import com.sun.tools.javac.util.JCDiagnostic.DiagnosticFlag;
 
@@ -1318,6 +1320,12 @@
     public void visitDoLoop(JCDoWhileLoop tree) {
         attribStat(tree.body, env.dup(tree));
         attribExpr(tree.cond, env, syms.booleanType);
+        if (!breaksOutOf(tree, tree.body)) {
+            List<BindingSymbol> bindings = getMatchBindings(types, log, tree.cond, false);
+
+            bindings.forEach(env.info.scope::enter);
+            bindings.forEach(BindingSymbol::preserveBinding);
+        }
         result = null;
     }
 
@@ -1330,9 +1338,29 @@
         } finally {
             whileEnv.info.scope.leave();
         }
+        if (!breaksOutOf(tree, tree.body)) {
+            List<BindingSymbol> bindings = getMatchBindings(types, log, tree.cond, false);
+
+            bindings.forEach(env.info.scope::enter);
+            bindings.forEach(BindingSymbol::preserveBinding);
+        }
         result = null;
     }
 
+    private boolean breaksOutOf(JCTree loop, JCTree body) {
+        //TODO: should correctly reflect liveness:
+        boolean[] breaksOut = new boolean[1];
+        new TreeScanner() {
+            @Override
+            public void visitBreak(JCBreak tree) {
+                breaksOut[0] |= tree.target == loop;
+                super.visitBreak(tree);
+            }
+        }.scan(body);
+
+        return breaksOut[0];
+    }
+
     public void visitForLoop(JCForLoop tree) {
         Env<AttrContext> loopEnv =
             env.dup(env.tree, env.info.dup(env.info.scope.dup()));
@@ -1352,6 +1380,12 @@
             } finally {
                 bodyEnv.info.scope.leave();
             }
+            if (!breaksOutOf(tree, tree.body)) {
+                List<BindingSymbol> bindings = getMatchBindings(types, log, tree.cond, false);
+
+                bindings.forEach(env.info.scope::enter);
+                bindings.forEach(BindingSymbol::preserveBinding);
+            }
             result = null;
         }
         finally {
@@ -1895,22 +1929,46 @@
 
         // if (x) { y } [ else z ] include x.T in y; include x.F in z
 
-        Env<AttrContext> thenEnv = bindingEnv(env, getMatchBindings(types, log, tree.cond, true));
+        List<BindingSymbol> thenBindings = getMatchBindings(types, log, tree.cond, true);
+        Env<AttrContext> thenEnv = bindingEnv(env, thenBindings);
+
         try {
             attribStat(tree.thenpart, thenEnv);
         } finally {
             thenEnv.info.scope.leave();
         }
 
+        boolean aliveAfterThen = flow.aliveAfter(env, tree.thenpart, make);
+        boolean aliveAfterElse;
+        List<BindingSymbol> elseBindings = List.nil();
+
         if (tree.elsepart != null) {
-            Env<AttrContext> elseEnv = bindingEnv(env, getMatchBindings(types, log, tree.cond, false));
+            elseBindings = getMatchBindings(types, log, tree.cond, false);
+
+            Env<AttrContext> elseEnv = bindingEnv(env, elseBindings);
             try {
                 attribStat(tree.elsepart, elseEnv);
             } finally {
                 elseEnv.info.scope.leave();
             }
-        }
+            aliveAfterElse = flow.aliveAfter(env, tree.elsepart, make);
+        } else {
+            aliveAfterElse = true;
+        }
+
         chk.checkEmptyIf(tree);
+
+        List<BindingSymbol> afterIfBindings = List.nil();
+
+        if (aliveAfterThen && !aliveAfterElse) {
+            afterIfBindings = thenBindings;
+        } else if (aliveAfterElse && !aliveAfterThen) {
+            afterIfBindings = elseBindings;
+        }
+
+        afterIfBindings.forEach(env.info.scope::enter);
+        afterIfBindings.forEach(BindingSymbol::preserveBinding);
+
         result = null;
     }
 
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java	Tue Sep 11 12:40:55 2018 +0200
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java	Tue Sep 11 12:42:39 2018 +0200
@@ -255,6 +255,23 @@
         }
     }
 
+    public boolean aliveAfter(Env<AttrContext> env, JCTree that, TreeMaker make) {
+        //we need to disable diagnostics temporarily; the problem is that if
+        //a lambda expression contains e.g. an unreachable statement, an error
+        //message will be reported and will cause compilation to skip the flow analyis
+        //step - if we suppress diagnostics, we won't stop at Attr for flow-analysis
+        //related errors, which will allow for more errors to be detected
+        Log.DiagnosticHandler diagHandler = new Log.DiscardDiagnosticHandler(log);
+        try {
+            SnippetAliveAnalyzer analyzer = new SnippetAliveAnalyzer();
+
+            analyzer.analyzeTree(env, that, make);
+            return analyzer.isAlive();
+        } finally {
+            log.popDiagnosticHandler(diagHandler);
+        }
+    }
+
     /**
      * Definite assignment scan mode
      */
@@ -1425,6 +1442,19 @@
     }
 
     /**
+     * Determine if alive after the given tree.
+     */
+    class SnippetAliveAnalyzer extends AliveAnalyzer {
+        @Override
+        public void visitClassDef(JCClassDecl tree) {
+            //skip
+        }
+        public boolean isAlive() {
+            return super.alive;
+        }
+    }
+
+    /**
      * Specialized pass that performs DA/DU on a lambda
      */
     class LambdaAssignAnalyzer extends AssignAnalyzer {
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/MatchBindingsComputer.java	Tue Sep 11 12:40:55 2018 +0200
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/MatchBindingsComputer.java	Tue Sep 11 12:42:39 2018 +0200
@@ -201,6 +201,14 @@
         List<BindingSymbol> aliases() {
             return List.of(this);
         }
+
+        public void preserveBinding() {
+            flags_field |= Flags.MATCH_BINDING_TO_OUTER;
+        }
+
+        public boolean isPreserved() {
+            return (flags_field & Flags.MATCH_BINDING_TO_OUTER) != 0;
+        }
     }
 
     public static class IntersectionBindingSymbol extends BindingSymbol {
@@ -218,5 +226,14 @@
         List<BindingSymbol> aliases() {
             return aliases;
         }
+
+        @Override
+        public void preserveBinding() {
+            aliases.stream().forEach(BindingSymbol::preserveBinding);
+        }
+
+        public boolean isPreserved() {
+            return aliases.stream().allMatch(BindingSymbol::isPreserved);
+        }
     }
 }
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java	Tue Sep 11 12:40:55 2018 +0200
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java	Tue Sep 11 12:42:39 2018 +0200
@@ -58,11 +58,14 @@
 import com.sun.tools.javac.util.Options;
 
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.stream.Collectors;
 
 import com.sun.tools.javac.code.Symbol.MethodSymbol;
 import static com.sun.tools.javac.code.TypeTag.BOOLEAN;
 import static com.sun.tools.javac.code.TypeTag.BOT;
+import com.sun.tools.javac.tree.JCTree.JCBlock;
+import com.sun.tools.javac.tree.JCTree.JCStatement;
 
 /**
  * This pass translates pattern-matching constructs, such as instanceof <pattern>.
@@ -107,6 +110,11 @@
             //do nothing
             return this;
         }
+
+        @Override
+        boolean tryPrepend(BindingSymbol binding, JCVariableDecl var) {
+            return false;
+        }
     };
 
     JCLabeledStatement pendingMatchLabel = null;
@@ -256,6 +264,28 @@
         }
     }
 
+    @Override
+    public void visitBlock(JCBlock tree) {
+        ListBuffer<JCStatement> statements = new ListBuffer<>();
+        bindingContext = new BasicBindingContext(List.nil()) {
+            boolean tryPrepend(BindingSymbol binding, JCVariableDecl var) {
+                hoistedVarMap.put(binding, var.sym);
+                statements.append(var);
+                return true;
+            }
+        };
+        try {
+            for (List<JCStatement> l = tree.stats; l.nonEmpty(); l = l.tail) {
+                statements.append(translate(l.head));
+            }
+
+            tree.stats = statements.toList();
+            result = tree;
+        } finally {
+            bindingContext.pop();
+        }
+    }
+
     public JCTree translateTopLevelClass(Env<AttrContext> env, JCTree cdef, TreeMaker make) {
         try {
             this.make = make;
@@ -344,6 +374,7 @@
         abstract JCStatement decorateStatement(JCStatement stat);
         abstract JCExpression decorateExpression(JCExpression expr);
         abstract BindingContext pop();
+        abstract boolean tryPrepend(BindingSymbol binding, JCVariableDecl var);
     }
 
     class BasicBindingContext extends BindingContext {
@@ -375,11 +406,18 @@
         JCStatement decorateStatement(JCStatement stat) {
             if (hoistedVarMap.isEmpty()) return stat;
             ListBuffer<JCStatement> stats = new ListBuffer<>();
-            for (VarSymbol vsym : hoistedVarMap.values()) {
-                stats.add(makeHoistedVarDecl(stat.pos, vsym));
+            for (Entry<BindingSymbol, VarSymbol> e : hoistedVarMap.entrySet()) {
+                JCVariableDecl decl = makeHoistedVarDecl(stat.pos, e.getValue());
+                if (!e.getKey().isPreserved() ||
+                    !parent.tryPrepend(e.getKey(), decl)) {
+                    stats.add(decl);
+                }
             }
-            stats.add(stat);
-            return make.at(stat.pos).Block(0, stats.toList());
+            if (stats.nonEmpty()) {
+                stats.add(stat);
+                stat = make.at(stat.pos).Block(0, stats.toList());
+            }
+            return stat;
         }
 
         @Override
@@ -395,6 +433,11 @@
             return bindingContext = parent;
         }
 
+        @Override
+        boolean tryPrepend(BindingSymbol binding, JCVariableDecl var) {
+            return false;
+        }
+
         private JCVariableDecl makeHoistedVarDecl(int pos, VarSymbol varSymbol) {
             return make.at(pos).VarDef(varSymbol, makeDefaultValue(pos, varSymbol.erasure(types)));
         }
--- a/test/langtools/tools/javac/patterns/BindingsTest1.java	Tue Sep 11 12:40:55 2018 +0200
+++ b/test/langtools/tools/javac/patterns/BindingsTest1.java	Tue Sep 11 12:42:39 2018 +0200
@@ -111,7 +111,32 @@
             s.length();
         }
 
+        L1: {
+            if (o1 instanceof String s) {
+                s.length();
+            } else {
+                break L1;
+            }
+            s.length();
+        }
 
+        L2: {
+            if (!(o1 instanceof String s)) {
+                break L2;
+            } else {
+                s.length();
+            }
+            s.length();
+        }
+
+        L3: {
+            if ((o1 instanceof String s) || (o3 instanceof String s)) {
+                s.length();
+            } else {
+                break L3;
+            }
+            s.length();
+        }
 
         System.out.println("BindingsTest1 complete");
     }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/patterns/scope/ScopeTest.java	Tue Sep 11 12:42:39 2018 +0200
@@ -0,0 +1,147 @@
+/*
+ * Copyright (c) 2018, 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.
+ */
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.testng.ITestResult;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.Test;
+import tools.javac.combo.JavacTemplateTestBase;
+
+import static java.util.stream.Collectors.toList;
+
+@Test
+public class ScopeTest extends JavacTemplateTestBase {
+
+    private static String st_block(String... statements) {
+        return Arrays.stream(statements).collect(Collectors.joining("", "{", "}"));
+    }
+
+    private static String st_if(String condition, String then, String els) {
+        return "if (" + condition + ") " + then + " else " + els;
+    }
+
+    private static String st_while(String condition, String body) {
+        return "while (" + condition + ") " + body;
+    }
+
+    private static String st_do_while(String body, String condition) {
+        return "do " + body + " while (" + condition + ");";
+    }
+
+    private static String st_for(String init, String condition, String update, String body) {
+        return "for (" + init + "; " + condition + "; " + update + ") " + body;
+    }
+
+    private static String st_s_use() {
+        return "s.length();";
+    }
+
+    private static String st_break() {
+        return "break;";
+    }
+
+    private static String st_return() {
+        return "return;";
+    }
+
+    private static String st_noop() {
+        return ";";
+    }
+
+    private static String expr_empty() {
+        return "";
+    }
+
+    private static String expr_o_match_str() {
+        return "o instanceof String s";
+    }
+
+    private static String expr_not(String expr) {
+        return "!(" + expr + ")";
+    }
+
+    @AfterMethod
+    public void dumpTemplateIfError(ITestResult result) {
+        // Make sure offending template ends up in log file on failure
+        if (!result.isSuccess()) {
+            System.err.printf("Diagnostics: %s%nTemplate: %s%n", diags.errorKeys(), sourceFiles.stream().map(p -> p.snd).collect(toList()));
+        }
+    }
+
+    private void program(String block) {
+        String s = "class C { void m(Object o) " + block + "}";
+        addSourceFile("C.java", new StringTemplate(s));
+    }
+
+    private void assertOK(String block) {
+        reset();
+        program(block);
+        try {
+            compile();
+        }
+        catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+        assertCompileSucceeded();
+    }
+
+    private void assertFail(String expectedDiag, String block) {
+        reset();
+        addCompileOptions("--enable-preview", "-source", "12");
+        program(block);
+        try {
+            compile();
+        }
+        catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+        assertCompileFailed(expectedDiag);
+    }
+
+    public void testIf() {
+        assertOK(st_block(st_if(expr_o_match_str(), st_s_use(), st_return()), st_s_use()));
+        assertOK(st_block(st_if(expr_not(expr_o_match_str()), st_return(), st_s_use()), st_s_use()));
+        assertFail("compiler.err.cant.resolve.location", st_block(st_if(expr_o_match_str(), st_s_use(), st_noop()), st_s_use()));
+        assertFail("compiler.err.cant.resolve.location", st_block(st_if(expr_not(expr_o_match_str()), st_noop(), st_s_use()), st_s_use()));
+    }
+
+    public void testWhile() {
+        assertOK(st_block(st_while(expr_not(expr_o_match_str()), st_noop()), st_s_use()));
+        assertFail("compiler.err.cant.resolve.location", st_block(st_while(expr_not(expr_o_match_str()), st_break()), st_s_use()));
+    }
+
+    public void testDoWhile() {
+        assertOK(st_block(st_do_while(st_noop(), expr_not(expr_o_match_str())), st_s_use()));
+        assertFail("compiler.err.cant.resolve.location", st_block(st_do_while(st_break(), expr_not(expr_o_match_str())), st_s_use()));
+    }
+
+    public void testFor() {
+        assertOK(st_block(st_for(expr_empty(), expr_not(expr_o_match_str()), expr_empty(), st_noop()), st_s_use()));
+        assertFail("compiler.err.cant.resolve.location", st_block(st_for(expr_empty(), expr_not(expr_o_match_str()), expr_empty(), st_break()), st_s_use()));
+    }
+
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/patterns/scope/TEST.properties	Tue Sep 11 12:42:39 2018 +0200
@@ -0,0 +1,6 @@
+TestNG.dirs = .
+
+lib.dirs = /lib/combo
+
+modules = \
+        jdk.compiler/com.sun.tools.javac.util