Alternative approach to inject 'requires transitive java.base;'

lahodaj · Oct 1, 2024 · 71f7957 · 71f7957
1 parent 52b755c
commit 71f7957
Show file tree

Hide file tree

Showing 6 changed files with 101 additions and 10 deletions.
diff --git a/make/CompileJavaModules.gmk b/make/CompileJavaModules.gmk
@@ -88,11 +88,6 @@ CreateHkTargets = \
 
 -include Java.gmk
 
-ifeq ($(TARGET_RELEASE), )
-  # If unspecified, default to the new jdk we're building
-  TARGET_RELEASE := $(TARGET_RELEASE_NEWJDK)
-endif
-
 ################################################################################
 # Setup the main compilation
 
@@ -113,12 +108,11 @@ $(eval $(call SetupJavaCompilation, $(MODULE), \
     EXCLUDE_FILES := $(EXCLUDE_FILES), \
     KEEP_ALL_TRANSLATIONS := $(KEEP_ALL_TRANSLATIONS), \
     JAVAC_FLAGS := \
-        $(DOCLINT) -doe \
+        $(DOCLINT) \
         $(JAVAC_FLAGS) \
         --module-source-path $(MODULESOURCEPATH) \
         --module-path $(MODULEPATH) \
         --system none, \
-    TARGET_RELEASE=$(TARGET_RELEASE) \
 ))
 
 TARGETS += $($(MODULE))

diff --git a/make/Docs.gmk b/make/Docs.gmk
@@ -108,7 +108,6 @@ JAVADOC_OPTIONS := -use -keywords -notimestamp \
     -encoding ISO-8859-1 -docencoding UTF-8 -breakiterator \
     -splitIndex --system none -javafx --expand-requires transitive \
     --override-methods=summary \
-    --enable-preview --source $(JDK_SOURCE_TARGET_VERSION) \
     --no-external-specs-page
 
 # The reference options must stay stable to allow for comparisons across the

