Skip to content

Introduce a binary API with Scala functions in Reflect. #5161

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
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sjrd
Copy link
Member

@sjrd sjrd commented May 1, 2025

And make its internals independent of JS interop.
This way, it will be usable in Wasm without JS host.

We use new names for the methods, instead of overloads, because older compilers look for those methods by name only. If an older compiler gets a newer library on the classpath, and the methods with those names became overloaded, the compiler would crash.

In the commit, we do not change the compiler yet, to test that the deprecated methods are correct.

In the second commit, we change the compiler to use the new APIs.


This does not bring any immediate benefit. It's more for a long-term vision. But it also shouldn't hurt, besides the cost of maintaining a deprecated pair of methods. If we had had NewLambda all along, I think we would have designed this API using the Scala types to begin with. The Reflect object never pretended to be Scala-agnostic, since its public API is full of Scala types anyway.

@sjrd sjrd requested a review from gzm0 May 1, 2025 15:17
@sjrd sjrd force-pushed the reflect-use-newlambda branch from 6b00b7f to b223873 Compare May 3, 2025 09:16
@gzm0
Copy link
Contributor

gzm0 commented May 4, 2025

I think this is a worthwhile thing to do. Have you given any thought whether we should attempt to make the reflect API Scala agnostic as well? It seems it only relies on Scala only (i.e. non Scala.js IR) concepts superficially.

IDK when we first designed this, but it might have been well before the times of the Scala independent javalib. With it, I wonder if we should at least make the interface we design now Scala independent.

@sjrd
Copy link
Member Author

sjrd commented May 4, 2025

We can make the new registration API not use any Scala type. But it won't change the fact that the public API exposes Options, Lists and Seqs (through varargs). We could add new Java-friendly accessors. Whatever we do, however, that library will need a dependency on the Scala library.

We also need a Java type for the pair of Class to constructor data. Maybe a java.util.Map.Entry[_]?

@gzm0
Copy link
Contributor

gzm0 commented May 20, 2025

I've been thinking about this and I'm wondering if it would make more sense to specify registration in terms of IR opcodes. It's essentially functionality provided by the runtime.

But it won't change the fact that the public API exposes Options, Lists and Seqs (through varargs). We could add new Java-friendly accessors. Whatever we do, however, that library will need a dependency on the Scala library.

But couldn't we make the core implementation rely on Java only and keep the Scala interface an adapter?

We also need a Java type for the pair of Class to constructor data. Maybe a java.util.Map.Entry[_]?

Yeah... feels a bit of a stretch after having read the Javadoc, but maybe :-/

@sjrd
Copy link
Member Author

sjrd commented May 27, 2025

#5183 is a variant that relies on a Java-only API in the javalibintf.

@gzm0
Copy link
Contributor

gzm0 commented Jun 1, 2025

TY for the alternative in #5183. I'll comment here to keep the discussion in one thread.

The reflect API has been bothering me and I have been trying to pinpoint why. I think the core of it relates to the fact that, unlike most of the other things, the reflect API is not merely an interface to an existing construct (JS classes, dynamic imports, java buffers, etc.) but an entirely new / standalone feature.

