Java parser: add support for JEP-445 #18790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Open

povder wants to merge 1 commit into scala:main

from povder:i18584

Open

Java parser: add support for JEP-445 #18790

povder wants to merge 1 commit into scala:main from povder:i18584

Conversation

povder

Copy link

Contributor

@povder povder commented Oct 30, 2023

Starting a draft PR for #18584

@povder povder force-pushed the i18584 branch 3 times, most recently from 4053e13 to a55e5bc Compare

November 5, 2023 06:02

@povder povder mentioned this pull request

Nov 5, 2023

Install Java 21 lampepfl/dotty-ci#24

Merged

@povder


 Java parser: add support for unnamed classes (JEP-445)

01540b0

Fixes scala#18584

@povder povder force-pushed the i18584 branch from a55e5bc to 01540b0 Compare

November 5, 2023 20:19

@povder

Copy link

Contributor Author

povder commented Nov 5, 2023

This is now ready for review. To run newly introduced compilation tests in CI, lampepfl/dotty-ci#24 needs to be released.

@povder povder marked this pull request as ready for review

November 5, 2023 20:25

@smarter

Copy link

Member

smarter commented Nov 23, 2023 •

edited

Loading

Thanks for working on this! Here are my thoughts after a first look at it, let me know if you'd like more guidance.

For reference, while the JEP itself lacks some details, spec changes are available:
- JLS changes: https://cr.openjdk.org/~gbierman/jep463/jep463-20231030/specs/implicitly-declared-classes-instance-main-methods-jls.html
- JVMS changes: https://cr.openjdk.org/~gbierman/jep463/jep463-20231030/specs/implicitly-declared-classes-instance-main-methods-jvms.html
Because all this stuff is still experimental, we need some mechanism to not enable it by default, normally we'd use @experimental but that only works for Scala code, so it's not clear to me what we should do here, adding a temporary -Y flag might be our best bet /cc @sjrd
If I understand the spec correctly, user code should not be able to refer to an unnamed class by its compiler-synthesized name. Luckily we do have a flag to express this already: Invisible, so the JavaParser should set this flag, and we need tests to ensure it works properly here.
The changes to the main method resolution in the JVMS will need to be reflected in a few places (I guess this could be handled in a separate PR since just being able to compile code would already be useful):
- https://github.com/lampepfl/dotty/blob/main/compiler/src/dotty/tools/dotc/config/Platform.scala#L45-L49
- https://github.com/lampepfl/dotty/blob/052bce44c6eccc7b6a0bf6cba3141e79a205e68a/compiler/src/dotty/tools/dotc/config/JavaPlatform.scala#L23-L28
- https://github.com/lampepfl/dotty/blob/052bce44c6eccc7b6a0bf6cba3141e79a205e68a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala#L256-L259
- It's possible sbt will need to be adjusted to correctly deal with the mainClass we feed it from ExtractAPI, but that's something we can worry about later as long as we keep this stuff experimental on our side.

@sjrd

Copy link

Member

sjrd commented Nov 23, 2023

A -Y flag would be perfectly fine here. We don't have to deal with pollution of experimental products since the actual products are created by javac.