Fix review notes

compiler/code-gen/vertex-compiler.cpp

@@ -1149,8 +1149,7 @@ void compile_switch_str(VertexAdaptor<op_switch> 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<op_switch> root, CodeGenerator &W) {
 
 void compile_switch_int(VertexAdaptor<op_switch> 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<void>(" << root->condition_on_switch() << ");" << NL;
@@ -1213,8 +1212,7 @@ void compile_switch_int(VertexAdaptor<op_switch> root, CodeGenerator &W) {
       if (val->type() == op_int_const) {
         const std::string &str = val.as<op_int_const>()->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<op_switch> 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<op_switch> 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;

compiler/gentree.cpp

@@ -1228,7 +1228,7 @@ VertexAdaptor<op_switch> GenTree::get_switch() {
 }
 
 
-VertexAdaptor<op_match_proxy> GenTree::get_match() {
+VertexAdaptor<op_match> GenTree::get_match() {
   const auto location = auto_location();
   next_cur();
   CE(expect(tok_oppar, "'('"));
@@ -1248,16 +1248,15 @@ VertexAdaptor<op_match_proxy> 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!");
   }
 
   CE(expect(tok_clbrc, "'}'"));
 
-  return VertexAdaptor<op_match_proxy>::create(match_condition, std::move(cases)).set_location(location);
+  return VertexAdaptor<op_match>::create(match_condition, std::move(cases)).set_location(location);
 }
 
 VertexAdaptor<op_match_case> GenTree::get_match_case() {
@@ -1281,12 +1280,10 @@ VertexAdaptor<op_match_case> 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<op_double_arrow>()) {
+  } else if (const auto double_arrow = cur_expr.try_as<op_double_arrow>()) {
     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<op_match_default> GenTree::get_match_default() {
   CE(expect(tok_double_arrow, "'=>'"));
   return VertexAdaptor<op_match_default>::create(get_expression()).set_location(location);
 }
+
 VertexAdaptor<op_shape> GenTree::get_shape() {
   auto location = auto_location();
 

compiler/gentree.h

@@ -100,7 +100,7 @@ class GenTree {
   VertexAdaptor<op_for> get_for();
   VertexAdaptor<op_do> get_do();
   VertexAdaptor<op_switch> get_switch();
-  VertexAdaptor<op_match_proxy> get_match();
+  VertexAdaptor<op_match> get_match();
   VertexAdaptor<op_match_case> get_match_case();
   VertexAdaptor<op_match_default> get_match_default();
   VertexAdaptor<op_shape> get_shape();

compiler/pipes/collect-main-edges.cpp

@@ -25,9 +25,9 @@ namespace {
 
 SwitchKind get_switch_kind(VertexAdaptor<op_switch> 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,22 +40,22 @@ SwitchKind get_switch_kind(VertexAdaptor<op_switch> s) {
       num_const_int_cases++;
     } else if (auto as_string = val.try_as<op_string>()) {
       // 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++;
       }
     }
   }
 
   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<op_switch> 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));

compiler/pipes/gen-tree-postprocess.cpp

@@ -4,16 +4,13 @@
 
 #include "compiler/pipes/gen-tree-postprocess.h"
 
-#include "common/algorithms/find.h"
+#include <vector>
+
 #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 <vector>
 
 namespace {
 template <typename F>
@@ -252,7 +249,7 @@ VertexPtr GenTreePostprocessPass::on_exit_vertex(VertexPtr root) {
     return convert_array_with_spread_operators(array);
   }
 
-  if (auto match = root.try_as<op_match_proxy>()) {
+  if (auto match = root.try_as<op_match>()) {
     return convert_match(match);
   }
 
@@ -321,7 +318,7 @@ VertexPtr GenTreePostprocessPass::convert_array_with_spread_operators(VertexAdap
   return call;
 }
 
-VertexPtr GenTreePostprocessPass::convert_match(VertexAdaptor<op_match_proxy> match_vertex) {
+VertexPtr GenTreePostprocessPass::convert_match(VertexAdaptor<op_match> match_vertex) {
   auto gen_superlocal = [&](const std::string& name_prefix) {
     auto v = VertexAdaptor<op_var>::create().set_location(match_vertex);
     v->str_val = gen_unique_name(name_prefix);

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<op_array> array_vertex);
   // converts match to a statement expression that contains a switch operator and temporary variable
-  static VertexPtr convert_match(VertexAdaptor<op_match_proxy> match_vertex);
+  static VertexPtr convert_match(VertexAdaptor<op_match> match_vertex);
 
 };

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",