IIUC it would not really be difficult to offer this in a completely separate compiler plugin (of course, since Scala.js core uses the feature, that's not an option).

From that point of view, I think we should explicitly discuss and decide whether we expect usages of scala.scalajs.reflect to be inter-compatible. As in: Do we expect to have all reflective instantiation of a Scala.js linking unit / application to be handled by a single registry / mechanism.

If yes, we should push relevant pieces down to at least javalib, maybe even IR.
If no, having a Scala only API (like in this PR) is fine.

To kick-off that decision making, I've assembled an initial list of consequences of either decision:

What Isolated Global Importance
3rd party lib interop Low High ???1
Impl. Complexity Low High Mid
Code size Higher Lower Low?
Mem usage Higher Lower Low?
Performance ??? ??? Irrelevant

From this cursory look, it seems that it might not be necessary / a good idea to attempt to generalize this. @sjrd WDYT? Did I miss anything in the list?

Footnotes

  1. It is not even clear to me whether this is desirable at all, or if libraries should keep the exact means of reflective instantiation an implementation detail.

@sjrd
Copy link
Member Author

sjrd commented Jun 1, 2025

The Reflect API definitely is one of the not-so-great things in the overall design. IMO it has always been a "necessary evil", which we need for testing frameworks to be viable. And well, we do want testing frameworks to work out of the box.

From that perspective, the centralization was good because all testing frameworks could rely on a unique feature, without having to introduce their own compiler plugin and registry. Compiler plugins have a significant cost, since they are not transitively resolved. Every user of a testing framework would need an addCompilerPlugin in their jsSettings, which is not great.

An alternative design would have been to:

  • special-case what we need fore core in the JUnit compiler plugin (which we need anyway for other reasons), and
  • offer a more feature-full library+plugin in a separate library that all testing frameworks would depend on (like portable-scala-reflect, though that one is pure library--no plugin--so it is transparent to users).

Another complication is that the feature is not actually implementable in "compiler plugin space". It needs to generate static initializers in classes. No other feature of the language, even seen at the backend, allows to generate static initializers (without the side effect of exporting a fake top-level static field). To allow compiler plugins to enable Reflect, we would need to introduce another feature to generate static initializers.

Altogether, I think the centralization of Reflect is not accidental. It seems to be the least-bad design that does not cause ripple effects through testing frameworks and their users. Not because we want 3rd party lib interop (I agree that this is not desirable per se) but because we want 3rd library libraries to have something that does not require more external out-of-language tooling.

@gzm0
Copy link
Contributor

gzm0 commented Jun 8, 2025

Thanks for the write-up here. I think it surfaces an important distinction: Reflect support is primarily a feature of the compilation subsystem, not the IR subsystem.

In that sense, it has very similar status than @JSExport annotations or export forwarders (except that these don't need additional runtime support beyond what the IR offers anyway).

However, I feel this discussion actually shows that 3rd party library interop is crucial here, but not how I originally thought:

If a library marks a type as @EnableReflectiveInstantiation it is (theoretically) ok, if no other compilation unit can reflectively instantiate these types. However, it is absolutely crucial for the correct working that other compilation units opt-in subtypes of this type into reflective instantiation (of whichever way the orignal compilation unit requested).

I think even just that makes a very strong case that we should aim to centralize the feature so it is available across compilation ecosystems (lest we are willing to sacrifice interoperability between them).

We will probably have to live with the fact that some of the compilation time artifacts (e.g. annotations) are scalajs namespaced, but probably that's OK.

WDYT? (we probably still need to have the discussion how centralize, but that's the next step).

@sjrd
Copy link
Member Author

sjrd commented Jun 8, 2025

I agree.

Indeed, that's a good analysis. The opt-in via subtype across libraries is crucial here.

sjrd added 2 commits July 28, 2025 16:17
And make its internals independent of JS interop.
This way, it will be usable in Wasm without JS host.

We use new names for the methods, instead of overloads, because
older compilers look for those methods by name only. If an older
compiler gets a newer library on the classpath, and the methods
with those names became overloaded, the compiler would crash.

In this commit, we do not change the compiler yet, to test that the
deprecated methods are correct.
@sjrd sjrd force-pushed the reflect-use-newlambda branch from 8741ebd to f12e99f Compare July 28, 2025 14:35
@sjrd
Copy link
Member Author

sjrd commented Jul 28, 2025

Coming back to this to try and move it forward.

Given that (it seems) we agree that centralization is desirable, it seems either this PR or #5183 would be appropriate (in some form or another). This PR centralizes among Scala compilers and frameworks, while #5183 centralizes even across several languages. Of course "several languages" is still hypothetical, but it's been a good design framework for the IR, so it's probably still relevant.

Even in the Java API variant, it is still left up to compilers "according to the rules of the source language" what classes are actually registered. In that sense, it's not clear that there would be true interoperability across languages. For example, if we reimplemented JUnit in a hypothetical Java-to-SJSIR compiler, the mechanism to register classes for reflective instantiation would not really make sense across languages. That's an argument in favor of the Scala API, and other languages would have to offer their own reflect mechanism and API.

On the other hand, if we did have a Java-to-SJSIR compiler one day, we would probably want JUnit, of all things, to be usable equally from Java and Scala. The implementation could be either in Scala or Java, but the interface of the reflect system would have to be such that Java-produced SJSIR can use it. So that's an argument in favor the Java API for Reflect.


AFAIKT, Kotlin does not use a Reflect API like we do to support its testing frameworks. Instead, they have their own framework-agnostic API to write tests in the first place. Then the compilers for JVM and JS seem to produce different code for them. In particular, on JS, it seems they generate calls to the methods of a FrameworkAdapter, which directly registers test suites and individual tests. So IIUC, their compiler has dedicated support for test suites, with a test framework abstraction. Whereas we have support for basic reflection, and we let testing frameworks use that underlying mechanism.

So, basically there's not much to learn from their design, in our context.


Overall, I don't think there's a winner either way. Given that, and even though I kind of like the Java-API design, I'd suggest we stay with the current PR, with a Scala API. It's a much smaller diff compared to the status quo. And if in the future, we really want a Java API, we could still take the next step and go there.

WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants