From 779fe017ba2b373a81388fdf029ef8b863e17d02 Mon Sep 17 00:00:00 2001 From: Nate Biggs Date: Fri, 11 Aug 2023 20:59:30 +0000 Subject: [PATCH] [dart2js] Add Dart web speciailized expectation helpers. There are many failing Dart2JS language/library tests due to differences in backend implementations. We want to adopt a strategy that allows us to make explicit the different expectations we have of each backend. This adds logic for the 2 most common causes of these backend related failures (numbers and implicit checks) but we can add more of these as we discover more use cases. This is an iteration on https://dart-review.googlesource.com/c/sdk/+/293463 attempting to integrate feedback from that change. Change-Id: Ie3a1954066199695d92881497e940385467c9a12 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311780 Commit-Queue: Nate Biggs Reviewed-by: Stephen Adams Reviewed-by: Lasse Nielsen --- pkg/compiler/lib/src/options.dart | 8 ++++++++ pkg/expect/lib/expect.dart | 25 +++++++++++++++++++++++++ tests/corelib/apply_test.dart | 9 +++++++-- tests/corelib/hash_set_test.dart | 30 +++++++++++++++++++++++++----- tests/corelib_2/apply_test.dart | 9 +++++++-- tests/corelib_2/hash_set_test.dart | 30 +++++++++++++++++++++++++----- 6 files changed, 97 insertions(+), 14 deletions(-) diff --git a/pkg/compiler/lib/src/options.dart b/pkg/compiler/lib/src/options.dart index fdc3e404ad3..c3d5e1f994a 100644 --- a/pkg/compiler/lib/src/options.dart +++ b/pkg/compiler/lib/src/options.dart @@ -1231,6 +1231,14 @@ class CompilerOptions implements DiagnosticOptions { } environment['dart.web.assertions_enabled'] = '$enableUserAssertions'; + environment['dart.tool.dart2js'] = '${true}'; + // Eventually pragmas and commandline flags should be aligned so that users + // setting these flag is equivalent to setting the relevant pragmas + // globally. + // See: https://github.com/dart-lang/sdk/issues/49475 + // https://github.com/dart-lang/sdk/blob/main/pkg/compiler/doc/pragmas.md + environment['dart.tool.dart2js.primitives:trust'] = '$trustPrimitives'; + environment['dart.tool.dart2js.types:trust'] = '$omitImplicitChecks'; } /// Returns `true` if warnings and hints are shown for all packages. diff --git a/pkg/expect/lib/expect.dart b/pkg/expect/lib/expect.dart index 06f47782269..27e1a6350b1 100644 --- a/pkg/expect/lib/expect.dart +++ b/pkg/expect/lib/expect.dart @@ -12,6 +12,31 @@ bool get hasUnsoundNullSafety => const [] is List; /// Whether the program is running with sound null safety. bool get hasSoundNullSafety => !hasUnsoundNullSafety; +/// Whether the test was compiled by Dart2JS as production web code. +/// +/// Production code unsafely omits some type checks that are required by the +/// language, under the assumption that production code will have no type +/// errors. +/// +/// For example an invalid implicit down-cast like +/// `dynamic d = 3; String s = d;` might not be caught in production code. +final dart2jsProductionMode = const bool.fromEnvironment( + 'dart.tool.dart2js.types:trust', + defaultValue: false); + +/// Whether the test is running in a web environment, using JavaScript numbers. +/// +/// In web compiled code, Dart integers are compiled to JavaScript numbers, +/// and have different ranges and behaviors of some operations, compared to +/// native compiled code (see [int] for more details about the difference +/// between native numbers and numbers on the web). +/// +/// For example, using JavaScript numbers, an `int` value like `1` also +/// implements `double` and is the same object as `1.0`. In native numbers, +/// those values are two different objects, and integers do not implement +/// `double`. +final bool webNumbers = identical(1, 1.0); + /// Expect is used for tests that do not want to make use of the /// Dart unit test library - for example, the core language tests. /// Third parties are discouraged from using this, and should use diff --git a/tests/corelib/apply_test.dart b/tests/corelib/apply_test.dart index 364f2f4bef8..2c7d2842054 100644 --- a/tests/corelib/apply_test.dart +++ b/tests/corelib/apply_test.dart @@ -72,6 +72,11 @@ main() { // Test that apply works on callable objects when it is passed to a method // that expects Function (and not dynamic). - Expect.throws(() => testList(42, new Callable(), [13, 29])); //# 01: ok - testListTyped(42, new Callable(), [13, 29]); //# 02: ok + if (dart2jsProductionMode) { + testList(42, new Callable(), [13, 29]); + } else { + Expect.throws(() => testList(42, new Callable(), [13, 29])); + } + + testListTyped(42, new Callable(), [13, 29]); } diff --git a/tests/corelib/hash_set_test.dart b/tests/corelib/hash_set_test.dart index 965ace17dd7..c3a38858570 100644 --- a/tests/corelib/hash_set_test.dart +++ b/tests/corelib/hash_set_test.dart @@ -65,7 +65,9 @@ testSet(Set newSet(), Set newSetFrom(Iterable from)) { { // Check concurrent modification - Set set = newSet()..add(0)..add(1); + Set set = newSet() + ..add(0) + ..add(1); { // Test adding before a moveNext. @@ -301,15 +303,33 @@ void testIdentitySet(Set create()) { // All compile time constants are identical to themselves. var constants = [ double.infinity, - double.nan, -0.0, //# 01: ok - 0.0, 42, "", null, false, true, #bif, testIdentitySet + double.nan, + -0.0, + 0.0, + 42, + "", + null, + false, + true, + #bif, + testIdentitySet ]; set.addAll(constants); - Expect.equals(constants.length, set.length); + if (webNumbers) { + // 0.0 and -0.0 are identical in JS. + Expect.equals(constants.length - 1, set.length); + Expect.isTrue( + set.containsAll(constants.where((e) => !(e is double && e.isNaN))), + "constants: $set"); + } else { + Expect.equals(constants.length, set.length); + Expect.isTrue(set.containsAll(constants), "constants: $set"); + } for (var c in constants) { + // identical(double.nan, double.nan) == false in JS. + if (webNumbers && c is double && c.isNaN) continue; Expect.isTrue(set.contains(c), "constant: $c"); } - Expect.isTrue(set.containsAll(constants), "constants: $set"); set.clear(); var m1 = new Mutable(1); diff --git a/tests/corelib_2/apply_test.dart b/tests/corelib_2/apply_test.dart index a5563fbc81a..981925adbc0 100644 --- a/tests/corelib_2/apply_test.dart +++ b/tests/corelib_2/apply_test.dart @@ -75,6 +75,11 @@ main() { // Test that apply works on callable objects when it is passed to a method // that expects Function (and not dynamic). - Expect.throws(() => testList(42, new Callable(), [13, 29])); //# 01: ok - testListTyped(42, new Callable(), [13, 29]); //# 02: ok + if (dart2jsProductionMode) { + testList(42, new Callable(), [13, 29]); + } else { + Expect.throws(() => testList(42, new Callable(), [13, 29])); + } + + testListTyped(42, new Callable(), [13, 29]); } diff --git a/tests/corelib_2/hash_set_test.dart b/tests/corelib_2/hash_set_test.dart index 511dd50dcea..0181794526e 100644 --- a/tests/corelib_2/hash_set_test.dart +++ b/tests/corelib_2/hash_set_test.dart @@ -67,7 +67,9 @@ testSet(Set newSet(), Set newSetFrom(Iterable from)) { { // Check concurrent modification - Set set = newSet()..add(0)..add(1); + Set set = newSet() + ..add(0) + ..add(1); { // Test adding before a moveNext. @@ -303,15 +305,33 @@ void testIdentitySet(Set create()) { // All compile time constants are identical to themselves. var constants = [ double.infinity, - double.nan, -0.0, //# 01: ok - 0.0, 42, "", null, false, true, #bif, testIdentitySet + double.nan, + -0.0, + 0.0, + 42, + "", + null, + false, + true, + #bif, + testIdentitySet ]; set.addAll(constants); - Expect.equals(constants.length, set.length); + if (webNumbers) { + // 0.0 and -0.0 are identical in JS. + Expect.equals(constants.length - 1, set.length); + Expect.isTrue( + set.containsAll(constants.where((e) => !(e is double && e.isNaN))), + "constants: $set"); + } else { + Expect.equals(constants.length, set.length); + Expect.isTrue(set.containsAll(constants), "constants: $set"); + } for (var c in constants) { + // identical(double.nan, double.nan) == false in JS. + if (webNumbers && c is double && c.isNaN) continue; Expect.isTrue(set.contains(c), "constant: $c"); } - Expect.isTrue(set.containsAll(constants), "constants: $set"); set.clear(); var m1 = new Mutable(1);