dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.08k stars 1.56k forks source link

Test runner should distinguish different web backends #56654

Open mkustermann opened 1 week ago

mkustermann commented 1 week ago

Our test framework has the ability to check for errors on specific backends, for example it allows this

  check(0x7FFF00001111F000);
  check(0x7FFF00001111FC00);
  check(0x7FFF00001111FE00);
  //    ^
  // [web] The integer literal 0x7FFF00001111FE00 can't be represented exactly in JavaScript.

This test - testing that on web one gets an error/warning - is only really valid for our to-JavaScript compilers.

In dart2wasm we have wrap-around int64 semantics just like the VM.

=> So we'd want to split up [web] into [web_js] and [web_wasm].

/cc @athomas @munificent

dart-github-bot commented 1 week ago

Summary: The current test runner uses a single [web] tag for both JavaScript and WebAssembly backends. This is inaccurate because WebAssembly has different integer behavior than JavaScript. The issue proposes splitting [web] into [web_js] and [web_wasm] for more precise backend-specific error reporting.

athomas commented 1 week ago

So in the int64 semantics dart2wasm should be treated as a non-web platform? So in the proposed model, the test would have a [web_js] annotation, but no [web_wasm] annotation. Are there examples also examples where [web_wasm] would be used? A lot of uses of [web] are in JSInterop, is that were [web_wasm] would be needed?

Also, perhaps wasm will find non-web uses as well, so maybe we could just introduce a separate [wasm].

mkustermann commented 1 week ago

A lot of uses of [web] are in JSInterop, is that were [web_wasm] would be needed?

Yes, some actually web-specific things like dart:js_interop are for js & wasm web backends.

It may also make sense to instead of using backends one uses the features of backends, e.g. [js_numbers] vs [int64_numbers], ...

munificent commented 1 week ago

It may also make sense to instead of using backends one uses the features of backends, e.g. [js_numbers] vs [int64_numbers], ...

Yeah, I like this naming. Now that we have WASM, it's not really web number semantics, it's JS number semantics.

The other thing you could consider doing is forking these tests into separate ones based on number semantics and then use the "Requirements=" feature that the test runner supports to filter those tests by number semantics. @sigmundch and @kallentu recently worked on extending the test runner to have features for things like number semantics. Look for Siggy's doc "Updates to dart testing and its guidelines" for details.

lrhn commented 1 week ago

I'd prefer to keep the test categories to [cfe] and [analyzer], drop [web] again, and use the Requirements= feature to limit failure tests to where they fail.

The other extreme would be a full set of flags like // [cfe:web:js], // [cfe:web:wasm], // [cfe:native:aot], etc., so we can really pin down all differences in error behavior between platforms.

We shouldn't have that many differences between platforms, not even in errors. (We shouldn't even have big differences in error messages between CFE and Analyzer, IMO.)

The few JS-number error tests should be able to use Requirements= and have one version for each behavior, and then we don't need to introduce any more flags.