From b138541e745b1cac62fceda412ad49e102bb46b1 Mon Sep 17 00:00:00 2001 From: Mikhail Kornaukhov Date: Wed, 11 Jan 2023 07:16:32 +0300 Subject: [PATCH 1/7] Initial implementation of match --- compiler/debug.cpp | 1 + compiler/gentree.cpp | 72 +++++++++++++++++++++++++ compiler/gentree.h | 3 ++ compiler/keywords.gperf | 1 + compiler/pipes/gen-tree-postprocess.cpp | 69 ++++++++++++++++++++++++ compiler/pipes/gen-tree-postprocess.h | 3 ++ compiler/token.h | 1 + compiler/vertex-desc.json | 52 ++++++++++++++++++ 8 files changed, 202 insertions(+) diff --git a/compiler/debug.cpp b/compiler/debug.cpp index 795ed12fd9..91c469a800 100644 --- a/compiler/debug.cpp +++ b/compiler/debug.cpp @@ -61,6 +61,7 @@ std::string debugTokenName(TokenType t) { {tok_as, "tok_as"}, {tok_case, "tok_case"}, {tok_switch, "tok_switch"}, + {tok_match, "tok_match"}, {tok_class, "tok_class"}, {tok_interface, "tok_interface"}, {tok_trait, "tok_trait"}, diff --git a/compiler/gentree.cpp b/compiler/gentree.cpp index 11b70840e5..96dca0dcdd 100644 --- a/compiler/gentree.cpp +++ b/compiler/gentree.cpp @@ -19,6 +19,7 @@ #include "compiler/name-gen.h" #include "compiler/phpdoc.h" #include "compiler/stage.h" +#include "compiler/token.h" #include "compiler/type-hint.h" #include "compiler/utils/string-utils.h" #include "compiler/vertex.h" @@ -659,6 +660,9 @@ VertexPtr GenTree::get_expr_top(bool was_arrow, const PhpDocComment *phpdoc) { res = get_func_call(); CE (!kphp_error(res.as()->size(), "tuple() must have at least one argument")); break; + case tok_match: + res = get_match(); + break; case tok_shape: res = get_shape(); break; @@ -1221,6 +1225,74 @@ VertexAdaptor GenTree::get_switch() { return VertexUtil::create_switch_vertex(cur_function, switch_condition.set_location(location), std::move(cases)).set_location(location); } + +VertexAdaptor GenTree::get_match() { + const auto location = auto_location(); + next_cur(); + CE(expect(tok_oppar, "'('")); + skip_phpdoc_tokens(); + const auto match_condition = get_expression(); + CE(!kphp_error(match_condition, "Failed to parse 'match' expression")); + CE(expect(tok_clpar, "')'")); + + CE(expect(tok_opbrc, "'{'")); + std::vector cases; + while (cur->type() != tok_clbrc) { + skip_phpdoc_tokens(); + if (cur->type() == tok_comma) { + next_cur(); + continue; + } + + if (cur->type() == tok_default) { + // printf("%s: new default arm!\n", __FUNCTION__); + cases.emplace_back(get_match_default()); + } + else { + // printf("%s: new ordinary arm!\n", __FUNCTION__); + cases.emplace_back(get_match_case()); + } + } + + CE(expect(tok_clbrc, "'}'")); + + auto kek = VertexAdaptor::create(match_condition, std::move(cases)).set_location(location); + kek.debugPrint(); + return kek; +} + +VertexAdaptor GenTree::get_match_case() { + const auto location = auto_location(); + std::vector arms; + + VertexPtr cur_expr; + for (cur_expr = get_expression(); cur_expr->type() != op_double_arrow; cur_expr = get_expression()) { + cur_expr.debugPrint(); + if (cur_expr) { + arms.emplace_back(cur_expr); + } + if (cur->type() == tok_comma) { + next_cur(); + continue; + } + } + + cur_expr.debugPrint(); + + if (const auto double_arrow = cur_expr.try_as()) { + arms.emplace_back(double_arrow->key()); + return VertexAdaptor::create(VertexAdaptor::create(arms), double_arrow->value()).set_location(location); + } + assert(false && "Unreachable!"); + return {}; +} + +VertexAdaptor GenTree::get_match_default() { + const auto location = auto_location(); + next_cur(); + CE(expect(tok_double_arrow, "'=>'")); + return VertexAdaptor::create(get_expression()).set_location(location); +} VertexAdaptor GenTree::get_shape() { auto location = auto_location(); diff --git a/compiler/gentree.h b/compiler/gentree.h index 31e5fe6314..79939f91ea 100644 --- a/compiler/gentree.h +++ b/compiler/gentree.h @@ -100,6 +100,9 @@ class GenTree { VertexAdaptor get_for(); VertexAdaptor get_do(); VertexAdaptor get_switch(); + VertexAdaptor get_match(); + VertexAdaptor get_match_case(); + VertexAdaptor get_match_default(); VertexAdaptor get_shape(); VertexPtr get_by_name_construct(); VertexPtr get_member_by_name_after_var(VertexAdaptor v_before); diff --git a/compiler/keywords.gperf b/compiler/keywords.gperf index e1260987d0..dcc6b98d0f 100644 --- a/compiler/keywords.gperf +++ b/compiler/keywords.gperf @@ -28,6 +28,7 @@ Array, tok_array as, tok_as case, tok_case switch, tok_switch +match, tok_match class, tok_class interface, tok_interface trait, tok_trait diff --git a/compiler/pipes/gen-tree-postprocess.cpp b/compiler/pipes/gen-tree-postprocess.cpp index ddd9b2a94b..9014d71743 100644 --- a/compiler/pipes/gen-tree-postprocess.cpp +++ b/compiler/pipes/gen-tree-postprocess.cpp @@ -8,7 +8,11 @@ #include "compiler/data/class-data.h" #include "compiler/data/lib-data.h" #include "compiler/data/src-file.h" +#include "compiler/data/vertex-adaptor.h" +#include "compiler/name-gen.h" +#include "compiler/vertex-meta_op_base.h" #include "compiler/vertex-util.h" +#include namespace { template @@ -230,6 +234,10 @@ VertexPtr GenTreePostprocessPass::on_enter_vertex(VertexPtr root) { } VertexPtr GenTreePostprocessPass::on_exit_vertex(VertexPtr root) { + if (current_function->name == "foo" && root->type() == op_function) { + root.debugPrint(); + } + if (root->type() == op_var) { if (VertexUtil::is_superglobal(root->get_string())) { root->extra_type = op_ex_var_superglobal; @@ -247,6 +255,10 @@ VertexPtr GenTreePostprocessPass::on_exit_vertex(VertexPtr root) { return convert_array_with_spread_operators(array); } + if (auto match = root.try_as()) { + return convert_match(match); + } + return root; } @@ -311,3 +323,60 @@ VertexPtr GenTreePostprocessPass::convert_array_with_spread_operators(VertexAdap return call; } + +VertexPtr GenTreePostprocessPass::convert_match(VertexAdaptor match_vertex) { + auto gen_superlocal = [&](const std::string& name_prefix) { + auto v = VertexAdaptor::create().set_location(match_vertex); + v->str_val = gen_unique_name(name_prefix); + v->extra_type = op_ex_var_superlocal; + return v; + }; + + const auto tmp_result_var = gen_superlocal("tmp_result_of_match"); + const auto match_condition = match_vertex->condition(); + const auto tmp_condition_stored = gen_superlocal("tmp_condition_stored"); + const auto matched_with_one = gen_superlocal("matched_with_one"); + const auto cases = match_vertex->cases(); + + std::vector switch_arms; + bool has_default = false; + switch_arms.reserve(cases.size()); + + static const auto case_break = VertexAdaptor::create(VertexUtil::create_int_const(1)); + + for (const auto match_case : cases) { + // match_case.debugPrint(); + if (const auto ordinary_case = match_case.try_as()) { + // puts("Have hew ordinary case!\n"); + const auto set_result = VertexAdaptor::create(tmp_result_var, ordinary_case->result_expr()); + const auto case_body = VertexAdaptor::create(std::vector{set_result, case_break.clone()}).set_location(ordinary_case); + for (const auto case_condition : ordinary_case->conditions()->args()) { + // puts("\tHave new condition!\n"); + switch_arms.emplace_back(VertexAdaptor::create(case_condition, case_body)); + } + } + else if (const auto default_case = match_case.try_as()) { + has_default = true; + const auto set_result = VertexAdaptor::create(tmp_result_var, default_case->result_expr()); + const auto case_body = VertexAdaptor::create(std::vector{set_result, case_break.clone()}).set_location(default_case); + switch_arms.emplace_back(VertexAdaptor::create(case_body)); + } + else { + // TODO error + } + } + + if (!has_default) { + auto message = VertexAdaptor::create(); + message->str_val = "UnhandledMatchError!"; + auto emit_error = VertexAdaptor::create(std::vector{message}); + emit_error->set_string("warning"); + const auto case_body = VertexAdaptor::create(std::vector{emit_error, case_break.clone()}); + switch_arms.emplace_back(VertexAdaptor::create(case_body)); + } + + auto switch_vertex = VertexAdaptor::create(match_condition, tmp_condition_stored, matched_with_one, switch_arms); + switch_vertex->is_match = true; + + return VertexAdaptor::create(switch_vertex, tmp_result_var); +} diff --git a/compiler/pipes/gen-tree-postprocess.h b/compiler/pipes/gen-tree-postprocess.h index 0828172abb..87cf549f5b 100644 --- a/compiler/pipes/gen-tree-postprocess.h +++ b/compiler/pipes/gen-tree-postprocess.h @@ -23,4 +23,7 @@ class GenTreePostprocessPass final : public FunctionPassBase { // converts the spread operator (...$a) to a call to the array_merge_spread function static VertexPtr convert_array_with_spread_operators(VertexAdaptor array_vertex); + // converts match to a statement expression that contains a switch operator and temporary variable + static VertexPtr convert_match(VertexAdaptor match_vertex); + }; diff --git a/compiler/token.h b/compiler/token.h index 8303527035..7251bec1fe 100644 --- a/compiler/token.h +++ b/compiler/token.h @@ -44,6 +44,7 @@ enum TokenType { tok_as, tok_case, tok_switch, + tok_match, tok_class, tok_interface, tok_trait, diff --git a/compiler/vertex-desc.json b/compiler/vertex-desc.json index 493011e3c0..c8cbb8818c 100644 --- a/compiler/vertex-desc.json +++ b/compiler/vertex-desc.json @@ -748,6 +748,10 @@ "extra_fields": { "kind": { "type": "SwitchKind" + }, + "is_match": { + "type" : "bool", + "default" : "false" } }, "ranges": { @@ -757,6 +761,54 @@ ] } }, + { + "comment": "match (condition()) { cases()... }", + "sons": { + "condition": 0 + }, + "name": "op_match_proxy", + "base_name": "meta_op_cycle", + "props": { + "str": "match", + "rl": "rl_other" + }, + "ranges": { + "cases": [ + 1, + 0 + ] + } + }, + { + "comment": "$expr_1, ..., $expr_n => $result_expr", + "sons": { + "conditions" : { + "id": 0, + "type": "op_seq_comma" + }, + "result_expr" : { + "id": 1 + } + }, + "name": "op_match_case", + "base_name": "meta_op_base", + "props": { + "str": "case" + } + }, + { + "comment": "default => $result_expr", + "sons": { + "result_expr" : { + "id": 0 + } + }, + "name": "op_match_default", + "base_name": "meta_op_base", + "props": { + "str": "default" + } + }, { "comment": "if (cond()) true_cmd(); or if (cond()) true_cmd() else false_cmd()", "sons": { From b9982dd12cdcac6de49bb7444f9966ecdbf57ec6 Mon Sep 17 00:00:00 2001 From: Mikhail Kornaukhov Date: Thu, 12 Jan 2023 06:44:32 +0300 Subject: [PATCH 2/7] Add tests from previous attempt; fix corner cases --- compiler/code-gen/vertex-compiler.cpp | 2 +- compiler/gentree.cpp | 33 +++++++++---- compiler/pipes/collect-main-edges.cpp | 4 ++ tests/phpt/php8/match/001_simple.php | 22 +++++++++ tests/phpt/php8/match/002_several_cases.php | 13 +++++ ...003_several_cases_with_different_types.php | 10 ++++ tests/phpt/php8/match/004_several_in_case.php | 9 ++++ .../php8/match/005_expression_as_case.php | 13 +++++ .../match/006_several_expression_as_case.php | 19 ++++++++ ...007_string_condition_in_only_int_cases.php | 9 ++++ ...008_int_condition_in_only_string_cases.php | 9 ++++ tests/phpt/php8/match/009_print_bool.php | 12 +++++ tests/phpt/php8/match/010_default_case.php | 15 ++++++ .../php8/match/011_bool_expression_cases.php | 20 ++++++++ .../phpt/php8/match/012_discarding_result.php | 11 +++++ tests/phpt/php8/match/013_implicit_break.php | 12 +++++ .../php8/match/014_strict_comparisons.php | 29 +++++++++++ .../015_strict_comparisons_jump_table.php | 48 +++++++++++++++++++ ...016_strict_comparisons_true_expression.php | 14 ++++++ ...17_strict_comparisons_false_expression.php | 14 ++++++ .../match/018_mixed_int_string_jump_table.php | 29 +++++++++++ ...9_match_expression_with_trailing_comma.php | 22 +++++++++ tests/phpt/php8/match/020_bad_case.php | 7 +++ .../php8/match/021_several_default_arm.php | 8 ++++ .../php8/match/022_repeated_conditions.php | 9 ++++ .../phpt/php8/match/023_unhandeled_value.php | 8 ++++ .../php8/match/024_unhandeled_array_value.php | 14 ++++++ tests/phpt/php8/match/025_arrays.php | 13 +++++ 28 files changed, 417 insertions(+), 11 deletions(-) create mode 100644 tests/phpt/php8/match/001_simple.php create mode 100644 tests/phpt/php8/match/002_several_cases.php create mode 100644 tests/phpt/php8/match/003_several_cases_with_different_types.php create mode 100644 tests/phpt/php8/match/004_several_in_case.php create mode 100644 tests/phpt/php8/match/005_expression_as_case.php create mode 100644 tests/phpt/php8/match/006_several_expression_as_case.php create mode 100644 tests/phpt/php8/match/007_string_condition_in_only_int_cases.php create mode 100644 tests/phpt/php8/match/008_int_condition_in_only_string_cases.php create mode 100644 tests/phpt/php8/match/009_print_bool.php create mode 100644 tests/phpt/php8/match/010_default_case.php create mode 100644 tests/phpt/php8/match/011_bool_expression_cases.php create mode 100644 tests/phpt/php8/match/012_discarding_result.php create mode 100644 tests/phpt/php8/match/013_implicit_break.php create mode 100644 tests/phpt/php8/match/014_strict_comparisons.php create mode 100644 tests/phpt/php8/match/015_strict_comparisons_jump_table.php create mode 100644 tests/phpt/php8/match/016_strict_comparisons_true_expression.php create mode 100644 tests/phpt/php8/match/017_strict_comparisons_false_expression.php create mode 100644 tests/phpt/php8/match/018_mixed_int_string_jump_table.php create mode 100644 tests/phpt/php8/match/019_match_expression_with_trailing_comma.php create mode 100644 tests/phpt/php8/match/020_bad_case.php create mode 100644 tests/phpt/php8/match/021_several_default_arm.php create mode 100644 tests/phpt/php8/match/022_repeated_conditions.php create mode 100644 tests/phpt/php8/match/023_unhandeled_value.php create mode 100644 tests/phpt/php8/match/024_unhandeled_array_value.php create mode 100644 tests/phpt/php8/match/025_arrays.php diff --git a/compiler/code-gen/vertex-compiler.cpp b/compiler/code-gen/vertex-compiler.cpp index 772ac1193c..e4fd51e9e8 100644 --- a/compiler/code-gen/vertex-compiler.cpp +++ b/compiler/code-gen/vertex-compiler.cpp @@ -1239,7 +1239,7 @@ void compile_switch_var(VertexAdaptor root, CodeGenerator &W) { VertexAdaptor cmd; if (auto cs = one_case.try_as()) { cmd = cs->cmd(); - W << "if (" << temp_var_matched_with_a_case << " || eq2(" << temp_var_condition_on_switch << ", " << cs->expr() << ")) " << BEGIN; + W << "if (" << temp_var_matched_with_a_case << " || " << (root->is_match ? "equals(" : "eq2(") << temp_var_condition_on_switch << ", " << cs->expr() << ")) " << BEGIN; W << temp_var_matched_with_a_case << " = true;" << NL; } else { if (!default_case_is_the_last) { diff --git a/compiler/gentree.cpp b/compiler/gentree.cpp index 96dca0dcdd..c340fbc7dd 100644 --- a/compiler/gentree.cpp +++ b/compiler/gentree.cpp @@ -14,6 +14,7 @@ #include "compiler/data/function-data.h" #include "compiler/data/lib-data.h" #include "compiler/data/src-file.h" +#include "compiler/data/vertex-adaptor.h" #include "compiler/lambda-utils.h" #include "compiler/lexer.h" #include "compiler/name-gen.h" @@ -1252,6 +1253,7 @@ VertexAdaptor GenTree::get_match() { // printf("%s: new ordinary arm!\n", __FUNCTION__); cases.emplace_back(get_match_case()); } + kphp_assert_msg(cases.back(), "Invalid 'match' case!"); } CE(expect(tok_clbrc, "'}'")); @@ -1266,30 +1268,41 @@ VertexAdaptor GenTree::get_match_case() { std::vector arms; VertexPtr cur_expr; - for (cur_expr = get_expression(); cur_expr->type() != op_double_arrow; cur_expr = get_expression()) { - cur_expr.debugPrint(); - if (cur_expr) { - arms.emplace_back(cur_expr); - } + for (cur_expr = get_expression(); cur_expr && cur_expr->type() != op_double_arrow; cur_expr = get_expression()) { + arms.emplace_back(cur_expr); + if (cur->type() == tok_comma) { next_cur(); continue; } } - cur_expr.debugPrint(); + VertexPtr result; - if (const auto double_arrow = cur_expr.try_as()) { + // trailling comma: "fourty-two", => 42' + if (!cur_expr) { + CE(expect(tok_double_arrow, "'=>'")); + result = get_expression(); + } + else if (const auto double_arrow = cur_expr.try_as()) { arms.emplace_back(double_arrow->key()); - return VertexAdaptor::create(VertexAdaptor::create(arms), double_arrow->value()).set_location(location); + result = double_arrow->value(); } - assert(false && "Unreachable!"); - return {}; + else { + kphp_fail_msg("Ivalid syntax of 'match' cases!"); + } + + return VertexAdaptor::create(VertexAdaptor::create(arms), result).set_location(location); } VertexAdaptor GenTree::get_match_default() { const auto location = auto_location(); next_cur(); + + // trailling comma: default, => 42' + if (cur->type() == tok_comma) { + next_cur(); + } CE(expect(tok_double_arrow, "'=>'")); return VertexAdaptor::create(get_expression()).set_location(location); } diff --git a/compiler/pipes/collect-main-edges.cpp b/compiler/pipes/collect-main-edges.cpp index e4d4597f14..5b0abdb485 100644 --- a/compiler/pipes/collect-main-edges.cpp +++ b/compiler/pipes/collect-main-edges.cpp @@ -9,6 +9,7 @@ #include "compiler/data/src-file.h" #include "compiler/data/var-data.h" #include "compiler/function-pass.h" +#include "compiler/vertex-meta_op_base.h" #include "compiler/vertex-util.h" #include "compiler/inferring/edge.h" #include "compiler/inferring/ifi.h" @@ -49,6 +50,9 @@ SwitchKind get_switch_kind(VertexAdaptor s) { return SwitchKind::EmptySwitch; } + if (s->is_match) { + return SwitchKind::VarSwitch; + } if (num_const_string_cases == num_value_cases) { return SwitchKind::StringSwitch; } else if (num_const_int_cases == num_value_cases) { diff --git a/tests/phpt/php8/match/001_simple.php b/tests/phpt/php8/match/001_simple.php new file mode 100644 index 0000000000..2b0018fe0f --- /dev/null +++ b/tests/phpt/php8/match/001_simple.php @@ -0,0 +1,22 @@ +@ok php8 + 'Zero', + 1 => 'One', + 2 => 'Two', + 3 => 'Three', + 4 => 'Four', + 5 => 'Five', + 6 => 'Six', + 7 => 'Seven', + 8 => 'Eight', + 9 => 'Nine', + default => 'default', + }; +} + +for ($i = 0; $i <= 9; $i++) { + echo wordify($i) . "\n"; +} diff --git a/tests/phpt/php8/match/002_several_cases.php b/tests/phpt/php8/match/002_several_cases.php new file mode 100644 index 0000000000..141c891f05 --- /dev/null +++ b/tests/phpt/php8/match/002_several_cases.php @@ -0,0 +1,13 @@ +@ok php8 + false, + 2, 3, 4, 5, 6 => true, + }; +} + +for ($i = 1; $i <= 7; $i++) { + var_dump(is_working_day($i)); +} diff --git a/tests/phpt/php8/match/003_several_cases_with_different_types.php b/tests/phpt/php8/match/003_several_cases_with_different_types.php new file mode 100644 index 0000000000..d3f278ea4a --- /dev/null +++ b/tests/phpt/php8/match/003_several_cases_with_different_types.php @@ -0,0 +1,10 @@ +@ok php8 + "oops, error", + "hello" => "ok", + default => "oops, default", +}); diff --git a/tests/phpt/php8/match/004_several_in_case.php b/tests/phpt/php8/match/004_several_in_case.php new file mode 100644 index 0000000000..cf43bb891a --- /dev/null +++ b/tests/phpt/php8/match/004_several_in_case.php @@ -0,0 +1,9 @@ +@ok php8 + "ok", + default => "oops, default", +}); diff --git a/tests/phpt/php8/match/005_expression_as_case.php b/tests/phpt/php8/match/005_expression_as_case.php new file mode 100644 index 0000000000..559cc08403 --- /dev/null +++ b/tests/phpt/php8/match/005_expression_as_case.php @@ -0,0 +1,13 @@ +@ok php8 + "ok", + default => "oops, default", +}); diff --git a/tests/phpt/php8/match/006_several_expression_as_case.php b/tests/phpt/php8/match/006_several_expression_as_case.php new file mode 100644 index 0000000000..b2a384a66f --- /dev/null +++ b/tests/phpt/php8/match/006_several_expression_as_case.php @@ -0,0 +1,19 @@ +@ok php8 + "ok", + f2() => "oops", + default => "oops, default", +}); diff --git a/tests/phpt/php8/match/007_string_condition_in_only_int_cases.php b/tests/phpt/php8/match/007_string_condition_in_only_int_cases.php new file mode 100644 index 0000000000..db56422e75 --- /dev/null +++ b/tests/phpt/php8/match/007_string_condition_in_only_int_cases.php @@ -0,0 +1,9 @@ +@ok php8 + "oops, type casting", + default => "ok", +}); diff --git a/tests/phpt/php8/match/008_int_condition_in_only_string_cases.php b/tests/phpt/php8/match/008_int_condition_in_only_string_cases.php new file mode 100644 index 0000000000..e67ba96c5f --- /dev/null +++ b/tests/phpt/php8/match/008_int_condition_in_only_string_cases.php @@ -0,0 +1,9 @@ +@ok php8 + "oops, type casting", + default => "ok", +}); diff --git a/tests/phpt/php8/match/009_print_bool.php b/tests/phpt/php8/match/009_print_bool.php new file mode 100644 index 0000000000..1289c69477 --- /dev/null +++ b/tests/phpt/php8/match/009_print_bool.php @@ -0,0 +1,12 @@ +@ok php8 + "true\n", + false => "false\n" + }; +} + +print_bool(true); +print_bool(false); diff --git a/tests/phpt/php8/match/010_default_case.php b/tests/phpt/php8/match/010_default_case.php new file mode 100644 index 0000000000..62a2c22c4e --- /dev/null +++ b/tests/phpt/php8/match/010_default_case.php @@ -0,0 +1,15 @@ +@ok php8 + 1, + 2 => 2, + default => 'default', + }; +} + +echo get_value(0) . "\n"; +echo get_value(1) . "\n"; +echo get_value(2) . "\n"; +echo get_value(3) . "\n"; diff --git a/tests/phpt/php8/match/011_bool_expression_cases.php b/tests/phpt/php8/match/011_bool_expression_cases.php new file mode 100644 index 0000000000..5ca7d97490 --- /dev/null +++ b/tests/phpt/php8/match/011_bool_expression_cases.php @@ -0,0 +1,20 @@ +@ok php8 += 50 => '50+', + $i >= 40 => '40-50', + $i >= 30 => '30-40', + $i >= 20 => '20-30', + $i >= 10 => '10-20', + default => '0-10', + }; +} + +echo get_range(22) . "\n"; +echo get_range(0) . "\n"; +echo get_range(59) . "\n"; +echo get_range(13) . "\n"; +echo get_range(39) . "\n"; +echo get_range(40) . "\n"; diff --git a/tests/phpt/php8/match/012_discarding_result.php b/tests/phpt/php8/match/012_discarding_result.php new file mode 100644 index 0000000000..3f3a289a3f --- /dev/null +++ b/tests/phpt/php8/match/012_discarding_result.php @@ -0,0 +1,11 @@ +@ok php8 + print_this(), +}; diff --git a/tests/phpt/php8/match/013_implicit_break.php b/tests/phpt/php8/match/013_implicit_break.php new file mode 100644 index 0000000000..36f135cc92 --- /dev/null +++ b/tests/phpt/php8/match/013_implicit_break.php @@ -0,0 +1,12 @@ +@ok php8 + dump_and_return('foo'), + 'bar' => dump_and_return('bar'), +}); diff --git a/tests/phpt/php8/match/014_strict_comparisons.php b/tests/phpt/php8/match/014_strict_comparisons.php new file mode 100644 index 0000000000..cc95ee6f36 --- /dev/null +++ b/tests/phpt/php8/match/014_strict_comparisons.php @@ -0,0 +1,29 @@ +@ok php8 + wrong(), + false => wrong(), + 0.0 => wrong(), + [] => wrong(), + '' => wrong(), + 0 => 'int', +}); + +function get_value2() { + return 0; +} + +var_dump(match (get_value2()) { + null => wrong(), + false => wrong(), + 0.0 => wrong(), + [] => wrong(), + '' => wrong(), + 0 => 'int', + default => 'default', +}); diff --git a/tests/phpt/php8/match/015_strict_comparisons_jump_table.php b/tests/phpt/php8/match/015_strict_comparisons_jump_table.php new file mode 100644 index 0000000000..3206638307 --- /dev/null +++ b/tests/phpt/php8/match/015_strict_comparisons_jump_table.php @@ -0,0 +1,48 @@ +@ok php8 + wrong(), + 1 => wrong(), + 2 => wrong(), + 3 => wrong(), + 4 => wrong(), + 5 => wrong(), + 6 => wrong(), + 7 => wrong(), + 8 => wrong(), + 9 => wrong(), + default => 'Not matched', + }; +} + +foreach (range(0, 9) as $int) { + var_dump((string)$int); + var_dump(test_int((string)$int)); +} + +function test_string($int) { + return match ($int) { + '0' => wrong(), + '1' => wrong(), + '2' => wrong(), + '3' => wrong(), + '4' => wrong(), + '5' => wrong(), + '6' => wrong(), + '7' => wrong(), + '8' => wrong(), + '9' => wrong(), + default => 'Not matched', + }; +} + +foreach (range(0, 9) as $int) { + var_dump($int); + var_dump(test_string($int)); +} diff --git a/tests/phpt/php8/match/016_strict_comparisons_true_expression.php b/tests/phpt/php8/match/016_strict_comparisons_true_expression.php new file mode 100644 index 0000000000..9f0fcf4fd9 --- /dev/null +++ b/tests/phpt/php8/match/016_strict_comparisons_true_expression.php @@ -0,0 +1,14 @@ +@ok php8 + wrong(), + ['truthy'] => wrong(), + 1 => wrong(), + 1.0 => wrong(), + true => "true\n", +}; diff --git a/tests/phpt/php8/match/017_strict_comparisons_false_expression.php b/tests/phpt/php8/match/017_strict_comparisons_false_expression.php new file mode 100644 index 0000000000..1e256d6364 --- /dev/null +++ b/tests/phpt/php8/match/017_strict_comparisons_false_expression.php @@ -0,0 +1,14 @@ +@ok php8 + wrong(), + [] => wrong(), + 0 => wrong(), + 0.0 => wrong(), + false => "false\n", +}; diff --git a/tests/phpt/php8/match/018_mixed_int_string_jump_table.php b/tests/phpt/php8/match/018_mixed_int_string_jump_table.php new file mode 100644 index 0000000000..ea8fcd4521 --- /dev/null +++ b/tests/phpt/php8/match/018_mixed_int_string_jump_table.php @@ -0,0 +1,29 @@ +@ok php8 + '1 int', + '1' => '1 string', + 2 => '2 int', + '2' => '2 string', + 3 => '3 int', + '3' => '3 string', + 4 => '4 int', + '4' => '4 string', + 5 => '5 int', + '5' => '5 string', + }; + echo "\n"; +} + +test(1); +test('1'); +test(2); +test('2'); +test(3); +test('3'); +test(4); +test('4'); +test(5); +test('5'); diff --git a/tests/phpt/php8/match/019_match_expression_with_trailing_comma.php b/tests/phpt/php8/match/019_match_expression_with_trailing_comma.php new file mode 100644 index 0000000000..d6dbc50e36 --- /dev/null +++ b/tests/phpt/php8/match/019_match_expression_with_trailing_comma.php @@ -0,0 +1,22 @@ +@ok php8 + "false\n", + true, + 1, + => "true\n", + default, + => "not bool\n", + }; +} + +print_bool(false); +print_bool(0); +print_bool(true); +print_bool(1); +print_bool(2); +print_bool('foo'); diff --git a/tests/phpt/php8/match/020_bad_case.php b/tests/phpt/php8/match/020_bad_case.php new file mode 100644 index 0000000000..4e9e58ffa8 --- /dev/null +++ b/tests/phpt/php8/match/020_bad_case.php @@ -0,0 +1,7 @@ +@kphp_should_fail php8 +/Expected expression before token '=>'/ + "ok", +}; diff --git a/tests/phpt/php8/match/021_several_default_arm.php b/tests/phpt/php8/match/021_several_default_arm.php new file mode 100644 index 0000000000..a6f2898713 --- /dev/null +++ b/tests/phpt/php8/match/021_several_default_arm.php @@ -0,0 +1,8 @@ +@kphp_should_fail php8 +/Match expressions may only contain one default arm/ + "default", + default => "default", +}; diff --git a/tests/phpt/php8/match/022_repeated_conditions.php b/tests/phpt/php8/match/022_repeated_conditions.php new file mode 100644 index 0000000000..7ddcf4c847 --- /dev/null +++ b/tests/phpt/php8/match/022_repeated_conditions.php @@ -0,0 +1,9 @@ +@kphp_should_fail php8 +/Repeated case \[10\] in switch\/match/ + "1", + 10 => "2", + default => "default", +}; diff --git a/tests/phpt/php8/match/023_unhandeled_value.php b/tests/phpt/php8/match/023_unhandeled_value.php new file mode 100644 index 0000000000..a16a939dec --- /dev/null +++ b/tests/phpt/php8/match/023_unhandeled_value.php @@ -0,0 +1,8 @@ +@kphp_runtime_should_warn php8 +/Unhandled match value '100'/ + "1", + 30 => "2", +}; diff --git a/tests/phpt/php8/match/024_unhandeled_array_value.php b/tests/phpt/php8/match/024_unhandeled_array_value.php new file mode 100644 index 0000000000..14c7509276 --- /dev/null +++ b/tests/phpt/php8/match/024_unhandeled_array_value.php @@ -0,0 +1,14 @@ +@kphp_runtime_should_warn php8 +/Unhandled match value 'Array/ + 1, + ['yearly', 'credit-card'] => 2, + ['monthly', 'credit-card'] => 3, +}; diff --git a/tests/phpt/php8/match/025_arrays.php b/tests/phpt/php8/match/025_arrays.php new file mode 100644 index 0000000000..a6c5290d47 --- /dev/null +++ b/tests/phpt/php8/match/025_arrays.php @@ -0,0 +1,13 @@ +@ok php8 + 1, + ['yearly', 'credit-card'] => 2, + ['monthly', 'credit-card'] => 3, +}; From 3737e5a1aae34234eb725c96df62ebfb6d852606 Mon Sep 17 00:00:00 2001 From: Mikhail Kornaukhov Date: Mon, 16 Jan 2023 17:34:58 +0300 Subject: [PATCH 3/7] Complete task --- compiler/code-gen/vertex-compiler.cpp | 36 +++++++++++++++--- compiler/gentree.cpp | 2 + compiler/pipes/collect-main-edges.cpp | 20 ++++++++-- compiler/pipes/gen-tree-postprocess.cpp | 4 +- .../015_strict_comparisons_jump_table.php | 38 +++++++++---------- tests/phpt/php8/match/020_bad_case.php | 2 +- .../php8/match/021_several_default_arm.php | 2 +- .../php8/match/022_repeated_conditions.php | 2 +- .../phpt/php8/match/023_unhandeled_value.php | 2 +- .../php8/match/024_unhandeled_array_value.php | 1 - 10 files changed, 74 insertions(+), 35 deletions(-) diff --git a/compiler/code-gen/vertex-compiler.cpp b/compiler/code-gen/vertex-compiler.cpp index e4fd51e9e8..2a94095ccf 100644 --- a/compiler/code-gen/vertex-compiler.cpp +++ b/compiler/code-gen/vertex-compiler.cpp @@ -4,6 +4,7 @@ #include "compiler/code-gen/vertex-compiler.h" +#include "compiler/inferring/primitive-type.h" #include #include @@ -1147,8 +1148,12 @@ void compile_switch_str(VertexAdaptor root, CodeGenerator &W) { auto temp_var_strval_of_condition = root->condition_on_switch(); auto temp_var_matched_with_a_case = root->matched_with_one_case(); + // because we checked types before in case of match + const auto *const convert_function = root->is_match ? "" : "f$strval"; + + W << BEGIN; - W << temp_var_strval_of_condition << " = f$strval (" << root->condition() << ");" << NL; + W << temp_var_strval_of_condition << " = " << convert_function << "(" << root->condition() << ");" << NL; W << temp_var_matched_with_a_case << " = false;" << NL; W << "switch (" << temp_var_strval_of_condition << ".hash()) " << BEGIN; @@ -1191,7 +1196,9 @@ void compile_switch_str(VertexAdaptor root, CodeGenerator &W) { } void compile_switch_int(VertexAdaptor root, CodeGenerator &W) { - W << "switch (f$intval (" << root->condition() << "))" << BEGIN; + // because we checked types before in case of match + const auto *const convert_function = root->is_match ? "" : "f$intval"; + W << "switch (" << convert_function << " (" << root->condition() << "))" << BEGIN; W << "static_cast(" << root->condition_on_switch() << ");" << NL; W << "static_cast(" << root->matched_with_one_case() << ");" << NL; @@ -1206,7 +1213,8 @@ void compile_switch_int(VertexAdaptor root, CodeGenerator &W) { if (val->type() == op_int_const) { const std::string &str = val.as()->str_val; W << str; - kphp_error(used.insert(str).second, fmt_format("Switch: repeated cases found [{}]", str)); + const std::string op = root->is_match ? "Match" : "Switch"; + kphp_error(used.insert(str).second, fmt_format("{}: repeated cases found [{}]", op, str)); } else { kphp_assert(VertexUtil::is_const_int(val)); W << val; @@ -1228,6 +1236,7 @@ void compile_switch_var(VertexAdaptor root, CodeGenerator &W) { auto temp_var_condition_on_switch = root->condition_on_switch(); auto temp_var_matched_with_a_case = root->matched_with_one_case(); + const auto *const eq_function = root->is_match ? "equals" : "eq2"; W << "do " << BEGIN; W << temp_var_condition_on_switch << " = " << root->condition() << ";" << NL; @@ -1239,7 +1248,7 @@ void compile_switch_var(VertexAdaptor root, CodeGenerator &W) { VertexAdaptor cmd; if (auto cs = one_case.try_as()) { cmd = cs->cmd(); - W << "if (" << temp_var_matched_with_a_case << " || " << (root->is_match ? "equals(" : "eq2(") << temp_var_condition_on_switch << ", " << cs->expr() << ")) " << BEGIN; + W << "if (" << temp_var_matched_with_a_case << " || " << eq_function << "(" << temp_var_condition_on_switch << ", " << cs->expr() << ")) " << BEGIN; W << temp_var_matched_with_a_case << " = true;" << NL; } else { if (!default_case_is_the_last) { @@ -1278,12 +1287,29 @@ void compile_switch(VertexAdaptor root, CodeGenerator &W) { for (auto one_case : root->cases()) { if (one_case->type() == op_default) { - kphp_error_return(!has_default, "Switch: several `default` cases found"); + const std::string op = root->is_match ? "Match" : "Switch"; + kphp_error_return(!has_default, op + ": several `default` cases found"); has_default = true; continue; } } + if (root->is_match) { + const auto *const cond_type = tinf::get_type(root->condition()); + if (!cond_type) { + compile_switch_var(root, W); + return; + } + + if (root->kind == SwitchKind::StringSwitch && cond_type->get_real_ptype() == tp_string) { + compile_switch_str(root, W); + } else if (root->kind == SwitchKind::IntSwitch && cond_type->get_real_ptype() == tp_int) { + compile_switch_int(root, W); + } else { + compile_switch_var(root, W); + } + return; + } if (root->kind == SwitchKind::StringSwitch) { compile_switch_str(root, W); } else if (root->kind == SwitchKind::IntSwitch) { diff --git a/compiler/gentree.cpp b/compiler/gentree.cpp index c340fbc7dd..69e05fa72b 100644 --- a/compiler/gentree.cpp +++ b/compiler/gentree.cpp @@ -15,6 +15,7 @@ #include "compiler/data/lib-data.h" #include "compiler/data/src-file.h" #include "compiler/data/vertex-adaptor.h" +#include "compiler/kphp_assert.h" #include "compiler/lambda-utils.h" #include "compiler/lexer.h" #include "compiler/name-gen.h" @@ -1282,6 +1283,7 @@ VertexAdaptor GenTree::get_match_case() { // trailling comma: "fourty-two", => 42' if (!cur_expr) { CE(expect(tok_double_arrow, "'=>'")); + kphp_error(!arms.empty(), "Expected expression before '=>'"); result = get_expression(); } else if (const auto double_arrow = cur_expr.try_as()) { diff --git a/compiler/pipes/collect-main-edges.cpp b/compiler/pipes/collect-main-edges.cpp index 5b0abdb485..a8ee0e485b 100644 --- a/compiler/pipes/collect-main-edges.cpp +++ b/compiler/pipes/collect-main-edges.cpp @@ -27,6 +27,7 @@ SwitchKind get_switch_kind(VertexAdaptor s) { int num_const_int_cases = 0; int num_const_string_cases = 0; int num_value_cases = 0; + int num_const_total_string_cases = 0; for (auto one_case : s->cases()) { if (one_case->type() == op_default) { @@ -38,6 +39,8 @@ SwitchKind get_switch_kind(VertexAdaptor s) { if (VertexUtil::is_const_int(val)) { num_const_int_cases++; } else if (auto as_string = val.try_as()) { + // In case of op_switch node that was generated from op_match + num_const_total_string_cases++; // PHP would use a numerical comparison for strings that look like a number, // we shouldn't rewrite these switches as a string-only switch if (!php_is_numeric(as_string->str_val.data())) { @@ -49,13 +52,13 @@ SwitchKind get_switch_kind(VertexAdaptor s) { if (num_value_cases == 0) { return SwitchKind::EmptySwitch; } - - if (s->is_match) { - return SwitchKind::VarSwitch; + if (s->is_match && num_const_total_string_cases == num_value_cases) { + return SwitchKind::StringSwitch; } if (num_const_string_cases == num_value_cases) { return SwitchKind::StringSwitch; - } else if (num_const_int_cases == num_value_cases) { + } + if (num_const_int_cases == num_value_cases) { return SwitchKind::IntSwitch; } return SwitchKind::VarSwitch; @@ -408,6 +411,15 @@ void CollectMainEdgesPass::on_switch(VertexAdaptor switch_op) { // int-only and string-only switches separately (these simple switch statements // form a majority of all switch statements) switch_op->kind = get_switch_kind(switch_op); + if (switch_op->is_match) { + // in case of converted from op_match + create_set(as_lvalue(switch_op->condition_on_switch()->var_id), switch_op->condition()); + for (const auto &c : switch_op->cases()) { + if (auto as_case = c.try_as()) { + create_set(as_lvalue(switch_op->condition_on_switch()->var_id), as_case->expr()); + } + } + } if (switch_op->kind == SwitchKind::IntSwitch) { // in case of int-only switch, condition var is discarded, so there is // no real need in trying to insert an assignment node here diff --git a/compiler/pipes/gen-tree-postprocess.cpp b/compiler/pipes/gen-tree-postprocess.cpp index 9014d71743..fbba525914 100644 --- a/compiler/pipes/gen-tree-postprocess.cpp +++ b/compiler/pipes/gen-tree-postprocess.cpp @@ -368,9 +368,9 @@ VertexPtr GenTreePostprocessPass::convert_match(VertexAdaptor ma if (!has_default) { auto message = VertexAdaptor::create(); - message->str_val = "UnhandledMatchError!"; + message->str_val = "unhandled value in match!"; auto emit_error = VertexAdaptor::create(std::vector{message}); - emit_error->set_string("warning"); + emit_error->set_string("critical_error"); const auto case_body = VertexAdaptor::create(std::vector{emit_error, case_break.clone()}); switch_arms.emplace_back(VertexAdaptor::create(case_body)); } diff --git a/tests/phpt/php8/match/015_strict_comparisons_jump_table.php b/tests/phpt/php8/match/015_strict_comparisons_jump_table.php index 3206638307..bd6328ad7a 100644 --- a/tests/phpt/php8/match/015_strict_comparisons_jump_table.php +++ b/tests/phpt/php8/match/015_strict_comparisons_jump_table.php @@ -5,26 +5,26 @@ function wrong() { return "wrong"; } -function test_int($char) { - return match ($char) { - 0 => wrong(), - 1 => wrong(), - 2 => wrong(), - 3 => wrong(), - 4 => wrong(), - 5 => wrong(), - 6 => wrong(), - 7 => wrong(), - 8 => wrong(), - 9 => wrong(), - default => 'Not matched', - }; -} +// function test_int($char) { +// return match ($char) { +// 0 => wrong(), +// 1 => wrong(), +// 2 => wrong(), +// 3 => wrong(), +// 4 => wrong(), +// 5 => wrong(), +// 6 => wrong(), +// 7 => wrong(), +// 8 => wrong(), +// 9 => wrong(), +// default => 'Not matched', +// }; +// } -foreach (range(0, 9) as $int) { - var_dump((string)$int); - var_dump(test_int((string)$int)); -} +// foreach (range(0, 9) as $int) { +// var_dump((string)$int); +// var_dump(test_int((string)$int)); +// } function test_string($int) { return match ($int) { diff --git a/tests/phpt/php8/match/020_bad_case.php b/tests/phpt/php8/match/020_bad_case.php index 4e9e58ffa8..f41eeb4af8 100644 --- a/tests/phpt/php8/match/020_bad_case.php +++ b/tests/phpt/php8/match/020_bad_case.php @@ -1,5 +1,5 @@ @kphp_should_fail php8 -/Expected expression before token '=>'/ +Expected expression before '=>' Date: Mon, 16 Jan 2023 18:33:23 +0300 Subject: [PATCH 4/7] Add new test; remove debug prints; add assertions --- compiler/gentree.cpp | 4 +- compiler/pipes/gen-tree-postprocess.cpp | 13 ++----- .../015_strict_comparisons_jump_table.php | 38 +++++++++---------- .../match/{025_arrays.php => 020_arrays.php} | 0 tests/phpt/php8/match/021_double_match.php | 18 +++++++++ .../{020_bad_case.php => 101_bad_case.php} | 0 ...lt_arm.php => 102_several_default_arm.php} | 0 ...itions.php => 103_repeated_conditions.php} | 0 ...led_value.php => 104_unhandeled_value.php} | 0 ...lue.php => 105_unhandeled_array_value.php} | 0 10 files changed, 41 insertions(+), 32 deletions(-) rename tests/phpt/php8/match/{025_arrays.php => 020_arrays.php} (100%) create mode 100644 tests/phpt/php8/match/021_double_match.php rename tests/phpt/php8/match/{020_bad_case.php => 101_bad_case.php} (100%) rename tests/phpt/php8/match/{021_several_default_arm.php => 102_several_default_arm.php} (100%) rename tests/phpt/php8/match/{022_repeated_conditions.php => 103_repeated_conditions.php} (100%) rename tests/phpt/php8/match/{023_unhandeled_value.php => 104_unhandeled_value.php} (100%) rename tests/phpt/php8/match/{024_unhandeled_array_value.php => 105_unhandeled_array_value.php} (100%) diff --git a/compiler/gentree.cpp b/compiler/gentree.cpp index 69e05fa72b..051ca6cef0 100644 --- a/compiler/gentree.cpp +++ b/compiler/gentree.cpp @@ -1259,9 +1259,7 @@ VertexAdaptor GenTree::get_match() { CE(expect(tok_clbrc, "'}'")); - auto kek = VertexAdaptor::create(match_condition, std::move(cases)).set_location(location); - kek.debugPrint(); - return kek; + return VertexAdaptor::create(match_condition, std::move(cases)).set_location(location); } VertexAdaptor GenTree::get_match_case() { diff --git a/compiler/pipes/gen-tree-postprocess.cpp b/compiler/pipes/gen-tree-postprocess.cpp index fbba525914..3c7bf9c56b 100644 --- a/compiler/pipes/gen-tree-postprocess.cpp +++ b/compiler/pipes/gen-tree-postprocess.cpp @@ -4,6 +4,8 @@ #include "compiler/pipes/gen-tree-postprocess.h" +#include "auto/compiler/vertex/vertex-types.h" +#include "common/algorithms/find.h" #include "compiler/compiler-core.h" #include "compiler/data/class-data.h" #include "compiler/data/lib-data.h" @@ -234,10 +236,6 @@ VertexPtr GenTreePostprocessPass::on_enter_vertex(VertexPtr root) { } VertexPtr GenTreePostprocessPass::on_exit_vertex(VertexPtr root) { - if (current_function->name == "foo" && root->type() == op_function) { - root.debugPrint(); - } - if (root->type() == op_var) { if (VertexUtil::is_superglobal(root->get_string())) { root->extra_type = op_ex_var_superglobal; @@ -345,13 +343,10 @@ VertexPtr GenTreePostprocessPass::convert_match(VertexAdaptor ma static const auto case_break = VertexAdaptor::create(VertexUtil::create_int_const(1)); for (const auto match_case : cases) { - // match_case.debugPrint(); if (const auto ordinary_case = match_case.try_as()) { - // puts("Have hew ordinary case!\n"); const auto set_result = VertexAdaptor::create(tmp_result_var, ordinary_case->result_expr()); const auto case_body = VertexAdaptor::create(std::vector{set_result, case_break.clone()}).set_location(ordinary_case); for (const auto case_condition : ordinary_case->conditions()->args()) { - // puts("\tHave new condition!\n"); switch_arms.emplace_back(VertexAdaptor::create(case_condition, case_body)); } } @@ -361,9 +356,7 @@ VertexPtr GenTreePostprocessPass::convert_match(VertexAdaptor ma const auto case_body = VertexAdaptor::create(std::vector{set_result, case_break.clone()}).set_location(default_case); switch_arms.emplace_back(VertexAdaptor::create(case_body)); } - else { - // TODO error - } + kphp_error(vk::any_of_equal(match_case->type(), op_match_case, op_match_default), "Internel error: invalid case type in match expression"); } if (!has_default) { diff --git a/tests/phpt/php8/match/015_strict_comparisons_jump_table.php b/tests/phpt/php8/match/015_strict_comparisons_jump_table.php index bd6328ad7a..3206638307 100644 --- a/tests/phpt/php8/match/015_strict_comparisons_jump_table.php +++ b/tests/phpt/php8/match/015_strict_comparisons_jump_table.php @@ -5,26 +5,26 @@ function wrong() { return "wrong"; } -// function test_int($char) { -// return match ($char) { -// 0 => wrong(), -// 1 => wrong(), -// 2 => wrong(), -// 3 => wrong(), -// 4 => wrong(), -// 5 => wrong(), -// 6 => wrong(), -// 7 => wrong(), -// 8 => wrong(), -// 9 => wrong(), -// default => 'Not matched', -// }; -// } +function test_int($char) { + return match ($char) { + 0 => wrong(), + 1 => wrong(), + 2 => wrong(), + 3 => wrong(), + 4 => wrong(), + 5 => wrong(), + 6 => wrong(), + 7 => wrong(), + 8 => wrong(), + 9 => wrong(), + default => 'Not matched', + }; +} -// foreach (range(0, 9) as $int) { -// var_dump((string)$int); -// var_dump(test_int((string)$int)); -// } +foreach (range(0, 9) as $int) { + var_dump((string)$int); + var_dump(test_int((string)$int)); +} function test_string($int) { return match ($int) { diff --git a/tests/phpt/php8/match/025_arrays.php b/tests/phpt/php8/match/020_arrays.php similarity index 100% rename from tests/phpt/php8/match/025_arrays.php rename to tests/phpt/php8/match/020_arrays.php diff --git a/tests/phpt/php8/match/021_double_match.php b/tests/phpt/php8/match/021_double_match.php new file mode 100644 index 0000000000..a52df993a2 --- /dev/null +++ b/tests/phpt/php8/match/021_double_match.php @@ -0,0 +1,18 @@ +@ok php8 + "one", + 2 => "two", + default => "many", + }) { + "one " => 1, + "two" => 2, + default => 42, + }; +} + +echo f(1) . "\n"; +echo f(2) . "\n"; +echo f(42) . "\n"; diff --git a/tests/phpt/php8/match/020_bad_case.php b/tests/phpt/php8/match/101_bad_case.php similarity index 100% rename from tests/phpt/php8/match/020_bad_case.php rename to tests/phpt/php8/match/101_bad_case.php diff --git a/tests/phpt/php8/match/021_several_default_arm.php b/tests/phpt/php8/match/102_several_default_arm.php similarity index 100% rename from tests/phpt/php8/match/021_several_default_arm.php rename to tests/phpt/php8/match/102_several_default_arm.php diff --git a/tests/phpt/php8/match/022_repeated_conditions.php b/tests/phpt/php8/match/103_repeated_conditions.php similarity index 100% rename from tests/phpt/php8/match/022_repeated_conditions.php rename to tests/phpt/php8/match/103_repeated_conditions.php diff --git a/tests/phpt/php8/match/023_unhandeled_value.php b/tests/phpt/php8/match/104_unhandeled_value.php similarity index 100% rename from tests/phpt/php8/match/023_unhandeled_value.php rename to tests/phpt/php8/match/104_unhandeled_value.php diff --git a/tests/phpt/php8/match/024_unhandeled_array_value.php b/tests/phpt/php8/match/105_unhandeled_array_value.php similarity index 100% rename from tests/phpt/php8/match/024_unhandeled_array_value.php rename to tests/phpt/php8/match/105_unhandeled_array_value.php From 4df186d1280a1c0a32d66051068676dbfc726ee2 Mon Sep 17 00:00:00 2001 From: Mikhail Kornaukhov Date: Mon, 16 Jan 2023 18:41:28 +0300 Subject: [PATCH 5/7] Small fix --- compiler/gentree.cpp | 2 -- compiler/pipes/collect-main-edges.cpp | 2 +- compiler/pipes/gen-tree-postprocess.cpp | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler/gentree.cpp b/compiler/gentree.cpp index 051ca6cef0..6a2b2764b0 100644 --- a/compiler/gentree.cpp +++ b/compiler/gentree.cpp @@ -1247,11 +1247,9 @@ VertexAdaptor GenTree::get_match() { } if (cur->type() == tok_default) { - // printf("%s: new default arm!\n", __FUNCTION__); cases.emplace_back(get_match_default()); } else { - // printf("%s: new ordinary arm!\n", __FUNCTION__); cases.emplace_back(get_match_case()); } kphp_assert_msg(cases.back(), "Invalid 'match' case!"); diff --git a/compiler/pipes/collect-main-edges.cpp b/compiler/pipes/collect-main-edges.cpp index a8ee0e485b..33e9291fe4 100644 --- a/compiler/pipes/collect-main-edges.cpp +++ b/compiler/pipes/collect-main-edges.cpp @@ -420,7 +420,7 @@ void CollectMainEdgesPass::on_switch(VertexAdaptor switch_op) { } } } - if (switch_op->kind == SwitchKind::IntSwitch) { + else if (switch_op->kind == SwitchKind::IntSwitch) { // in case of int-only switch, condition var is discarded, so there is // no real need in trying to insert an assignment node here create_type_assign(as_lvalue(switch_op->condition_on_switch()), TypeData::get_type(tp_int)); diff --git a/compiler/pipes/gen-tree-postprocess.cpp b/compiler/pipes/gen-tree-postprocess.cpp index 3c7bf9c56b..bd5d0c4610 100644 --- a/compiler/pipes/gen-tree-postprocess.cpp +++ b/compiler/pipes/gen-tree-postprocess.cpp @@ -4,7 +4,6 @@ #include "compiler/pipes/gen-tree-postprocess.h" -#include "auto/compiler/vertex/vertex-types.h" #include "common/algorithms/find.h" #include "compiler/compiler-core.h" #include "compiler/data/class-data.h" From 80e3b14c5d28447c8ab7e68e16621440a97af5cf Mon Sep 17 00:00:00 2001 From: mkornaukhov03 Date: Wed, 31 May 2023 11:40:08 +0300 Subject: [PATCH 6/7] Fix review notes --- compiler/code-gen/vertex-compiler.cpp | 15 ++++++--------- compiler/gentree.cpp | 14 ++++++-------- compiler/gentree.h | 2 +- compiler/pipes/collect-main-edges.cpp | 15 +++++++-------- compiler/pipes/gen-tree-postprocess.cpp | 11 ++++------- compiler/pipes/gen-tree-postprocess.h | 2 +- compiler/vertex-desc.json | 2 +- 7 files changed, 26 insertions(+), 35 deletions(-) diff --git a/compiler/code-gen/vertex-compiler.cpp b/compiler/code-gen/vertex-compiler.cpp index 2a94095ccf..18c84a7652 100644 --- a/compiler/code-gen/vertex-compiler.cpp +++ b/compiler/code-gen/vertex-compiler.cpp @@ -1149,8 +1149,7 @@ void compile_switch_str(VertexAdaptor root, CodeGenerator &W) { auto temp_var_matched_with_a_case = root->matched_with_one_case(); // because we checked types before in case of match - const auto *const convert_function = root->is_match ? "" : "f$strval"; - + const char *convert_function = root->is_match ? "" : "f$strval"; W << BEGIN; W << temp_var_strval_of_condition << " = " << convert_function << "(" << root->condition() << ");" << NL; @@ -1197,7 +1196,7 @@ void compile_switch_str(VertexAdaptor root, CodeGenerator &W) { void compile_switch_int(VertexAdaptor root, CodeGenerator &W) { // because we checked types before in case of match - const auto *const convert_function = root->is_match ? "" : "f$intval"; + const char *convert_function = root->is_match ? "" : "f$intval"; W << "switch (" << convert_function << " (" << root->condition() << "))" << BEGIN; W << "static_cast(" << root->condition_on_switch() << ");" << NL; @@ -1213,8 +1212,7 @@ void compile_switch_int(VertexAdaptor root, CodeGenerator &W) { if (val->type() == op_int_const) { const std::string &str = val.as()->str_val; W << str; - const std::string op = root->is_match ? "Match" : "Switch"; - kphp_error(used.insert(str).second, fmt_format("{}: repeated cases found [{}]", op, str)); + kphp_error(used.insert(str).second, fmt_format("{}: repeated cases found [{}]", root->is_match ? "Match" : "Switch", str)); } else { kphp_assert(VertexUtil::is_const_int(val)); W << val; @@ -1236,7 +1234,7 @@ void compile_switch_var(VertexAdaptor root, CodeGenerator &W) { auto temp_var_condition_on_switch = root->condition_on_switch(); auto temp_var_matched_with_a_case = root->matched_with_one_case(); - const auto *const eq_function = root->is_match ? "equals" : "eq2"; + const char *eq_function = root->is_match ? "equals" : "eq2"; W << "do " << BEGIN; W << temp_var_condition_on_switch << " = " << root->condition() << ";" << NL; @@ -1287,15 +1285,14 @@ void compile_switch(VertexAdaptor root, CodeGenerator &W) { for (auto one_case : root->cases()) { if (one_case->type() == op_default) { - const std::string op = root->is_match ? "Match" : "Switch"; - kphp_error_return(!has_default, op + ": several `default` cases found"); + kphp_error_return(!has_default, fmt_format("%s: several `default` cases found", root->is_match ? "Match" : "Switch")); has_default = true; continue; } } if (root->is_match) { - const auto *const cond_type = tinf::get_type(root->condition()); + const TypeData *cond_type = tinf::get_type(root->condition()); if (!cond_type) { compile_switch_var(root, W); return; diff --git a/compiler/gentree.cpp b/compiler/gentree.cpp index 6a2b2764b0..6dfa6b0445 100644 --- a/compiler/gentree.cpp +++ b/compiler/gentree.cpp @@ -1228,7 +1228,7 @@ VertexAdaptor GenTree::get_switch() { } -VertexAdaptor GenTree::get_match() { +VertexAdaptor GenTree::get_match() { const auto location = auto_location(); next_cur(); CE(expect(tok_oppar, "'('")); @@ -1248,8 +1248,7 @@ VertexAdaptor GenTree::get_match() { if (cur->type() == tok_default) { cases.emplace_back(get_match_default()); - } - else { + } else { cases.emplace_back(get_match_case()); } kphp_assert_msg(cases.back(), "Invalid 'match' case!"); @@ -1257,7 +1256,7 @@ VertexAdaptor GenTree::get_match() { CE(expect(tok_clbrc, "'}'")); - return VertexAdaptor::create(match_condition, std::move(cases)).set_location(location); + return VertexAdaptor::create(match_condition, std::move(cases)).set_location(location); } VertexAdaptor GenTree::get_match_case() { @@ -1281,12 +1280,10 @@ VertexAdaptor GenTree::get_match_case() { CE(expect(tok_double_arrow, "'=>'")); kphp_error(!arms.empty(), "Expected expression before '=>'"); result = get_expression(); - } - else if (const auto double_arrow = cur_expr.try_as()) { + } else if (const auto double_arrow = cur_expr.try_as()) { arms.emplace_back(double_arrow->key()); result = double_arrow->value(); - } - else { + } else { kphp_fail_msg("Ivalid syntax of 'match' cases!"); } @@ -1304,6 +1301,7 @@ VertexAdaptor GenTree::get_match_default() { CE(expect(tok_double_arrow, "'=>'")); return VertexAdaptor::create(get_expression()).set_location(location); } + VertexAdaptor GenTree::get_shape() { auto location = auto_location(); diff --git a/compiler/gentree.h b/compiler/gentree.h index 79939f91ea..5109a2add2 100644 --- a/compiler/gentree.h +++ b/compiler/gentree.h @@ -100,7 +100,7 @@ class GenTree { VertexAdaptor get_for(); VertexAdaptor get_do(); VertexAdaptor get_switch(); - VertexAdaptor get_match(); + VertexAdaptor get_match(); VertexAdaptor get_match_case(); VertexAdaptor get_match_default(); VertexAdaptor get_shape(); diff --git a/compiler/pipes/collect-main-edges.cpp b/compiler/pipes/collect-main-edges.cpp index 33e9291fe4..3d736ee5b8 100644 --- a/compiler/pipes/collect-main-edges.cpp +++ b/compiler/pipes/collect-main-edges.cpp @@ -25,9 +25,9 @@ namespace { SwitchKind get_switch_kind(VertexAdaptor s) { int num_const_int_cases = 0; - int num_const_string_cases = 0; + int num_const_non_numeric_string_cases = 0; int num_value_cases = 0; - int num_const_total_string_cases = 0; + int num_const_string_cases = 0; for (auto one_case : s->cases()) { if (one_case->type() == op_default) { @@ -40,11 +40,11 @@ SwitchKind get_switch_kind(VertexAdaptor s) { num_const_int_cases++; } else if (auto as_string = val.try_as()) { // In case of op_switch node that was generated from op_match - num_const_total_string_cases++; + num_const_string_cases++; // PHP would use a numerical comparison for strings that look like a number, // we shouldn't rewrite these switches as a string-only switch if (!php_is_numeric(as_string->str_val.data())) { - num_const_string_cases++; + num_const_non_numeric_string_cases++; } } } @@ -52,10 +52,10 @@ SwitchKind get_switch_kind(VertexAdaptor s) { if (num_value_cases == 0) { return SwitchKind::EmptySwitch; } - if (s->is_match && num_const_total_string_cases == num_value_cases) { + if (s->is_match && num_const_string_cases == num_value_cases) { return SwitchKind::StringSwitch; } - if (num_const_string_cases == num_value_cases) { + if (num_const_non_numeric_string_cases == num_value_cases) { return SwitchKind::StringSwitch; } if (num_const_int_cases == num_value_cases) { @@ -419,8 +419,7 @@ void CollectMainEdgesPass::on_switch(VertexAdaptor switch_op) { create_set(as_lvalue(switch_op->condition_on_switch()->var_id), as_case->expr()); } } - } - else if (switch_op->kind == SwitchKind::IntSwitch) { + } else if (switch_op->kind == SwitchKind::IntSwitch) { // in case of int-only switch, condition var is discarded, so there is // no real need in trying to insert an assignment node here create_type_assign(as_lvalue(switch_op->condition_on_switch()), TypeData::get_type(tp_int)); diff --git a/compiler/pipes/gen-tree-postprocess.cpp b/compiler/pipes/gen-tree-postprocess.cpp index bd5d0c4610..e2aae3bab4 100644 --- a/compiler/pipes/gen-tree-postprocess.cpp +++ b/compiler/pipes/gen-tree-postprocess.cpp @@ -4,16 +4,13 @@ #include "compiler/pipes/gen-tree-postprocess.h" -#include "common/algorithms/find.h" +#include + #include "compiler/compiler-core.h" -#include "compiler/data/class-data.h" #include "compiler/data/lib-data.h" #include "compiler/data/src-file.h" -#include "compiler/data/vertex-adaptor.h" #include "compiler/name-gen.h" -#include "compiler/vertex-meta_op_base.h" #include "compiler/vertex-util.h" -#include namespace { template @@ -252,7 +249,7 @@ VertexPtr GenTreePostprocessPass::on_exit_vertex(VertexPtr root) { return convert_array_with_spread_operators(array); } - if (auto match = root.try_as()) { + if (auto match = root.try_as()) { return convert_match(match); } @@ -321,7 +318,7 @@ VertexPtr GenTreePostprocessPass::convert_array_with_spread_operators(VertexAdap return call; } -VertexPtr GenTreePostprocessPass::convert_match(VertexAdaptor match_vertex) { +VertexPtr GenTreePostprocessPass::convert_match(VertexAdaptor match_vertex) { auto gen_superlocal = [&](const std::string& name_prefix) { auto v = VertexAdaptor::create().set_location(match_vertex); v->str_val = gen_unique_name(name_prefix); diff --git a/compiler/pipes/gen-tree-postprocess.h b/compiler/pipes/gen-tree-postprocess.h index 87cf549f5b..d22180247b 100644 --- a/compiler/pipes/gen-tree-postprocess.h +++ b/compiler/pipes/gen-tree-postprocess.h @@ -24,6 +24,6 @@ class GenTreePostprocessPass final : public FunctionPassBase { // converts the spread operator (...$a) to a call to the array_merge_spread function static VertexPtr convert_array_with_spread_operators(VertexAdaptor array_vertex); // converts match to a statement expression that contains a switch operator and temporary variable - static VertexPtr convert_match(VertexAdaptor match_vertex); + static VertexPtr convert_match(VertexAdaptor match_vertex); }; diff --git a/compiler/vertex-desc.json b/compiler/vertex-desc.json index c8cbb8818c..3bb6794a9f 100644 --- a/compiler/vertex-desc.json +++ b/compiler/vertex-desc.json @@ -766,7 +766,7 @@ "sons": { "condition": 0 }, - "name": "op_match_proxy", + "name": "op_match", "base_name": "meta_op_cycle", "props": { "str": "match", From 932fb93470a71bfa38a873c31c8541d41127ae1c Mon Sep 17 00:00:00 2001 From: mkornaukhov03 Date: Wed, 31 May 2023 11:57:13 +0300 Subject: [PATCH 7/7] Add description --- compiler/pipes/gen-tree-postprocess.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/compiler/pipes/gen-tree-postprocess.cpp b/compiler/pipes/gen-tree-postprocess.cpp index e2aae3bab4..65a9d80d3b 100644 --- a/compiler/pipes/gen-tree-postprocess.cpp +++ b/compiler/pipes/gen-tree-postprocess.cpp @@ -318,6 +318,17 @@ VertexPtr GenTreePostprocessPass::convert_array_with_spread_operators(VertexAdap return call; } +// convert op_match node to op_seq_rval like +// { +// var res; +// switch (...) { +// ... +// ... => { res = ...} +// ... +// } +// res; +// } +// we need this because `match` is expression VertexPtr GenTreePostprocessPass::convert_match(VertexAdaptor match_vertex) { auto gen_superlocal = [&](const std::string& name_prefix) { auto v = VertexAdaptor::create().set_location(match_vertex); @@ -352,7 +363,7 @@ VertexPtr GenTreePostprocessPass::convert_match(VertexAdaptor match_ve const auto case_body = VertexAdaptor::create(std::vector{set_result, case_break.clone()}).set_location(default_case); switch_arms.emplace_back(VertexAdaptor::create(case_body)); } - kphp_error(vk::any_of_equal(match_case->type(), op_match_case, op_match_default), "Internel error: invalid case type in match expression"); + kphp_error(vk::any_of_equal(match_case->type(), op_match_case, op_match_default), "Internal error: invalid case type in match expression"); } if (!has_default) {