RFR: 8344708: Compiler Implementation of Module Import Declarations [v7]

Maurizio Cimadamore mcimadamore at openjdk.org
Tue Apr 29 10:16:49 UTC 2025


On Mon, 28 Apr 2025 13:26:30 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> This is a patch to finalize the module imports feature. Please see:
>> https://bugs.openjdk.org/browse/JDK-8344700
>
> Jan Lahoda has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 13 commits:
> 
>  - Merge branch 'master' into JDK-8344708
>  - Adjusting ToolProviderTest to the updated persistence handling.
>  - Merge branch 'master' into JDK-8344708
>  - Reflecting review feedback: cleanup formatting in ModuleInfo.
>  - Reflecting review feedback - avoiding hardcoded constants.
>  - Fixing test.
>  - Cleanup, fixing tests.
>  - Adjusting JShell defaults.
>  - Fixing tests.
>  - Cleanup - updated specification will permit requires transitive java.base; for all classfile versions; java.se no longer participates in preview.
>  - ... and 3 more: https://git.openjdk.org/jdk/compare/1f228e55...da39d0ef

src/jdk.jshell/share/classes/jdk/internal/jshell/tool/Startup.java line 345:

> 343:         boolean hasModuleImports = source == null ||
> 344:                                    Feature.MODULE_IMPORTS.allowedInSource(source);
> 345:         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).

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23801#discussion_r2065991188


More information about the core-libs-dev mailing list