On Fri, 5 May 2023 09:28:04 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Jim Laskey has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Anonymous main classes renamed to unnamed classes
>> - Add test
>
> src/java.base/share/classes/jdk/internal/misc/MainMethodFinder.java line 139:
>
>> 137: public static Method findMainMethod(Class<?> mainClass) throws
>> NoSuchMethodException {
>> 138: try {
>> 139: Method mainMethod = mainClass.getMethod("main",
>> String[].class);
>
> Hello Jim, I think this specific line is trying to find a `public static void
> main(String[])` method from the launched class. In the current form of this
> implementation, this has the potential of returning a non-static `public void
> main(String[])` from here. I think a `isStatic(mainMethod)` would be needed
> here before returning this method as the found method.
The problem with modifying behaviour that has existed since JDK 1.0 is that it
has to be compatible in unexpected ways. If you look at the code modification:
- mainMethod = mainClass.getMethod("main", String[].class);
+ mainMethod = MainMethodFinder.findMainMethod(mainClass);
you can see that nothing has really changed. The mainMethod was always possibly
non-static and the caller produced an appropriate error message if that was the
case. There is the rub. If I didn't do it the way I have, I would also have to
modify 130+ tests that expect to fail with a "method is not static" message as
oppose to a "method not found" error message (there are other related issues to
args and return types). After spending several days wading through a handful of
these ancient tests, I realized I did not have time to complete the
modifications necessary before targeting. And, there is also the issue of what
happens to tests and expectations downstream. For a preview feature, behaviour
has to remain the same when preview is not enabled.
I was also hopeful to be able to unify the _java launcher_ and the _source code
launcher_. Similar problem with error messaging, though the tests for the
source code launcher are fewer and newer.
Unifying the launchers would also introduce another downstream issue. Since
launching would occur from Java code, addition frames appear on the stack.
Seems there are a several tests, tools and debuggers that rely on "main" being
the first frame (even if hidden by printStackTrace). Ugh.
Before the next cycle, I hope to unify the messaging, have someone clean up the
tests and figure out a scheme that unifies the launching.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13689#discussion_r1186216248
