Skip to content

Classfiles generated with -scalajs need to work properly on the JVM since they can be classloaded from a macro call #10918

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

Open
smarter opened this issue Dec 26, 2020 · 4 comments

Comments

@smarter
Copy link
Member

smarter commented Dec 26, 2020

In several places in the compiler pipeline we assume that we only need to think about the Scala.js backend semantics when ctx.settings.scalajs.value is true. For example, because JS is single-threaded, we emit all lazy vals as if they were @threadUnsafe:
https://github.com/lampepfl/dotty/blob/2a32d6ae8a2667930d58997e9bc73f621045bf69/compiler/src/dotty/tools/dotc/transform/LazyVals.scala#L89
But this is incorrect: even when we're running with -scalajs, we still emit .class files, and these files can actually end up being loaded by the compiler since macros are run by classloading. So we need to be very careful to only interpret ctx.settings.scalajs.value to mean "emit something which will have the correct semantics using both the JVM and JS backend". I had a quick look at usages of this setting and most of them look like they don't break JVM semantics (just prevent some minor optimizations so not a big deal), though I have doubt about:
https://github.com/lampepfl/dotty/blob/2a32d6ae8a2667930d58997e9bc73f621045bf69/compiler/src/dotty/tools/dotc/transform/CompleteJavaEnums.scala#L113-L119
I don't know if Java enum members have to be static fields, or if it's just an optimization (/cc @bishabosha).

@bishabosha
Copy link
Member

I don't know if Java enum members have to be static fields, or if it's just an optimization (/cc @bishabosha).

Java enums are accessed from Java by public static field access so that is a requirement for Java interop.

@smarter smarter modified the milestones: 3.0.0-RC1, 3.1.0 Jan 4, 2021
@smarter
Copy link
Member Author

smarter commented Jan 4, 2021

The enum issue is probably not a big deal, it seems that to trigger it you'd need .java files in a scala.js project that happen to be used only from macro code. The lazy val issue is more serious but also not critical since multi-threaded macros are (hopefully) rare, since the thread-safe lazy val implementation will be rewritten post-3.0 (#7140), it makes sense to wait for that to happen before tackling this.

@nicolasstucki
Copy link
Contributor

@sjrd will we have a fix for this issue for 3.1.0 or should we remove it from that milestone?

@sjrd
Copy link
Member

sjrd commented Aug 26, 2021

Let's remove it. So far we don't even have a concrete use case where this is problematic.

@sjrd sjrd removed this from the 3.1.0 milestone Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants