RFR: JDK-8306112 Implementation of JEP 445: Unnamed Classes and Instance Main Methods (Preview) [v13]

[Jim Laskey]

On Mon, 15 May 2023 07:13:49 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Jim Laskey has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update VirtualParser.java
>
> src/java.base/share/classes/jdk/internal/misc/MainMethodFinder.java line 134:
> 
>> 132: 
>> 133:     /**
>> 134:      * {@return priority main method or null if none found}
> 
> "or null if none found", is that out of date?

Changed

> src/java.base/share/classes/jdk/internal/misc/MainMethodFinder.java line 156:
> 
>> 154: 
>> 155:             List<Method> mains = new ArrayList<>();
>> 156:             gatherMains(mainClass, mainClass, mains);
> 
> Instead of gatherMains, did you consider first looking for static main(String[], then static main()? Asking because I expected to only see the walk up the hierarchy when looking for an instance main.

99.99% of the time it will be a single method in a shallow hierarchy, so cost its low. Only reason I broke out public static main was to ensure performance for existing code was the same.

> src/java.base/share/classes/sun/launcher/LauncherHelper.java line 872:
> 
>> 870: 
>> 871:     // Check the existence and signature of main and abort if incorrect
>> 872:     public static void validateMainClass(Class<?> mainClass) {
> 
> Is there a reason that this is changed to public, maybe left over from a previous iteration?

Remnant. Changed.

> src/java.base/share/classes/sun/launcher/LauncherHelper.java line 904:
> 
>> 902: 
>> 903:         if (!PreviewFeatures.isEnabled()) {
>> 904:             if (!isStatic || !isPublic || noArgs) {
> 
> You can use && here and avoid the nested if.

Easier to see when removing preview code.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13689#discussion_r1194138214
PR Review Comment: https://git.openjdk.org/jdk/pull/13689#discussion_r1194137852
PR Review Comment: https://git.openjdk.org/jdk/pull/13689#discussion_r1194141538
PR Review Comment: https://git.openjdk.org/jdk/pull/13689#discussion_r1194141140