On Mon, 15 May 2023 07:13:49 GMT, Alan Bateman <al...@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
