On Mon, 20 Nov 2023 13:56:13 GMT, Christian Stein <cst...@openjdk.org> wrote:
>> src/java.base/share/classes/java/lang/Class.java line 4839:
>>
>>> 4837: @PreviewFeature(feature=PreviewFeature.Feature.IMPLICIT_CLASSES)
>>> 4838: @CallerSensitive
>>> 4839: public Method getMainMethod() {
>>
>> This is a new addition in the last few days. It's somewhat surprising as the
>> launch protocol has never really surfaced in the API, there's nothing to
>> identify the main class for example. I'm in two minds on surfacing this but
>> it does make the java launcher code much simpler.
>>
>> Class.getMethod/getMethods return public methods, the proposed getMainMethod
>> may return a public or non-public method and may walk the class hierarchy.
>> So I'm not sure about the naming, it feels more like a findMainMethod to
>> emphasize the search or just mainMethod.
>>
>> The javadoc probably needs to be fleshed out a bit more to specify how the
>> main method is found. It currently says "be declared this class's
>> hierarchy" (typo) but it has to say more and make it clear that it may
>> return a Method with a declared class that may be a superclass.
>>
>> "(@jls 8.4.2)", I assume that should be "{@jls 8.4.2}".
>>
>> The return description says "the candidate main method". This begs the
>> question if there are other candidates, how do I find them?
>>
>> The SM permission check looks right, it has to be the same as
>> getDeclaredMethod.
>
>> [...] it does make the java launcher code much simpler.
>
> Yes. That's a very nice effect of surfacing the launch protocol directyl in
> `java.lang.Class`
>
>> So I'm not sure about the naming, it feels more like a `findMainMethod` to
>> emphasize the search or just `mainMethod`.
>
> +1 for `findMainMethod`, and also an explicit `Optional<Method>` as the
> return type.
>
> Or `Optional<Method> mainMethod()` similar to
> [ModuleDescriptor::mainClass](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/module/ModuleDescriptor.html#mainClass())
Updated.
Optional is stylizing preference. Since this method is used frequently (almost
always) at startup, loading extra classes should probably be avoided. It's not
hard to simply `Optional.ofNullable(cls.findMainMethod())`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16461#discussion_r1399597600
