RFR: JDK-8306112 Implementation of JEP 445: Flexible Main Methods and Anonymous Main Classes (Preview) [v3]

[Jan Lahoda]

On Thu, 27 Apr 2023 18:21:56 GMT, Jim Laskey <jlaskey at openjdk.org> wrote:

>> Add flexible main methods and anonymous main classes to the Java language.
>
> Jim Laskey has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits:
> 
>  - Merge branch 'master' into 8306112
>  - PreviewFeatures.isEnabled()
>  - Clean up isPreview
>  - Missing exception
>  - Corrections
>  - Update VM.java
>  - Clean up testing
>  - Update TestJavacTaskScanner.java
>  - Merge branch 'master' into 8306112
>  - Clean up
>  - ... and 4 more: https://git.openjdk.org/jdk/compare/96cdf93b...53a5321d

src/java.base/share/classes/jdk/internal/misc/MainMethodFinder.java line 64:

> 62: 
> 63:             for (Method method : refc.getDeclaredMethods()) {
> 64:                 int argc = method.getParameterCount();

Nit: unused variable?

src/java.base/share/classes/jdk/internal/misc/MainMethodFinder.java line 147:

> 145:             }
> 146: 
> 147:             return mainClass.getMethod("main", String[].class);

Nit: could return `mainMethod` here, correct?

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Enter.java line 453:

> 451:         }
> 452:         if (!SourceVersion.isIdentifier(simplename) || SourceVersion.isKeyword(simplename)) {
> 453:             log.error(null, Errors.BadFileName(simplename));

Suggestion:

            log.error(tree.pos(), Errors.BadFileName(simplename));

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Enter.java line 462:

> 460:         for (JCTree def : tree.defs) {
> 461:             if (def.hasTag(Tag.PACKAGEDEF)) {
> 462:                 log.error(null, Errors.AnonymousMainClassShouldNotHavePackageDeclaration);

Suggestion:

                log.error(def.pos(), Errors.AnonymousMainClassShouldNotHavePackageDeclaration);

src/jdk.compiler/share/classes/com/sun/tools/javac/launcher/Main.java line 438:

> 436:             Class<?> appClass = Class.forName(mainClassName, true, cl);
> 437:             Method main = MainMethodFinder.findMainMethod(appClass);
> 438:             if (!PreviewFeatures.isEnabled() && (!isStatic(main) || !isPublic(main))) {

It seems one can run a main method without parameters without `--enable-preview` using this codepath. That is presumably not intended.

src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java line 627:

> 625:         }
> 626: 
> 627:         public boolean isAnonymousMainClass() {

This method seems a bit confusing to me. I believe the fields and methods will be stripped from `defs` if/when the wrapping class is created, which will mean this method will start to return `false`, no? It overall does not seem like a generally useful predicate.

(If I understand this correctly, if we created the wrapping class in parser neither of these two methods would be needed.)

src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java line 640:

> 638:             // Find anonymous main class.
> 639:             for (JCTree def : defs) {
> 640:                 if (def.hasTag(CLASSDEF)) {

Nit, in cases like this, we lately tend to write `def instanceof JCClassDecl cls`, although we understand this way to check the AST is only safe inside javac.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13689#discussion_r1180360702
PR Review Comment: https://git.openjdk.org/jdk/pull/13689#discussion_r1180360546
PR Review Comment: https://git.openjdk.org/jdk/pull/13689#discussion_r1180378753
PR Review Comment: https://git.openjdk.org/jdk/pull/13689#discussion_r1180378611
PR Review Comment: https://git.openjdk.org/jdk/pull/13689#discussion_r1180375691
PR Review Comment: https://git.openjdk.org/jdk/pull/13689#discussion_r1180389813
PR Review Comment: https://git.openjdk.org/jdk/pull/13689#discussion_r1180392123