8344708: Implement JEP 511: Module Import Declarations #23801

lahodaj · 2025-02-26T13:50:52Z

This is a patch to finalize the module imports feature. Please see:
https://bugs.openjdk.org/browse/JDK-8344700

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue
Change requires a JEP request to be targeted
Change requires CSR request JDK-8350781 to be approved

Issues

JDK-8344708: Implement JEP 511: Module Import Declarations (Sub-task - P4)
JDK-8344700: JEP 511: Module Import Declarations (JEP)
JDK-8350781: Compiler Implementation of Module Import Declarations (CSR)

Reviewers

Vicente Romero (@vicente-romero-oracle - Reviewer) Review applies to da39d0ef
Alan Bateman (@AlanBateman - Reviewer) Review applies to 029e0dd5
Maurizio Cimadamore (@mcimadamore - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/23801/head:pull/23801
$ git checkout pull/23801

Update a local copy of the PR:
$ git checkout pull/23801
$ git pull https://git.openjdk.org/jdk.git pull/23801/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23801

View PR using the GUI difftool:
$ git pr show -t 23801

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/23801.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-02-26T13:51:30Z

👋 Welcome back jlahoda! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-02-26T13:51:50Z

@lahodaj This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8344708: Implement JEP 511: Module Import Declarations

Reviewed-by: mcimadamore, vromero, alanb

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 67 new commits pushed to the master branch:

e2ae50d: 8355569: Some nsk/jdi tests can glean the "main" thread by using the ClassPrepareEvent for the debuggee main class
e36756b: 8297727: Forcing LF interpretation lead to StackOverflowError in reflection code
24bc714: 8355956: Prepare javap for class file format aware access flag parsing
... and 64 more: https://git.openjdk.org/jdk/compare/1f228e5539a5faa3b28e12548f8ad97eeacf3298...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

lahodaj · 2025-02-26T13:52:20Z

/jep JDK-8344700
/csr needed

openjdk · 2025-02-26T13:52:30Z

@lahodaj The following labels will be automatically applied to this pull request:

compiler
core-libs
kulla

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

openjdk · 2025-02-26T13:53:37Z

@lahodaj
This pull request will not be integrated until the JEP JDK-8344700 has been targeted.

lahodaj · 2025-02-26T13:53:59Z

/csr needed

openjdk · 2025-02-26T13:54:16Z

@lahodaj has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@lahodaj please create a CSR request for issue JDK-8344708 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

openjdk · 2025-02-26T13:54:48Z

@lahodaj an approved CSR request is already required for this pull request.

mlbridge · 2025-03-04T11:31:45Z

Webrevs

vicente-romero-oracle

looks sensible

vicente-romero-oracle · 2025-03-04T19:21:04Z

test/langtools/tools/javac/modules/EdgeCases.java

            .callback(verifyJavaSEDependency(true, 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:2:8: compiler.err.feature.not.supported.in.source.plural: (compiler.misc.feature.module.imports), 24, 25",


isn't there a similar test above?

…base; for all classfile versions; java.se no longer participates in preview.

bridgekeeper · 2025-04-01T22:04:47Z

@lahodaj This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

vicente-romero-oracle · 2025-04-02T15:23:54Z

src/jdk.jshell/share/classes/jdk/internal/jshell/tool/Startup.java

-    static Startup defaultStartup(boolean preview, MessageHandler mh) {
-        int idx = preview ? 1 : 0;
+    static Startup defaultStartup(int sourceLevel, boolean preview, MessageHandler mh) {
+        boolean startup24 = sourceLevel < 25;


should we use a constant here?

Yes, there should be a constant. Done here:
d66703c
Thanks!

vicente-romero-oracle

lgtm

AlanBateman · 2025-04-09T17:52:26Z

src/java.base/share/classes/jdk/internal/module/ModuleInfo.java

-                        flagName = "ACC_TRANSITIVE";
-                    }
+                    && mods.contains(Requires.Modifier.STATIC)) {
+                    String flagName = "ACC_STATIC_PHASE";


A minor comment on the style here, do you mind combin L413+414 as it will make it easier to read. The change is okay of course.

I did this:
029e0dd
Please let me know if something more/different is needed.

vicente-romero-oracle

lg

openjdk · 2025-04-24T13:14:55Z

@lahodaj this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8344708
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

vicente-romero-oracle

lgtm

mcimadamore · 2025-04-29T10:12:07Z

src/jdk.jshell/share/classes/jdk/internal/jshell/tool/Startup.java

+        Source source = Source.lookup(sourceLevel);
+        boolean hasModuleImports = source == null ||
+                                   Feature.MODULE_IMPORTS.allowedInSource(source);
+        int idx = preview ? 2 : !hasModuleImports ? 1 : 0;


It feels like this would be cleaner with an if/else if/else? I'm also not a big fan of hardcoded constants into an array. Maybe this code can benefit from stable values (when they are integrated).

(in the interim, maybe using an enum with the possible values, instead of indices, and then turning defaultStartup into an EnumMap might make the code slightly clearer?)

Thanks for the comments, attempted to fix here:
4f95755

mcimadamore · 2025-04-29T10:23:26Z

src/jdk.jshell/share/classes/jdk/jshell/SnippetMaps.java

@@ -186,6 +194,13 @@ String fullClassNameAndPackageToClass(String full, String pkg) {
        if (Stream.concat(Stream.of("java.lang"), pkgs).anyMatch(pkg::equals)) {
            return full.substring(pkg.length() + 1);
        }
+        Stream<String> mods = importSnippets()


Should this variable be called mods ? AFAIU, it lists all the packages exported by a module.

Thanks, attempted to improve here:
4f95755

lahodaj · 2025-05-01T07:41:54Z

/integrate

openjdk · 2025-05-01T07:42:39Z

Going to push as commit 0a697f6.
Since your change was applied there have been 69 commits pushed to the master branch:

b218410: 8354292: Remove unused PRAGMA_FORMAT_IGNORED
8ef7e25: 8355769: Optimize nmethod dependency recording
e2ae50d: 8355569: Some nsk/jdi tests can glean the "main" thread by using the ClassPrepareEvent for the debuggee main class
... and 66 more: https://git.openjdk.org/jdk/compare/1f228e5539a5faa3b28e12548f8ad97eeacf3298...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-05-01T07:42:46Z

@lahodaj Pushed as commit 0a697f6.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

lahodaj added 3 commits February 25, 2025 11:38

Finalizing module imports.

ae6d620

Cleanup.

50fbbed

Cleanup.

46c8454

openjdk bot added core-libs [email protected] compiler [email protected] kulla [email protected] labels Feb 26, 2025

openjdk bot added the jep label Feb 26, 2025

openjdk bot added the csr Pull request needs approved CSR before integration label Feb 26, 2025

lahodaj marked this pull request as ready for review March 4, 2025 11:27

openjdk bot added the rfr Pull request is ready for review label Mar 4, 2025

vicente-romero-oracle approved these changes Mar 4, 2025

View reviewed changes

lahodaj added 5 commits March 14, 2025 10:26

Cleanup - updated specification will permit requires transitive java.…

1476f97

…base; for all classfile versions; java.se no longer participates in preview.

Fixing tests.

caf0ec6

Adjusting JShell defaults.

1c55e51

Cleanup, fixing tests.

cffcf85

Fixing test.

067a044

vicente-romero-oracle reviewed Apr 2, 2025

View reviewed changes

Reflecting review feedback - avoiding hardcoded constants.

d66703c

vicente-romero-oracle approved these changes Apr 2, 2025

View reviewed changes

AlanBateman reviewed Apr 9, 2025

View reviewed changes

Reflecting review feedback: cleanup formatting in ModuleInfo.

029e0dd

AlanBateman approved these changes Apr 10, 2025

View reviewed changes

lahodaj added 2 commits April 11, 2025 08:00

Merge branch 'master' into JDK-8344708

71588f5

Adjusting ToolProviderTest to the updated persistence handling.

4c14c3b

vicente-romero-oracle approved these changes Apr 11, 2025

View reviewed changes

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Apr 24, 2025

Merge branch 'master' into JDK-8344708

da39d0e

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Apr 28, 2025

vicente-romero-oracle approved these changes Apr 28, 2025

View reviewed changes

mcimadamore reviewed Apr 29, 2025

View reviewed changes

Reflecting review feedback.

4f95755

openjdk bot removed the jep label Apr 29, 2025

mcimadamore approved these changes Apr 30, 2025

View reviewed changes

lahodaj changed the title ~~8344708: Compiler Implementation of Module Import Declarations~~ 8344708: Implement JEP 511: Module Import Declarations Apr 30, 2025

openjdk bot added ready Pull request is ready to be integrated and removed csr Pull request needs approved CSR before integration labels May 1, 2025

openjdk bot added the integrated Pull request has been integrated label May 1, 2025

openjdk bot closed this May 1, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels May 1, 2025

This was referenced May 3, 2025

support JEP 476: Module Import Declarations scala/scala3#20530

Open

support JEP 476: Module Import Declarations in JavaParsers scala/bug#13005

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8344708: Implement JEP 511: Module Import Declarations #23801

8344708: Implement JEP 511: Module Import Declarations #23801

lahodaj commented Feb 26, 2025 •

edited by openjdk bot

Loading

bridgekeeper bot commented Feb 26, 2025

openjdk bot commented Feb 26, 2025 •

edited

Loading

lahodaj commented Feb 26, 2025

openjdk bot commented Feb 26, 2025

openjdk bot commented Feb 26, 2025

lahodaj commented Feb 26, 2025

openjdk bot commented Feb 26, 2025

openjdk bot commented Feb 26, 2025

mlbridge bot commented Mar 4, 2025 •

edited

Loading

vicente-romero-oracle left a comment

vicente-romero-oracle Mar 4, 2025

bridgekeeper bot commented Apr 1, 2025

vicente-romero-oracle Apr 2, 2025

lahodaj Apr 2, 2025

vicente-romero-oracle left a comment

AlanBateman Apr 9, 2025

lahodaj Apr 10, 2025

vicente-romero-oracle left a comment

openjdk bot commented Apr 24, 2025

vicente-romero-oracle left a comment

mcimadamore Apr 29, 2025

mcimadamore Apr 29, 2025

lahodaj Apr 29, 2025

mcimadamore Apr 29, 2025

lahodaj Apr 29, 2025

lahodaj commented May 1, 2025

openjdk bot commented May 1, 2025

openjdk bot commented May 1, 2025

8344708: Implement JEP 511: Module Import Declarations #23801

8344708: Implement JEP 511: Module Import Declarations #23801

Conversation

lahodaj commented Feb 26, 2025 • edited by openjdk bot Loading

Progress

Issues

Reviewers

Reviewing

bridgekeeper bot commented Feb 26, 2025

openjdk bot commented Feb 26, 2025 • edited Loading

lahodaj commented Feb 26, 2025

openjdk bot commented Feb 26, 2025

openjdk bot commented Feb 26, 2025

lahodaj commented Feb 26, 2025

openjdk bot commented Feb 26, 2025

openjdk bot commented Feb 26, 2025

mlbridge bot commented Mar 4, 2025 • edited Loading

Webrevs

vicente-romero-oracle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bridgekeeper bot commented Apr 1, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vicente-romero-oracle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vicente-romero-oracle left a comment

Choose a reason for hiding this comment

openjdk bot commented Apr 24, 2025

vicente-romero-oracle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lahodaj commented May 1, 2025

openjdk bot commented May 1, 2025

openjdk bot commented May 1, 2025

lahodaj commented Feb 26, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Feb 26, 2025 •

edited

Loading

mlbridge bot commented Mar 4, 2025 •

edited

Loading