8172240: javac should not need the transitive closure to compile a module
authorvromero
Fri, 03 Feb 2017 08:16:24 -0800
changeset 43579 0d00f3ea77bc
parent 43578 640eb9781ce5
child 43580 22134ce68814
8172240: javac should not need the transitive closure to compile a module Reviewed-by: jjg Contributed-by: jan.lahoda@oracle.com
langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java
langtools/test/tools/javac/modules/MissingModuleTest.java
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java	Thu Feb 02 14:55:23 2017 -0800
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java	Fri Feb 03 08:16:24 2017 -0800
@@ -165,6 +165,7 @@
     private final boolean lintOptions;
 
     private Set<ModuleSymbol> rootModules = null;
+    private final Set<ModuleSymbol> warnedMissing = new HashSet<>();
 
     public static Modules instance(Context context) {
         Modules instance = context.get(Modules.class);
@@ -525,7 +526,6 @@
             ModuleSymbol msym = moduleFinder.findModule((ModuleSymbol) sym);
 
             if (msym.kind == ERR) {
-                log.error(Errors.ModuleNotFound(msym));
                 //make sure the module is initialized:
                 msym.directives = List.nil();
                 msym.exports = List.nil();
@@ -674,6 +674,7 @@
             ModuleSymbol msym = lookupModule(tree.moduleName);
             if (msym.kind != MDL) {
                 log.error(tree.moduleName.pos(), Errors.ModuleNotFound(msym));
+                warnedMissing.add(msym);
             } else if (allRequires.contains(msym)) {
                 log.error(tree.moduleName.pos(), Errors.DuplicateRequires(msym));
             } else {
@@ -1184,26 +1185,44 @@
         return allModules == null || allModules.contains(msym);
     }
 
-    private Set<ModuleSymbol> computeTransitiveClosure(Iterable<? extends ModuleSymbol> base, Set<ModuleSymbol> observable) {
-        List<ModuleSymbol> todo = List.nil();
+    private Set<ModuleSymbol> computeTransitiveClosure(Set<? extends ModuleSymbol> base, Set<ModuleSymbol> observable) {
+        List<ModuleSymbol> primaryTodo = List.nil();
+        List<ModuleSymbol> secondaryTodo = List.nil();
 
         for (ModuleSymbol ms : base) {
-            todo = todo.prepend(ms);
+            primaryTodo = primaryTodo.prepend(ms);
         }
 
         Set<ModuleSymbol> result = new LinkedHashSet<>();
         result.add(syms.java_base);
 
-        while (todo.nonEmpty()) {
-            ModuleSymbol current = todo.head;
-            todo = todo.tail;
+        while (primaryTodo.nonEmpty() || secondaryTodo.nonEmpty()) {
+            ModuleSymbol current;
+            boolean isPrimaryTodo;
+            if (primaryTodo.nonEmpty()) {
+                current = primaryTodo.head;
+                primaryTodo = primaryTodo.tail;
+                isPrimaryTodo = true;
+            } else {
+                current = secondaryTodo.head;
+                secondaryTodo = secondaryTodo.tail;
+                isPrimaryTodo = false;
+            }
             if (observable != null && !observable.contains(current))
                 continue;
             if (!result.add(current) || current == syms.unnamedModule || ((current.flags_field & Flags.AUTOMATIC_MODULE) != 0))
                 continue;
             current.complete();
+            if (current.kind == ERR && isPrimaryTodo && warnedMissing.add(current)) {
+                log.error(Errors.ModuleNotFound(current));
+            }
             for (RequiresDirective rd : current.requires) {
-                todo = todo.prepend(rd.module);
+                if (rd.module == syms.java_base) continue;
+                if ((rd.isTransitive() && isPrimaryTodo) || base.contains(current)) {
+                    primaryTodo = primaryTodo.prepend(rd.module);
+                } else {
+                    secondaryTodo = secondaryTodo.prepend(rd.module);
+                }
             }
         }
 
@@ -1583,5 +1602,6 @@
     public void newRound() {
         rootModules = null;
         allModules = null;
+        warnedMissing.clear();
     }
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/modules/MissingModuleTest.java	Fri Feb 03 08:16:24 2017 -0800
@@ -0,0 +1,129 @@
+/*
+ * Copyright (c) 2017, 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 8172240
+ * @summary javac should not need the transitive closure to compile a module
+ * @library /tools/lib
+ * @modules
+ *      jdk.compiler/com.sun.tools.javac.api
+ *      jdk.compiler/com.sun.tools.javac.code
+ *      jdk.compiler/com.sun.tools.javac.main
+ *      jdk.compiler/com.sun.tools.javac.processing
+ * @build toolbox.ToolBox toolbox.JavacTask toolbox.ModuleBuilder ModuleTestBase
+ * @run main MissingModuleTest
+ */
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.List;
+
+import toolbox.JavacTask;
+import toolbox.Task;
+import toolbox.Task.Expect;
+
+public class MissingModuleTest extends ModuleTestBase {
+
+    public static void main(String... args) throws Exception {
+        new MissingModuleTest().runTests();
+    }
+
+    @Test
+    public void testMissingNotNeeded(Path base) throws Exception {
+        doTest(base, "m1x", new String[0]);
+    }
+
+    @Test
+    public void testMissingNeededTransitive(Path base) throws Exception {
+        doTest(base, "m2x", "- compiler.err.module.not.found: m2x", "1 error");
+    }
+
+    @Test
+    public void testMissingNeededDirect(Path base) throws Exception {
+        doTest(base, "m3x", "module-info.java:1:24: compiler.err.module.not.found: m3x", "1 error");
+    }
+
+    @Test
+    public void testMultipleErrors(Path base) throws Exception {
+        doTest(base, "m4x", "module-info.java:1:38: compiler.err.module.not.found: m4x", "1 error");
+    }
+
+    private void doTest(Path base, String toDelete, String... errors) throws Exception {
+        //note: avoiding use of java.base, as that gets special handling on some places:
+        Path srcMP = base.resolve("src-mp");
+        Path m1x = srcMP.resolve("m1x");
+        tb.writeJavaFiles(m1x,
+                          "module m1x { exports api1; }",
+                          "package api1; public interface Api { }");
+        Path m2x = srcMP.resolve("m2x");
+        tb.writeJavaFiles(m2x,
+                          "module m2x { requires m1x; exports api2; }",
+                          "package api2; public interface Api { }");
+        Path m3x = srcMP.resolve("m3x");
+        tb.writeJavaFiles(m3x,
+                          "module m3x { requires transitive m2x; }");
+        Path m4x = srcMP.resolve("m4x");
+        tb.writeJavaFiles(m4x,
+                          "module m4x { requires transitive m2x; }");
+        Path m5x = srcMP.resolve("m5x");
+        tb.writeJavaFiles(m5x,
+                          "module m5x { requires transitive m4x; }");
+        Path classesMP = base.resolve("classes-mp");
+        tb.createDirectories(classesMP);
+
+        new JavacTask(tb)
+            .options("--module-source-path", srcMP.toString())
+            .outdir(classesMP)
+            .files(findJavaFiles(srcMP))
+            .run()
+            .writeAll();
+
+        tb.cleanDirectory(classesMP.resolve(toDelete));
+        Files.delete(classesMP.resolve(toDelete));
+
+        Path src = base.resolve("src");
+        tb.writeJavaFiles(src,
+                          "module test { requires m3x; requires m4x; requires m5x; } ");
+        Path classes = base.resolve("classes");
+        tb.createDirectories(classes);
+
+        List<String> log = new JavacTask(tb)
+                .options("--module-path", classesMP.toString(),
+                         "-XDrawDiagnostics")
+                .outdir(classes)
+                .files(findJavaFiles(src))
+                .run(errors.length > 0 ? Expect.FAIL : Expect.SUCCESS)
+                .writeAll()
+                .getOutputLines(Task.OutputKind.DIRECT);
+
+        if (errors.length == 0) {
+            errors = new String[] {""};
+        }
+
+        if (!log.equals(Arrays.asList(errors)))
+            throw new Exception("expected output not found: " + log);
+    }
+
+}