[dart2wasm] Normalize `FutureOr<T>` as nullable when `T` is nullable
The type normalization rules specify that `FutureOr<T>?` is normalized
to `FutureOr<T>` when `T` is nullable. However, it's more practical
for subtype testing if the declared nullability on the runtime
representation of the `FutureOr` type reflects the true nullability
(nullable if the `FutureOr` is declared nullable or its type argument
is nullable), rather than being normalized as per the spec.
This changes the static and dynamic normalization rules in dart2wasm
thus and compensates by computing the proper spec normalization when
the type is converted to a string.
The added test exposed a number of bugs in DDC and the VM:
https://github.com/dart-lang/sdk/issues/53175
https://github.com/dart-lang/sdk/issues/53737
https://github.com/dart-lang/sdk/issues/53738
Change-Id: I0ad0a09fe935ccbd3eb65e6958c958d29e0bb088
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/320821
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Aske Simon Christensen <askesc@google.com>
diff --git a/pkg/dart2wasm/lib/types.dart b/pkg/dart2wasm/lib/types.dart
index 7919d63..85683b9 100644
--- a/pkg/dart2wasm/lib/types.dart
+++ b/pkg/dart2wasm/lib/types.dart
@@ -478,8 +478,12 @@
// The type is normalized, and remains a `FutureOr` so now we normalize its
// nullability.
+ // Note: We diverge from the spec here and normalize the type to nullable if
+ // its type argument is nullable, since this simplifies subtype checking.
+ // We compensate for this difference when converting the type to a string,
+ // making the discrepancy invisible to the user.
final declaredNullability = s.nullability == Nullability.nullable
- ? Nullability.nonNullable
+ ? Nullability.nullable
: type.declaredNullability;
return FutureOrType(s, declaredNullability);
}
diff --git a/sdk/lib/_internal/wasm/lib/type.dart b/sdk/lib/_internal/wasm/lib/type.dart
index 106245c..368b9f3 100644
--- a/sdk/lib/_internal/wasm/lib/type.dart
+++ b/sdk/lib/_internal/wasm/lib/type.dart
@@ -199,7 +199,9 @@
s.write("<");
s.write(typeArgument);
s.write(">");
- if (isDeclaredNullable) s.write("?");
+ // Omit the question mark if the type argument is nullable in order to match
+ // the specified normalization rules for `FutureOr` types.
+ if (isDeclaredNullable && !typeArgument.isDeclaredNullable) s.write("?");
return s.toString();
}
}
@@ -706,8 +708,12 @@
WasmObjectArray<_Type>.literal([typeArgument]));
}
+ // Note: We diverge from the spec here and normalize the type to nullable if
+ // its type argument is nullable, since this simplifies subtype checking.
+ // We compensate for this difference when converting the type to a string,
+ // making the discrepancy invisible to the user.
bool declaredNullability =
- typeArgument.isDeclaredNullable ? false : isDeclaredNullable;
+ isDeclaredNullable || typeArgument.isDeclaredNullable;
return _FutureOrType(declaredNullability, typeArgument);
}
@@ -881,20 +887,7 @@
if (identical(s, t)) return true;
// Compare nullabilities:
- if (s.isDeclaredNullable && !t.isDeclaredNullable) {
- // [s] is declared nullable and is therefore definitely nullable. [t] is
- // declared non-nullable and is therefore non-nullable or has undetermined
- // nullability, unless it is `FutureOr<T>` where `T` is nullable.
- nullable:
- {
- _Type t2 = t;
- while (t2.isFutureOr) {
- t2 = t2.as<_FutureOrType>().typeArgument;
- if (t2.isDeclaredNullable) break nullable;
- }
- return false;
- }
- }
+ if (s.isDeclaredNullable && !t.isDeclaredNullable) return false;
// Left bottom:
if (s.isBottom) return true;
diff --git a/tests/language/language_dart2js.status b/tests/language/language_dart2js.status
index 889a7ec..431d4bb 100644
--- a/tests/language/language_dart2js.status
+++ b/tests/language/language_dart2js.status
@@ -19,6 +19,7 @@
call/implicit_tearoff_exceptions_test: SkipByDesign # No argument type checks in production mode, issue 45528
control_flow_collections/for_non_bool_condition_test: Crash # Issue 36442
regress/regress45428_test: SkipByDesign # No argument type checks in production mode, issue 45528
+type_object/futureor_tostring_test: Skip # Type syntax is not preserved in minified mode
[ $compiler == dart2js && $runtime == chromeOnAndroid ]
override_field_test/02: Slow, Pass # TODO(kasperl): Please triage.
diff --git a/tests/language/type_object/futureor_tostring_test.dart b/tests/language/type_object/futureor_tostring_test.dart
new file mode 100644
index 0000000..3cadcd6
--- /dev/null
+++ b/tests/language/type_object/futureor_tostring_test.dart
@@ -0,0 +1,139 @@
+// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Tests the `toString` of a `Type` object for a type that is,
+// or that contains, a `FutureOr` type, including whether its trailing `? s
+// are correct.
+//
+// The `toString` is performed on the **NORM**-normalized type,
+// and **NORM**(`FutureOr<T>?`), with `N` bein **NORM**(`T`),
+// may be either `FutureOr<N>` or `FutureOr<N>?`, depending on
+// whether `T`/`N` is nullable or not.
+//
+// This test checks that the `toString` result always matches
+// the correctly normalized type format.
+// Also checks for types `T` where normalization removes or changes
+// the `FutureOr`.
+
+import "dart:async" show FutureOr;
+import "package:expect/expect.dart";
+
+void main() {
+ // Generally assumes that `Type.toString()` mathches the canonical
+ // format of the type, as used in the language specification,
+ // with a leading name, `<...>` around type arguments,
+ // and trailing `?` for the nullable union types,
+ // and that it uses the same name for the same type every time,
+ // even if the names have been minified.
+ //
+ // That is the case for all current implementations, in all modes,
+ // even if `Type.toString()` doesn't document it.
+
+ Match match = RegExp(r"^(.+)<(.+)<(.+)>>$")
+ .matchAsPrefix((A<FutureOr<String>>).toString())!;
+ String a = match[1]!;
+ String fo = match[2]!;
+
+ Expect.equals("$String", match[3]); // Sanity check.
+
+ void testType<T>(String expected) {
+ Expect.equals(expected, "$T");
+ }
+
+ void testRuntimeType(Object o, String expected) {
+ Expect.equals(expected, "${o.runtimeType}");
+ }
+
+ void testNullable<B>() {
+ assert(null is B);
+ var b = "$B";
+ // Equal to `B` for top types and `Object`.
+ var fob = "${FutureOr<B>}";
+ if (topOrObject<B>()) {
+ Expect.equals(b, fob);
+ } else if (isNull<B>()) {
+ Expect.equals("${Future<B>}?", fob);
+ } else {
+ Expect.equals("$fo<$b>", fob);
+ }
+ testType<FutureOr<B>>(fob);
+ testType<FutureOr<B>?>(fob);
+ testType<FutureOr<B?>>(fob);
+ testType<FutureOr<B?>?>(fob);
+ testType<FutureOr<N<B?>>>(fob);
+ testType<N<FutureOr<B>?>>(fob);
+
+ var afob = "$a<$fob>";
+ testType<A<FutureOr<B>?>>(afob);
+ testType<A<FutureOr<B?>>>(afob);
+ testType<A<FutureOr<B?>?>>(afob);
+ testType<A<FutureOr<N<B?>>>>(afob);
+ testType<A<N<FutureOr<B>?>>>(afob);
+ testRuntimeType(A<FutureOr<B>?>(), afob);
+ testRuntimeType(A<FutureOr<B?>>(), afob);
+ testRuntimeType(A<FutureOr<B?>?>(), afob);
+ testRuntimeType(A<FutureOr<N<B?>>>(), afob);
+ testRuntimeType(A<N<FutureOr<B>?>>(), afob);
+ }
+
+ void testNotNullable<B>({bool alsoTestNullable = true}) {
+ assert(null is! B);
+ assert(!top<B>());
+ var b = "$B";
+ var fob = "${FutureOr<B>}";
+ if (never<B>()) {
+ Expect.equals("${Future<B>}", fob);
+ } else if (topOrObject<B>()) {
+ // Object.
+ Expect.equals(b, fob);
+ } else {
+ Expect.equals("$fo<$b>", fob);
+ }
+ testType<FutureOr<B>>(fob);
+ testType<FutureOr<B>?>("$fob?");
+ testType<N<FutureOr<B>?>>("$fob?");
+ testType<A<FutureOr<B>?>>("$a<$fob?>");
+ testType<A<N<FutureOr<B>?>>>("$a<$fob?>");
+ testRuntimeType(A<FutureOr<B>?>(), "$a<$fob?>");
+ testRuntimeType(A<N<FutureOr<B>?>>(), "$a<$fob?>");
+
+ if (alsoTestNullable) testNullable<B?>();
+ }
+
+ testNotNullable<String>();
+
+ testNullable<dynamic>();
+ testNullable<void>();
+
+ testNotNullable<Object>();
+
+ testNullable<Null>();
+
+ testNotNullable<Never>();
+
+ // Some more cases that could be non-trivial, but really aren't.
+ testNotNullable<Future<Object>>();
+ testNotNullable<Future<Object?>>();
+
+ // Test nested `FutureOr`, since normalization of that is somewhat
+ // non-trivial.
+ testNotNullable<FutureOr<String>>(alsoTestNullable: false);
+ testNullable<FutureOr<String>?>();
+ testNullable<FutureOr<String?>>();
+ testNullable<FutureOr<String?>?>();
+
+ // A singleton record with nullable field, for good measure.
+ testNotNullable<(String?,)>();
+}
+
+// Supports nesting nullables, like `N<N<N<int>>>`, which cannot
+// syntactically be written as `int???`.
+typedef N<T> = T?;
+
+bool top<T>() => <Object?>[] is List<T>;
+bool topOrObject<T>() => <Object>[] is List<T>;
+bool never<T>() => <T>[] is List<Never>;
+bool isNull<T>() => <T>[] is List<Null> && null is T;
+
+class A<T> {}
