Need a well-defined configurable-import key for targets that use JavaScript numbers

[rakudrama]

We have historically used libraries for 'detecting' platforms and importing different versions

Long ago this used to work

import 'javascript_foo.dart'
  if (dart.library.io) 'native_64bit_foo.dart';

Then a stubbed-out version of dart:io was made available for dart2js (see #46917), so this no longer works.

The next iteration was to feature-test on dart:html:

import 'native_64bit_foo.dart'
  if (dart.library.html) 'javascript_foo.dart';

This works unless the program is compiled with the dart2js command-line --server-mode which uses the dart2js_server platform dill which does not contain this library. There are important apps that run on nodejs that need this platform.

Libraries with js in the name, e.g. dart:js are available on wasm platforms which use 64 bit ints, making these libraries unsuited for feature detection.

What is needed is a configurable-import key that directly maps to when JavaScript numbers are being used versus 'full' 64-bit ints, regardless of platform libraries included.

• [ ] Define and document the configuation key(s) that match the different number semantics. I suggest dart.NumberSemantics.js (and maybe dart.NumberSemantics.vm) after the enum

• [ ] Update the compilers to provide the configuration keys. Perhaps this can be done by adding the configuration enum to the Target

@lrhn I'm assigning to you for triage since it might be urgent as it is blocking Int64 work. I'm hoping that we can something reliable for for Dart 3.3.

@johnniwinther WDYT about this happening automatically? At first look it seems like environmentDefines and NumberSemantics are provided too late to direct the import resolution.

/cc @osa1

[sigmundch]

@rakudrama - I think it's worth noting that dart.library.io returns false in web compilers today, even though we support importing dart:io unconditionally. This was the main reason the library specification file allows to specify "supported: false" on individual libraries.

This doesn't necessarily address the issue, though, since dart.library.io is also false in wasm.

[sigmundch]

Given that we already use the libraries spec to include these configuration variables, it may be worth considering using that same file as the location to specify extra properties that get exposed as constants for configuration specific imports.

[lrhn]

I do want something like that, have wanted it for years (https://github.com/dart-lang/sdk/issues/41764#issuecomment-623887516, probably more places.)

I'm hoping that we can [have] something reliable for for Dart 3.3.

... so that might be optimistic.

We use dart.library.foo as the import tests, because it allow us to de feacture detection, rather than "platform detection + feature inference". That only works for features that are either there or not. We only allow those for configurable imports to make modular and reusable compilation possible. It limits the number of possible different configurations to a known, small, number.

The ones we'll need to convince are the compiler writers (DDC in particular, since it's the most modular of our compilers) and the internal build system maintainers. The implementation should be trivial. (It's the naming that is hard.)

For number semantics, or other things that have different behavior of the same feature, it would make sense to allow a configurable import to ask specifically for whether it's dart.options.numbers.web or dart.options.numbers.int64 (where exactly one is set to true on any compilation).

That should be "non-risky", within the constraints above, since the number semantics are strongly linked to the actual platform we're compile to. We're not going to share compilation artifacts between platforms that disagree on numbers. (I hope.)

I could also use dart.options.mode.development vs dart.options.mode.production (depends on compiler choice and flags). Maybe even dart.options.mode.debug (asserts enabled, probably implies development mode).

I'm a little more worried about those, because it makes it easier to have development-mode tests, which have assertions enabled, and which then do not check the code that actually runs in production.

The slipper-slope problem with mode flags is that you can have an infinity of them, it just depends on how granular you want to be about the differences. If we change web number semantics in the next version (say a better implementation of identical), would we need to have a new mode flag for that? (No, because it's versioned, you should use versioning. But know that somewhere out there, is someone who wants their package to work with every Dart version ever, using the same code, using conditional compilation to match every little difference between versions. Helping them is not the goal here.)

All in all, fine by me. Great even. If we can sell it.

[sigmundch]

... I suggest dart.NumberSemantics.js (and maybe dart.NumberSemantics.vm) after the enum

Minor nit: may I suggest we keep the snake_case convention on these constants? For example dart.number_semantics.vm. I also like Lasse's suggestion dart.options.numbers.web.

As Lasse's said, the biggest hurdle I see with this change is to propagate it to each build system we have (bazel and build_runner). Those systems impose limitations to keep the number of configurations bounded. That said, when it comes to number semantics it is special: we are not adding more configurations, we are just exposing different ways to choose between them.

Because of that, I have no reservations here and am very supportive of making these changes. Similarly, I believe our maintainers of build-systems will be supportive of this change.

cc @jakemac53 @emmanuel-p

I could also use dart.options.mode.development vs dart.options.mode.production (depends on compiler choice and flags). Maybe even dart.options.mode.debug (asserts enabled, probably implies development mode).

I do have reservations here :smile: - I'd prefer we don't push for these flags becoming exposed as constants.

Technically we already have dart.vm.product, dart.vm.profile, etc, and we've seen cases where teams are looking for an equivalent flag for the web. So while I agree it would be nice to have a backend agnostic way of representing these modes, I'd prefer them to be link-time flags instead and disallow them from conditional imports.

In dart2js we have an ongoing effort to move away from using const fromEnvironment as much as possible and instead move towards a link-time flag if that's reasonable. Note that link-time flags can still be used by AOT and dart2js to tree-shake and optimize, we just limit them to not be available as constants.

[rakudrama]

For number semantics, or other things that have different behavior of the same feature, it would make sense to allow a configurable import to ask specifically for whether it's dart.options.numbers.web or dart.options.numbers.int64 (where exactly one is set to true on any compilation).

The name web is going to become confusing with running in the browser compiled with wasm and having 64-bit integers. I would make the name specific to the JavaScript targets, e.g. dart.numbers.javascript. I'm not keen on the .options., as you don't get to choose with DDC or dart2js, and give the illusion that other 'options' may be testable.

I could also use dart.options.mode.development vs dart.options.mode.production (depends on compiler choice and flags). Maybe even dart.options.mode.debug (asserts enabled, probably implies development mode).

I'm a little more worried about those, because it makes it easier to have development-mode tests, which have assertions enabled, and which then do not check the code that actually runs in production.

I have other worries too (1) these are not actual flags to dart2js, but constellations of other options that could change (2) the individual options are also controlled in a more local scope via pragmas, so it is not an indicator of anything.

All in all, fine by me. Great even. If we can sell it.

I'm hoping that we can [have] something reliable for for Dart 3.3.

... so that might be optimistic.

It is blocking the previously mentioned change to Int64, which is why I am a little nervous about the rest of the discussion here. If we can agree that number semantics is a reasonable thing to test with a configurable import and on a name, it should possible to implement the javascript-positive test in the two compiler that have that.

[sigmundch]

Re the blocking issue - @rakudrama in package:dart2js_runtime_metrics you had a different workaround, would it be faster to unblock the Int64 change with that instead?

To be honest, I'd prefer we move forward with the proposal here, just trying to also keep the other effort unblocked and possibly moving in parallel.

[rakudrama]

Re the blocking issue - @rakudrama in package:dart2js_runtime_metrics you had a different workaround, would it be faster to unblock the Int64 change with that instead?

There is a lot of luck involved in that (link) not becoming more seriously broken when wasm was added! It mildly broken because the VM import reports stub data {'runtime': 'vm'} and wasm currently uses that. We should have wasm report as {'runtime': 'wasm'}, but this would be low priority until there was something worth reporting.

To be honest, I'd prefer we move forward with the proposal here, just trying to also keep the other effort unblocked and possibly moving in parallel.

Me too, but I'll defer to Ömer.

@osa1 I think dart.library.ffi currently correlates with 64-bit ints. It would be prudent to double-check all the targets to be sure. If you decide to use this approach, I would add a TODO at the configurable import to reference an new library work-item issue that says to fix the condition and make that work-item issue reference this issue.

There is no plan currently to support dart:ffi on the JavaScript compilers but there has been a request and that would open up some interesting interop opportunities. If we did that, the hack would break.

[lrhn]

Not too sold on .options. either, but I do want some namespacing to keep the top-level dart.something as free as possible. If there are more "implementation differences" that we want to make available to configurable imports, I want them in the same namespace, which is why I opposed the top-level dart.number_semantics.web. Just have to find good name. Can't be hard 😱 .

Considered flags, but it's even more misleading than options, looking like something you can choose. Considered modes/mode. A little too vague. Considered semantics, but that sounds more like a difference between language versions. So, dart.platform_difference.numbers.web? (Ick.)

(Should we allow configurable imports based on available language versions, like dart.version.2.12, ..., dart.version.3.2 being true for the current SDK? Or would that just encourage people to try to support every version of Dart, at the same time, in the same code-base? Which would just be keeping old language versioned code alive for much longer than necessary.)

[lrhn]

Here is one attempted implementation: https://dart-review.googlesource.com/c/sdk/+/331700

Still running the tests, but works for VM and dart2js locally.

It is fairly complicated because the DartLibrarySupport object needs (or is implemented to) do fairly late lookups in the available libraries, and gets a lot of booleans provided at the latest possible moment, with the object only really being a way to override the final result of the mostly dynamic computation.

The DartLibrarySupport class is also not particularly well-named for extending with non-library related constants, but as long as they are used for imports, it's at least related.

[jakemac53]

As Lasse's said, the biggest hurdle I see with this change is to propagate it to each build system we have (bazel and build_runner). Those systems impose limitations to keep the number of configurations bounded. That said, when it comes to number semantics it is special: we are not adding more configurations, we are just exposing different ways to choose between them.

Because of that, I have no reservations here and am very supportive of making these changes. Similarly, I believe our maintainers of build-systems will be supportive of this change.

+1 I do not see an issue as long as the number of configurations is fixed. It doesn't sound to me like we are even adding additional configurations, simply exposing more information about the current configurations?

Both build_runner and bazel do not have to hard-code all the permutations of possible constants, only the combinations that actually exists in practice, which it sounds like would still be O(backends) with some low constant multiplier for sound/unsound null safety etc.

[osa1]

@lrhn should we close this in favor of #54785?

[lrhn]

Depends on the priority. If this is more urgent than waiting for us to find a single unified solution, then keep it open. Otherwise I'd consider closing this, and adding a comment on #54785 saying that this particular functionality is important. (In case we choose to expose only a limited number of features.)

So maybe it's better to keep this open, and comment on #54785 too.