From 1026367e56dcd11ef8cf3e0266d8ded8eaa340d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20A=C4=9Facan?= Date: Mon, 14 Jul 2025 05:20:03 -0700 Subject: [PATCH] [dart2wasm] Fix switch-case comparisons with dynamic scrutinee MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #61098. Issue: https://github.com/dart-lang/sdk/issues/61098 Change-Id: Iabd1b3ffde6e4eafbdf98a2bb04f5949b799fcf6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/439940 Commit-Queue: Ömer Ağacan Reviewed-by: Slava Egorov --- pkg/dart2wasm/lib/code_generator.dart | 38 +++-- .../switch/switch_comparisons_1_test.dart | 95 +++++++++++ .../switch/switch_comparisons_2_test.dart | 149 ++++++++++++++++++ 3 files changed, 273 insertions(+), 9 deletions(-) create mode 100644 tests/language/switch/switch_comparisons_1_test.dart create mode 100644 tests/language/switch/switch_comparisons_2_test.dart diff --git a/pkg/dart2wasm/lib/code_generator.dart b/pkg/dart2wasm/lib/code_generator.dart index 08db21a715b..455113aedc9 100644 --- a/pkg/dart2wasm/lib/code_generator.dart +++ b/pkg/dart2wasm/lib/code_generator.dart @@ -4335,10 +4335,11 @@ class SwitchInfo { e is L || e is NullLiteral || (e is ConstantExpression && - (e.constant is C || e.constant is NullConstant) && - (translator.hierarchy.isSubInterfaceOf( - translator.classForType(codeGen.dartTypeOf(e)), - switchExprClass)))); + ((e.constant is C && + (translator.hierarchy.isSubInterfaceOf( + translator.classForType(codeGen.dartTypeOf(e)), + switchExprClass))) || + e.constant is NullConstant))); // Type objects should be compared using `==` rather than identity even // though the specification is not very clear about it. In language versions @@ -4367,7 +4368,7 @@ class SwitchInfo { nonNullableType = translator.runtimeTypeType; nullableType = translator.runtimeTypeTypeNullable; compare = (switchExprLocal, pushCaseExpr) { - // Virtual call to `Object.==`. + // Virtual call to `Type.==`. codeGen._virtualCall( node, translator.coreTypes.objectEquals, _VirtualCallKind.Call, (functionType) { @@ -4377,11 +4378,16 @@ class SwitchInfo { }, useUncheckedEntry: false); }; } else if (switchExprType is DynamicType) { - // Object equality switch + // Per spec, compare with ` == `. For performance, + // if we know that the cases all have the same type, we call the case + // expression's `==` implementation directly (instead of virtually calling + // `Object.==`). + // + // Note: this could be improved by directly calling a different `==` in + // each of the cases based on the case value. For now we only directly + // call a `==` if all of the cases have a compatible type. nonNullableType = translator.topTypeNonNullable; nullableType = translator.topType; - - // Per spec, compare with ` == `. final Member equalsMember; if (check()) { equalsMember = translator.boxedBoolEquals; @@ -4390,7 +4396,17 @@ class SwitchInfo { } else if (check()) { equalsMember = translator.jsStringEquals; } else { - equalsMember = translator.coreTypes.identicalProcedure; + compare = (switchExprLocal, pushCaseExpr) { + // Virtual call to `Object.==`. + codeGen._virtualCall(node, codeGen.translator.coreTypes.objectEquals, + _VirtualCallKind.Call, (functionType) { + codeGen.b.local_get(switchExprLocal); + }, (functionType, paramInfo) { + pushCaseExpr(); + }, useUncheckedEntry: false); + }; + _initializeSpecialCases(node); + return; } final equalsMemberSignature = @@ -4462,6 +4478,10 @@ class SwitchInfo { }; } + _initializeSpecialCases(node); + } + + void _initializeSpecialCases(SwitchStatement node) { // Special cases defaultCase = node.cases .cast() diff --git a/tests/language/switch/switch_comparisons_1_test.dart b/tests/language/switch/switch_comparisons_1_test.dart new file mode 100644 index 00000000000..b01bb65f9bd --- /dev/null +++ b/tests/language/switch/switch_comparisons_1_test.dart @@ -0,0 +1,95 @@ +// Copyright (c) 2025, 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. + +// This is a regression test for the SDK issue #61098, originally reported as +// Flutter issue #171803. +// +// Tests that the right comparison method (`identical` or `operator ==`) is used +// when comparing strings in switch statements and expressions. +// +// This is the smaller version of the original repro. More tests in +// `switch_comparisons_2_test.dart`. + +import 'dart:convert'; +import 'package:expect/expect.dart'; + +void main() { + _option = utf8.decoder.convert([ + 80, + 104, + 111, + 116, + 111, + 103, + 114, + 97, + 112, + 104, + 105, + 99, + 66, + 111, + 120, + ]); + + Expect.isTrue(infoSwitchCaseWithNull); + Expect.isTrue(infoSwitchCaseWithoutNull); + Expect.isTrue(infoSwitchExpression); + Expect.isTrue(infoIfCase); + Expect.isTrue(infoIfEquals); +} + +const kTypeString = 'PhotographicBox'; + +String? _option; + +String? get type => _option; + +bool get infoSwitchCaseWithNull { + switch (type) { + case kTypeString: + return true; + case null: + throw 'Type is null on SWITCH CASE WITH NULL'; + default: + throw 'Unexpected type on SWITCH CASE WITH NULL: $type'; + } +} + +bool get infoSwitchCaseWithoutNull { + switch (type) { + case kTypeString: + return true; + default: + throw 'Unexpected type on SWITCH CASE WITHOUT NULL: $type'; + } +} + +bool get infoSwitchExpression { + return switch (type) { + kTypeString => true, + null => throw 'Type is null with SWITCH EXPRESSION', + _ => throw 'Unexpected type with SWITCH EXPRESSION: $type', + }; +} + +bool get infoIfCase { + if (type case kTypeString) { + return true; + } else if (type case null) { + throw 'Type is null with IF CASE'; + } else { + throw 'Unexpected type with IF CASE: $type'; + } +} + +bool get infoIfEquals { + if (type == kTypeString) { + return true; + } else if (type == null) { + throw 'Type is null with IF'; + } else { + throw 'Unexpected type with IF: $type'; + } +} diff --git a/tests/language/switch/switch_comparisons_2_test.dart b/tests/language/switch/switch_comparisons_2_test.dart new file mode 100644 index 00000000000..021e64e96f1 --- /dev/null +++ b/tests/language/switch/switch_comparisons_2_test.dart @@ -0,0 +1,149 @@ +// Copyright (c) 2025, 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 equalities used when scrutinee is `dynamic` and cases have the same +// type and different types, with and without nulls. + +import 'dart:convert'; +import 'package:expect/expect.dart'; + +void main() { + var s = utf8.decoder.convert([ + 80, + 104, + 111, + 116, + 111, + 103, + 114, + 97, + 112, + 104, + 105, + 99, + 66, + 111, + 120, + ]); + + Expect.isTrue(test1(s)); + Expect.isTrue(test2(s)); + Expect.isTrue(test3(s)); + Expect.isTrue(test4(s)); + + // Test the same thing, but with another special cased type in dart2wasm + // (int). + var i = int.parse('123'); + + Expect.isTrue(test5(i)); + Expect.isTrue(test6(i)); + Expect.isTrue(test7(i)); + Expect.isTrue(test8(i)); +} + +const kTypeString = 'PhotographicBox'; + +// Dynamic scrutinee with mixed type cases. +Object? test1(dynamic v) { + switch (v) { + case 1: + throw 'int case in switch without null'; + case kTypeString: + return true; + default: + throw 'default case in switch without null'; + } +} + +// Dynamic scrutinee with mixed type cases and null. +Object? test2(dynamic v) { + switch (v) { + case 1: + throw 'int case in switch with null'; + case null: + throw 'null case'; + case kTypeString: + return true; + default: + throw 'default case in switch with null'; + } +} + +// Dynamic scrutinee with just string cases. +Object? test3(dynamic v) { + switch (v) { + case 'blah': + throw 'string case'; + case kTypeString: + return true; + default: + throw 'default case in string switch'; + } +} + +// Dynamic scrutinee with just string and null cases. +Object? test4(dynamic v) { + switch (v) { + case 'blah': + throw 'string case'; + case null: + throw 'null case'; + case kTypeString: + return true; + default: + throw 'default case in string switch'; + } +} + +// Dynamic scrutinee with mixed type cases. +Object? test5(dynamic v) { + switch (v) { + case kTypeString: + throw 'string case'; + case 123: + return true; + default: + throw 'default case in switch without null'; + } +} + +// Dynamic scrutinee with mixed type cases and null. +Object? test6(dynamic v) { + switch (v) { + case kTypeString: + throw 'string case'; + case null: + throw 'null case'; + case 123: + return true; + default: + throw 'default case in switch with null'; + } +} + +// Dynamic scrutinee with just int cases. +Object? test7(dynamic v) { + switch (v) { + case 0: + throw 'wrong int'; + case 123: + return true; + default: + throw 'default case in string switch'; + } +} + +// Dynamic scrutinee with just int and null cases. +Object? test8(dynamic v) { + switch (v) { + case 0: + throw 'wrong int'; + case null: + throw 'null case'; + case 123: + return true; + default: + throw 'default case in string switch'; + } +}