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