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

[Maurizio Cimadamore]

On Tue, 29 Apr 2025 10:12:07 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> 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).

(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?)

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

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