[dart2js] Fix as-check type registration. We would expect failing as checks to throw in all modes (unless --omit-as-casts is provided). However, the new test program fails in production mode. This is because we are not registering the type usage of the function's type parameter. This leads us to drop the as check completely later on. We should be doing subtype checks with nullability if we want to consider an as test omitted. Golem patch results: https://golem.corp.goog/Comparison?repository=dart#targetA%3Ddart2js%3BmachineTypeA%3Dlinux-x64%3BrevisionA%3D107840%3BpatchA%3Dnatebiggs--dart2js--Fix-as-check-type-registration.-7%3BtargetB%3Ddart2js%3BmachineTypeB%3Dlinux-x64%3BrevisionB%3D107839%3BpatchB%3DNone Fixed: https://github.com/dart-lang/sdk/issues/54419 Change-Id: If93f78a939a690f05f5398c1a5ca971df1fc9243 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/343821 Reviewed-by: Mayank Patke <fishythefish@google.com> Reviewed-by: Stephen Adams <sra@google.com> Commit-Queue: Nate Biggs <natebiggs@google.com>
diff --git a/pkg/compiler/lib/src/ir/impact_data.dart b/pkg/compiler/lib/src/ir/impact_data.dart index 314e42c..df94d21 100644 --- a/pkg/compiler/lib/src/ir/impact_data.dart +++ b/pkg/compiler/lib/src/ir/impact_data.dart
@@ -222,8 +222,8 @@ void handleAsExpression(ir.AsExpression node, ir.DartType operandType, {bool? isCalculatedTypeSubtype}) { if (isCalculatedTypeSubtype ?? - typeEnvironment.isSubtypeOf(operandType, node.type, - ir.SubtypeCheckMode.ignoringNullabilities)) { + typeEnvironment.isSubtypeOf( + operandType, node.type, ir.SubtypeCheckMode.withNullabilities)) { // Skip unneeded casts. return; }
diff --git a/pkg/compiler/lib/src/ir/static_type.dart b/pkg/compiler/lib/src/ir/static_type.dart index 2269da9..3b4040e 100644 --- a/pkg/compiler/lib/src/ir/static_type.dart +++ b/pkg/compiler/lib/src/ir/static_type.dart
@@ -1320,7 +1320,7 @@ // Check if the calculated operandType is a subtype of the type specified // in the `as` expression. final isCalculatedTypeSubtype = typeEnvironment.isSubtypeOf( - operandType, node.type, ir.SubtypeCheckMode.ignoringNullabilities); + operandType, node.type, ir.SubtypeCheckMode.withNullabilities); if (!isCalculatedTypeSubtype && operand is ir.VariableGet && !_invalidatedVariables.contains(operand.variable)) {
diff --git a/pkg/compiler/test/closure/data/generic.dart b/pkg/compiler/test/closure/data/generic.dart index 3fd1f77..354f8f8 100644 --- a/pkg/compiler/test/closure/data/generic.dart +++ b/pkg/compiler/test/closure/data/generic.dart
@@ -56,12 +56,10 @@ } var local2 = - /*prod.hasThis*/ - /*spec.fields=[S,this],free=[S,this],hasThis*/ + /*fields=[S,this],free=[S,this],hasThis*/ (o) { return - /*prod.hasThis*/ - /*spec.fields=[S,this],free=[S,this],hasThis*/ + /*fields=[S,this],free=[S,this],hasThis*/ () => Map<T, S>(); }; return local2(local<double>());
diff --git a/pkg/compiler/test/closure/data/list_literal_untested_class.dart b/pkg/compiler/test/closure/data/list_literal_untested_class.dart index c401959..26f724c 100644 --- a/pkg/compiler/test/closure/data/list_literal_untested_class.dart +++ b/pkg/compiler/test/closure/data/list_literal_untested_class.dart
@@ -9,8 +9,7 @@ /*member: A.method:hasThis*/ @pragma('dart2js:noInline') method() { - /*spec.fields=[this],free=[this],hasThis*/ - /*prod.hasThis*/ + /*fields=[this],free=[this],hasThis*/ dynamic local() => <T>[]; return local; }
diff --git a/pkg/compiler/test/closure/data/list_literal_untested_method.dart b/pkg/compiler/test/closure/data/list_literal_untested_method.dart index 57a4c3d..cc4e8ec 100644 --- a/pkg/compiler/test/closure/data/list_literal_untested_method.dart +++ b/pkg/compiler/test/closure/data/list_literal_untested_method.dart
@@ -6,7 +6,7 @@ @pragma('dart2js:noInline') method<T>() { - /*spec.fields=[T],free=[T]*/ + /*fields=[T],free=[T]*/ dynamic local() => <T>[]; return local; }
diff --git a/pkg/compiler/test/closure/data/map_literal_untested_class.dart b/pkg/compiler/test/closure/data/map_literal_untested_class.dart index 7cd568b..6b17fa5 100644 --- a/pkg/compiler/test/closure/data/map_literal_untested_class.dart +++ b/pkg/compiler/test/closure/data/map_literal_untested_class.dart
@@ -9,8 +9,7 @@ /*member: A.method:hasThis*/ @pragma('dart2js:noInline') method() { - /*spec.fields=[this],free=[this],hasThis*/ - /*prod.hasThis*/ + /*fields=[this],free=[this],hasThis*/ dynamic local() => <T, int>{}; return local; }
diff --git a/pkg/compiler/test/closure/data/map_literal_untested_method.dart b/pkg/compiler/test/closure/data/map_literal_untested_method.dart index 3ddf2c3..050351c 100644 --- a/pkg/compiler/test/closure/data/map_literal_untested_method.dart +++ b/pkg/compiler/test/closure/data/map_literal_untested_method.dart
@@ -6,7 +6,7 @@ @pragma('dart2js:noInline') method<T>() { - /*spec.fields=[T],free=[T]*/ + /*fields=[T],free=[T]*/ dynamic local() => <T, int>{}; return local; }
diff --git a/pkg/compiler/test/codegen/data/array_add.dart b/pkg/compiler/test/codegen/data/array_add.dart index 81ecb40..e2f7e01 100644 --- a/pkg/compiler/test/codegen/data/array_add.dart +++ b/pkg/compiler/test/codegen/data/array_add.dart
@@ -12,7 +12,7 @@ return t1; }*/ /*prod.member: test1:function() { - var t1 = []; + var t1 = A._setArrayType([], type$.JSArray_int); t1.push(1); return t1; }*/
diff --git a/pkg/compiler/test/codegen/literal_list_test.dart b/pkg/compiler/test/codegen/literal_list_test.dart index ae346c3..96eb940 100644 --- a/pkg/compiler/test/codegen/literal_list_test.dart +++ b/pkg/compiler/test/codegen/literal_list_test.dart
@@ -18,12 +18,18 @@ main() { runTest() async { await compile(TEST_ONE, entry: 'foo', check: (String generated) { - Expect.isTrue(generated.contains('print([1, 2]);'), - "Code pattern 'print([1, 2]);' not found in\n$generated"); - Expect.isTrue(generated.contains('print([3]);'), - "Code pattern 'print([3]);' not found in\n$generated"); - Expect.isTrue(generated.contains('print([4, 5]);'), - "Code pattern 'print([4, 5]);' not found in\n$generated"); + Expect.isTrue( + generated.contains('A.print(A._setArrayType([1, 2], t1));'), + "Code pattern 'A.print(A._setArrayType([1, 2], t1));' " + "not found in\n$generated"); + Expect.isTrue( + generated.contains('A.print(A._setArrayType([3], t1));'), + "Code pattern 'A.print(A._setArrayType([3], t1));' " + "not found in\n$generated"); + Expect.isTrue( + generated.contains('A.print(A._setArrayType([4, 5], t1));'), + "Code pattern 'A.print(A._setArrayType([4, 5], t1));' " + "not found in\n$generated"); }); }
diff --git a/pkg/compiler/test/rti/data/async_foreach.dart b/pkg/compiler/test/rti/data/async_foreach.dart index 09f4c48..7dfaa87 100644 --- a/pkg/compiler/test/rti/data/async_foreach.dart +++ b/pkg/compiler/test/rti/data/async_foreach.dart
@@ -7,7 +7,7 @@ import 'package:compiler/src/util/testing.dart'; /*spec.class: Class:explicit=[Class.T*],implicit=[Class.T],needsArgs,test*/ -/*prod.class: Class:needsArgs*/ +/*prod.class: Class:implicit=[Class.T],needsArgs,test*/ class Class<T> { method() { var list = <T>[];
diff --git a/pkg/compiler/test/rti/data/async_foreach_nonasync.dart b/pkg/compiler/test/rti/data/async_foreach_nonasync.dart index 99cb537..a464e0d 100644 --- a/pkg/compiler/test/rti/data/async_foreach_nonasync.dart +++ b/pkg/compiler/test/rti/data/async_foreach_nonasync.dart
@@ -7,6 +7,7 @@ import 'package:compiler/src/util/testing.dart'; /*spec.class: Class:explicit=[Class.T*],implicit=[Class.T],needsArgs,test*/ +/*prod.class: Class:implicit=[Class.T],needsArgs,test*/ class Class<T> { method() { var list = <T>[];
diff --git a/pkg/compiler/test/rti/data/generic_class_is2.dart b/pkg/compiler/test/rti/data/generic_class_is2.dart index 029adc6..00be9ee 100644 --- a/pkg/compiler/test/rti/data/generic_class_is2.dart +++ b/pkg/compiler/test/rti/data/generic_class_is2.dart
@@ -9,7 +9,7 @@ /*class: A:implicit=[List<A<C*>*>,List<A<C2*>*>]*/ class A<T> {} -/*spec.class: A1:implicit=[A1]*/ +/*class: A1:implicit=[A1]*/ class A1 implements A<C1> {} /*class: B:explicit=[B.T*],needsArgs,test*/
diff --git a/pkg/compiler/test/rti/data/generic_methods_dynamic_05.dart b/pkg/compiler/test/rti/data/generic_methods_dynamic_05.dart index 0cb1242..0d30b72 100644 --- a/pkg/compiler/test/rti/data/generic_methods_dynamic_05.dart +++ b/pkg/compiler/test/rti/data/generic_methods_dynamic_05.dart
@@ -9,20 +9,19 @@ // Test derived from language_2/generic_methods_dynamic_test/05 /*spec.class: global#JSArray:deps=[ArrayIterator,List],explicit=[JSArray,JSArray.E,JSArray<ArrayIterator.E>],implicit=[JSArray.E],needsArgs,test*/ -/*prod.class: global#JSArray:deps=[List],needsArgs*/ +/*prod.class: global#JSArray:deps=[List],implicit=[JSArray.E],needsArgs,test*/ /*spec.class: global#List:deps=[C.bar,JSArray.markFixedList],explicit=[List<B*>*,List<Object>,List<Object?>,List<String>?,List<markFixedList.T>],needsArgs,test*/ -/*prod.class: global#List:deps=[C.bar],explicit=[List<B*>*],needsArgs*/ +/*prod.class: global#List:deps=[C.bar],explicit=[List<B*>*],needsArgs,test*/ class A {} -/*spec.class: B:explicit=[List<B*>*],implicit=[B]*/ -/*prod.class: B:explicit=[List<B*>*]*/ +/*class: B:explicit=[List<B*>*],implicit=[B]*/ class B {} class C { /*spec.member: C.bar:explicit=[Iterable<bar.T*>*],implicit=[bar.T],needsArgs,selectors=[Selector(call, bar, arity=1, types=1)],test*/ - /*prod.member: C.bar:needsArgs,selectors=[Selector(call, bar, arity=1, types=1)]*/ + /*prod.member: C.bar:implicit=[bar.T],needsArgs,selectors=[Selector(call, bar, arity=1, types=1)],test*/ List<T> bar<T>(Iterable<T> t) => <T>[t.first]; }
diff --git a/pkg/compiler/test/rti/data/local_function_list_literal.dart b/pkg/compiler/test/rti/data/local_function_list_literal.dart index 0af6116..2ef3666 100644 --- a/pkg/compiler/test/rti/data/local_function_list_literal.dart +++ b/pkg/compiler/test/rti/data/local_function_list_literal.dart
@@ -7,11 +7,10 @@ import 'package:compiler/src/util/testing.dart'; /*spec.class: global#JSArray:deps=[ArrayIterator,List],explicit=[JSArray,JSArray.E,JSArray<ArrayIterator.E>],implicit=[JSArray.E],needsArgs,test*/ -/*prod.class: global#JSArray:deps=[List],needsArgs*/ +/*prod.class: global#JSArray:deps=[List],implicit=[JSArray.E],needsArgs,test*/ @pragma('dart2js:noInline') -/*spec.member: method:implicit=[method.T],needsArgs,test*/ -/*prod.member: method:needsArgs*/ +/*member: method:implicit=[method.T],needsArgs,test*/ method<T>() { return /*spec.*/ () => <T>[]; }
diff --git a/pkg/compiler/test/rti/data/local_function_map_literal.dart b/pkg/compiler/test/rti/data/local_function_map_literal.dart index 7130d69..32b5694 100644 --- a/pkg/compiler/test/rti/data/local_function_map_literal.dart +++ b/pkg/compiler/test/rti/data/local_function_map_literal.dart
@@ -7,7 +7,7 @@ import 'package:compiler/src/util/testing.dart'; /*spec.class: global#LinkedHashMap:deps=[Map],explicit=[LinkedHashMap<LinkedHashMap.K,LinkedHashMap.V>],implicit=[LinkedHashMap.K,LinkedHashMap.V],needsArgs,test*/ -/*prod.class: global#LinkedHashMap:deps=[Map],needsArgs*/ +/*prod.class: global#LinkedHashMap:deps=[Map],implicit=[LinkedHashMap.K,LinkedHashMap.V],needsArgs*/ @pragma('dart2js:noInline') /*spec.member: method:implicit=[method.T],needsArgs,test*/
diff --git a/pkg/compiler/test/rti/data/map_literal.dart b/pkg/compiler/test/rti/data/map_literal.dart index 60568fd..4d4f0d6 100644 --- a/pkg/compiler/test/rti/data/map_literal.dart +++ b/pkg/compiler/test/rti/data/map_literal.dart
@@ -4,13 +4,13 @@ // @dart = 2.7 -/*prod.class: global#Map:*/ +/*prod.class: global#Map:needsArgs*/ /*spec.class: global#Map:explicit=[Map,Map<Object?,Object?>],needsArgs,test*/ -/*prod.class: global#LinkedHashMap:deps=[Map]*/ +/*prod.class: global#LinkedHashMap:deps=[Map],implicit=[LinkedHashMap.K,LinkedHashMap.V],needsArgs*/ /*spec.class: global#LinkedHashMap:deps=[Map],explicit=[LinkedHashMap<LinkedHashMap.K,LinkedHashMap.V>],implicit=[LinkedHashMap.K,LinkedHashMap.V],needsArgs,test*/ -/*prod.class: global#JsLinkedHashMap:deps=[LinkedHashMap]*/ +/*prod.class: global#JsLinkedHashMap:deps=[LinkedHashMap],needsArgs*/ /*spec.class: global#JsLinkedHashMap:deps=[LinkedHashMap],explicit=[JsLinkedHashMap,JsLinkedHashMap.K,JsLinkedHashMap.V,void Function(JsLinkedHashMap.K,JsLinkedHashMap.V)],implicit=[JsLinkedHashMap.K],needsArgs,test*/ /*prod.class: global#double:*/
diff --git a/pkg/compiler/test/rti/data/map_to_set.dart b/pkg/compiler/test/rti/data/map_to_set.dart index 5aa278f..d33788b 100644 --- a/pkg/compiler/test/rti/data/map_to_set.dart +++ b/pkg/compiler/test/rti/data/map_to_set.dart
@@ -7,7 +7,7 @@ /*prod.class: global#Map:deps=[Class],needsArgs*/ /*spec.class: global#Map:deps=[Class],explicit=[Map,Map<Object?,Object?>],needsArgs,test*/ -/*prod.class: global#LinkedHashMap:deps=[Map],implicit=[LinkedHashMap.K],needsArgs*/ +/*prod.class: global#LinkedHashMap:deps=[Map],implicit=[LinkedHashMap.K,LinkedHashMap.V],needsArgs*/ /*spec.class: global#LinkedHashMap:deps=[Map],explicit=[LinkedHashMap<LinkedHashMap.K,LinkedHashMap.V>],implicit=[LinkedHashMap.K,LinkedHashMap.V],needsArgs,test*/ /*prod.class: global#JsLinkedHashMap:deps=[LinkedHashMap],implicit=[JsLinkedHashMap.K],needsArgs*/
diff --git a/tests/web/regress/issue/54419_test.dart b/tests/web/regress/issue/54419_test.dart new file mode 100644 index 0000000..854d682 --- /dev/null +++ b/tests/web/regress/issue/54419_test.dart
@@ -0,0 +1,17 @@ +// Copyright (c) 2024, 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. + +import 'package:expect/expect.dart'; + +// Ensure we don't omit as checks based on a subtype check ignoring nullability. + +void foo<T>(T? x) { + print(x as T); +} + +void main() { + if (hasSoundNullSafety) { + Expect.throws(() => foo<int>(null)); + } +}