diff --git a/make/langtools/src/classes/build/tools/symbolgenerator/JavadocElementList.java b/make/langtools/src/classes/build/tools/symbolgenerator/JavadocElementList.java
@@ -73,7 +73,6 @@ public static void main(String... args) throws IOException {
         List<String> options = List.of("-source", Source.DEFAULT.name,
                                        "-target", Target.DEFAULT.name,
                                        "-proc:only",
-                                       "--enable-preview",
                                        "--system", "none",
                                        "--module-source-path", args[1],
                                        "--add-modules", Arrays.stream(args)

diff --git a/src/java.se/share/classes/module-info.java b/src/java.se/share/classes/module-info.java
@@ -39,7 +39,6 @@
  * @since 9
  */
 module java.se {
-    requires transitive java.base;
     requires transitive java.compiler;
     requires transitive java.datatransfer;
     requires transitive java.desktop;

diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java
@@ -151,6 +151,7 @@ public class Modules extends JCTree.Visitor {
     private final boolean allowModules;
     private final boolean allowAccessIntoSystem;
     private final boolean allowRequiresTransitiveJavaBase;
+    private final boolean previewEnabled;
 
     public final boolean multiModuleMode;
 
@@ -209,6 +210,7 @@ protected Modules(Context context) {
 
         allowRequiresTransitiveJavaBase =
                 (Feature.JAVA_BASE_TRANSITIVE.allowedInSource(source) && (!preview.isPreview(Feature.JAVA_BASE_TRANSITIVE) || preview.isEnabled()));
+        previewEnabled = preview.isEnabled();
         lintOptions = options.isUnset(Option.XLINT_CUSTOM, "-" + LintCategory.OPTIONS.option);
 
         multiModuleMode = fileManager.hasLocation(StandardLocation.MODULE_SOURCE_PATH);
@@ -1506,6 +1508,14 @@ private void completeModule(ModuleSymbol msym) {
         initAddReads();
 
         msym.requires = msym.requires.appendList(List.from(addReads.getOrDefault(msym, Collections.emptySet())));
+
+        if (previewEnabled && msym.name == names.java_se) {
+            msym.requires.forEach(rd -> {
+                if (rd.module == syms.java_base) {
+                    rd.flags.add(RequiresFlag.TRANSITIVE);
+                }
+            });
+        }
 
         List<RequiresDirective> requires = msym.requires;
 

diff --git a/test/langtools/tools/javac/modules/EdgeCases.java b/test/langtools/tools/javac/modules/EdgeCases.java
@@ -73,6 +73,9 @@
 import com.sun.tools.javac.code.Symbol.ModuleSymbol;
 import com.sun.tools.javac.code.Symtab;
 import java.util.ArrayList;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Consumer;
+import javax.lang.model.element.ModuleElement.DirectiveKind;
 
 import toolbox.JarTask;
 import toolbox.JavacTask;
@@ -1153,4 +1156,91 @@ private void record(TaskEvent e, String phase) {
         }
     }
 
+    @Test
+    public void testJavaSEHasRequiresTransitiveJavaBase(Path base) throws Exception {
+        Path src = base.resolve("src");
+        Path a = src.resolve("a");
+        tb.writeJavaFiles(a,
+                          "module a { requires java.se; }",
+                          """
+                          package test;
+                          import module java.se;
+                          public class Test {
+                              ArrayList<String> l;
+                          }
+                          """);
+        Path classes = base.resolve("classes");
+        tb.createDirectories(classes);
+
+        AtomicBoolean seenJavaSEDependency = new AtomicBoolean();
+
+        List<String> log;
+
+        log = new JavacTask(tb)
+            .outdir(classes)
+            .options("-XDrawDiagnostics", "-XDshould-stop.at=FLOW")
+            .callback(verifyJavaSEDependency(false, seenJavaSEDependency))
+            .files(findJavaFiles(src))
+            .run(Task.Expect.FAIL)
+            .writeAll()
+            .getOutputLines(Task.OutputKind.DIRECT);
+
+        List<String> expected = List.of(
+                "Test.java:2:8: compiler.err.preview.feature.disabled.plural: (compiler.misc.feature.module.imports)",
+                "Test.java:4:5: compiler.err.cant.resolve.location: kindname.class, ArrayList, , , (compiler.misc.location: kindname.class, test.Test, null)",
+                "2 errors");
+
+        if (!expected.equals(log))
+            throw new Exception("expected output not found: " + log);
+
+        if (!seenJavaSEDependency.get()) {
+            throw new AssertionError("Didn't find the java.se dependency!");
+        }
+
+        seenJavaSEDependency.set(false);
+
+        new JavacTask(tb)
+            .outdir(classes)
+            .options("--enable-preview",
+                     "--source", System.getProperty("java.specification.version"))
+            .callback(verifyJavaSEDependency(true, seenJavaSEDependency))
+            .files(findJavaFiles(src))
+            .run(Task.Expect.SUCCESS)
+            .writeAll()
+            .getOutputLines(Task.OutputKind.DIRECT);
+
+        if (!seenJavaSEDependency.get()) {
+            throw new AssertionError("Didn't find the java.se dependency!");
+        }
+    }
+        private Consumer<com.sun.source.util.JavacTask> verifyJavaSEDependency(
+                boolean expectedTransitive,
+                AtomicBoolean seenJavaSEDependency) {
+            return t -> {
+                    t.addTaskListener(new TaskListener() {
+                        @Override
+                        public void finished(TaskEvent e) {
+                            if (e.getKind() == TaskEvent.Kind.ANALYZE) {
+                                ModuleElement javaBase =
+                                        t.getElements().getModuleElement("java.base");
+                                ModuleElement javaSE =
+                                        t.getElements().getModuleElement("java.se");
+                                RequiresDirective requiresJavaSE = 
+                                        javaSE.getDirectives()
+                                              .stream()
+                                              .filter(d -> d.getKind() == DirectiveKind.REQUIRES)
+                                              .map(d -> (RequiresDirective) d)
+                                              .filter(d -> d.getDependency() == javaBase)
+                                              .findAny()
+                                              .orElseThrow();
+                                if (requiresJavaSE.isTransitive() != expectedTransitive) {
+                                    throw new AssertionError("Expected: " + expectedTransitive + ", " +
+                                                             "but got: " + requiresJavaSE.isTransitive());
+                                }
+                                seenJavaSEDependency.set(true);
+                            }
+                        }
+                    });
+                };
+        }
 }