diff --git a/runtime/tests/vm/dart/regress_52214_test.dart b/runtime/tests/vm/dart/regress_52214_test.dart new file mode 100644 index 00000000000..b85d5a53357 --- /dev/null +++ b/runtime/tests/vm/dart/regress_52214_test.dart @@ -0,0 +1,85 @@ +// 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. + +// Verifies that switch statement with non-integer expression and +// int case constants is handled correctly. +// Regression test for https://github.com/dart-lang/sdk/issues/52214. + +import 'package:expect/expect.dart'; + +String test1() { + num switcher = 3.0; + switch (switcher) { + case 3: + return 'in case(3)'; + default: + return 'in default'; + } +} + +String test2(Object switcher) { + switch (switcher) { + case 3: + return 'in case(3)'; + default: + return 'in default'; + } +} + +String test3(dynamic switcher) { + switch (switcher) { + case 3: + return 'in case(3)'; + default: + return 'in default'; + } +} + +String test4(switcher) { + switch (switcher) { + case 1: + return 'in case(1)'; + case 2: + return 'in case(2)'; + case 3: + return 'in case(3)'; + case 4: + return 'in case(4)'; + case 5: + return 'in case(5)'; + case 6: + return 'in case(6)'; + case 7: + return 'in case(7)'; + case 8: + return 'in case(8)'; + case 9: + return 'in case(9)'; + case 10: + return 'in case(10)'; + case 11: + return 'in case(11)'; + case 12: + return 'in case(12)'; + case 13: + return 'in case(13)'; + case 14: + return 'in case(14)'; + case 15: + return 'in case(15)'; + case 16: + return 'in case(16)'; + default: + return 'in default'; + } +} + +void main() { + Expect.equals('in case(3)', test1()); + Expect.equals('in case(3)', test2(3.0)); + Expect.equals('in case(3)', test3(3.0)); + Expect.equals('in case(5)', test4(5.0)); + Expect.equals('in case(7)', test4(7.0)); + Expect.equals('in default', test4(0.0)); +} diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc index 81240288a2d..28a7a397e86 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc @@ -4875,14 +4875,18 @@ Fragment StreamingFlowGraphBuilder::BuildSwitchStatement( SwitchBlock block(flow_graph_builder_, case_count); Fragment instructions = BuildExpression(); // read condition. - SkipOptionalDartType(); // skip expression type + const AbstractType* expression_type = &Object::dynamic_type(); + if (ReadTag() == kSomething) { + expression_type = &T.BuildType(); // read expression type. + } instructions += StoreLocal(TokenPosition::kNoSource, scopes()->switch_variable); instructions += Drop(); case_count = ReadListLength(); // read number of cases. - SwitchHelper helper(Z, pos, is_exhaustive, &block, case_count); + SwitchHelper helper(Z, pos, is_exhaustive, *expression_type, &block, + case_count); // Build the case bodies and collect the expressions into the helper // for the next step. @@ -5076,8 +5080,7 @@ Fragment StreamingFlowGraphBuilder::BuildOptimizedSwitchPrelude( TargetEntryInstr* then_entry; TargetEntryInstr* otherwise_entry; - const AbstractType& expression_type = - AbstractType::ZoneHandle(Z, helper->expression_class().RareType()); + const AbstractType& expression_type = helper->expression_type(); ASSERT(dart::SimpleInstanceOfType(expression_type)); Fragment instructions; diff --git a/runtime/vm/compiler/frontend/kernel_to_il.cc b/runtime/vm/compiler/frontend/kernel_to_il.cc index d94f95eb06f..a39ff4a685f 100644 --- a/runtime/vm/compiler/frontend/kernel_to_il.cc +++ b/runtime/vm/compiler/frontend/kernel_to_il.cc @@ -5119,6 +5119,36 @@ Fragment FlowGraphBuilder::BuildDoubleHashCode() { return body; } +SwitchHelper::SwitchHelper(Zone* zone, + TokenPosition position, + bool is_exhaustive, + const AbstractType& expression_type, + SwitchBlock* switch_block, + intptr_t case_count) + : zone_(zone), + position_(position), + is_exhaustive_(is_exhaustive), + expression_type_(expression_type), + switch_block_(switch_block), + case_count_(case_count), + case_bodies_(case_count), + case_expression_counts_(case_count), + expressions_(case_count), + sorted_expressions_(case_count) { + case_expression_counts_.FillWith(0, 0, case_count); + + if (expression_type.nullability() == Nullability::kNonNullable) { + if (expression_type.IsIntType() || expression_type.IsSmiType()) { + is_optimizable_ = true; + } else if (expression_type.HasTypeClass() && + Class::Handle(zone_, expression_type.type_class()) + .is_enum_class()) { + is_optimizable_ = true; + is_enum_switch_ = true; + } + } +} + int64_t SwitchHelper::ExpressionRange() const { const int64_t min = expression_min().AsInt64Value(); const int64_t max = expression_max().AsInt64Value(); @@ -5162,7 +5192,7 @@ SwitchDispatch SwitchHelper::SelectDispatchStrategy() { // binary search to avoid code size explosion. const double kJumpTableMaxHolesRatio = 1.0; - if (!is_optimizable()) { + if (!is_optimizable() || expressions().is_empty()) { // The switch is not optimizable, so we can only use linear scan. return kSwitchDispatchLinearScan; } @@ -5306,15 +5336,10 @@ void SwitchHelper::AddExpression(intptr_t case_index, expressions_.Add(SwitchExpression(case_index, position, value)); - if (is_optimizable_ || expression_class_ == nullptr) { - // Check the type of the expression for use in an optimized switch. - const Class& value_class = Class::ZoneHandle(zone_, value.clazz()); - if (expression_class_ == nullptr) { - expression_class_ = &value_class; - // Only integer and enum expressions can be used in an optimized switch. - is_optimizable_ = value.IsInteger() || value_class.is_enum_class(); - } else if (value_class.ptr() != expression_class_->ptr()) { - // At least one expression has a different type than the others. + if (is_optimizable_) { + // Check the type of the case expression for use in an optimized switch. + if (!value.IsInstanceOf(expression_type_, Object::null_type_arguments(), + Object::null_type_arguments())) { is_optimizable_ = false; } } diff --git a/runtime/vm/compiler/frontend/kernel_to_il.h b/runtime/vm/compiler/frontend/kernel_to_il.h index 4c2df3787a5..acb57446b1f 100644 --- a/runtime/vm/compiler/frontend/kernel_to_il.h +++ b/runtime/vm/compiler/frontend/kernel_to_il.h @@ -1038,22 +1038,13 @@ class SwitchHelper { SwitchHelper(Zone* zone, TokenPosition position, bool is_exhaustive, + const AbstractType& expression_type, SwitchBlock* switch_block, - intptr_t case_count) - : zone_(zone), - position_(position), - is_exhaustive_(is_exhaustive), - switch_block_(switch_block), - case_count_(case_count), - case_bodies_(case_count), - case_expression_counts_(case_count), - expressions_(case_count), - sorted_expressions_(case_count) { - case_expression_counts_.FillWith(0, 0, case_count); - } + intptr_t case_count); - // A switch statement is optimizable if all expression are of the same type - // and have integer representations. + // A switch statement is optimizable if static type of the scrutinee + // expression is a non-nullable int or enum, and all case expressions + // are instances of the scrutinee static type. bool is_optimizable() const { return is_optimizable_; } const TokenPosition& position() const { return position_; } bool is_exhaustive() const { return is_exhaustive_; } @@ -1082,11 +1073,8 @@ class SwitchHelper { return sorted_expressions_; } - // The common class of all expressions. The statement must be optimizable. - const Class& expression_class() const { - ASSERT(expression_class_ != nullptr); - return *expression_class_; - } + // Static type of the scrutinee expression. + const AbstractType& expression_type() const { return expression_type_; } const Integer& expression_min() const { ASSERT(expression_min_ != nullptr); @@ -1099,7 +1087,7 @@ class SwitchHelper { bool has_default() const { return default_case_ >= 0; } - bool is_enum_switch() const { return expression_class().is_enum_class(); } + bool is_enum_switch() const { return is_enum_switch_; } // Returns size of [min..max] range, or kMaxInt64 on overflow. int64_t ExpressionRange() const; @@ -1120,8 +1108,10 @@ class SwitchHelper { Zone* zone_; bool is_optimizable_ = false; + bool is_enum_switch_ = false; const TokenPosition position_; const bool is_exhaustive_; + const AbstractType& expression_type_; SwitchBlock* const switch_block_; const intptr_t case_count_; intptr_t default_case_ = -1; @@ -1129,7 +1119,6 @@ class SwitchHelper { GrowableArray case_expression_counts_; GrowableArray expressions_; GrowableArray sorted_expressions_; - const Class* expression_class_ = nullptr; const Integer* expression_min_ = nullptr; const Integer* expression_max_ = nullptr; };