From c6fa736491d18c649ba2575ace05bd069281eafe Mon Sep 17 00:00:00 2001 From: i582 <51853996+i582@users.noreply.github.com> Date: Mon, 13 Sep 2021 03:00:26 +0300 Subject: [PATCH 1/5] added support for property promotion --- compiler/gentree.cpp | 47 +++++++++++++++-- compiler/gentree.h | 1 + compiler/pipes/gen-tree-postprocess.cpp | 51 +++++++++++++++++++ compiler/pipes/gen-tree-postprocess.h | 1 + compiler/pipes/preprocess-function.cpp | 2 +- compiler/pipes/register-variables.h | 2 +- compiler/pipes/resolve-self-static-parent.cpp | 2 +- compiler/vertex-desc.json | 4 ++ compiler/vertex-meta_op_base.h | 10 +++- 9 files changed, 113 insertions(+), 7 deletions(-) diff --git a/compiler/gentree.cpp b/compiler/gentree.cpp index aebdcd0ddc..9388dff9fe 100644 --- a/compiler/gentree.cpp +++ b/compiler/gentree.cpp @@ -593,7 +593,7 @@ VertexPtr GenTree::get_expr_top(bool was_arrow) { func_call->set_string("McMemcache"); } - res = gen_constructor_call_with_args(func_call->str_val, func_call->get_next()).set_location(func_call); + res = gen_constructor_call_with_args(func_call->str_val, func_call->as_vector()).set_location(func_call); CE(res); break; } @@ -872,9 +872,27 @@ VertexPtr GenTree::get_def_value() { return val; } +AccessModifiers GenTree::try_get_visibility_modifiers() { + if (test_expect(tok_public)) { + return AccessModifiers::public_; + } else if (test_expect(tok_private)) { + return AccessModifiers::private_; + } else if (test_expect(tok_protected)) { + return AccessModifiers::protected_; + } + return AccessModifiers::not_modifier_; +} + VertexAdaptor GenTree::get_func_param() { auto location = auto_location(); + // if is promoted property + const auto access_modifier = try_get_visibility_modifiers(); + // if there is a modifier + if (access_modifier != AccessModifiers::not_modifier_) { + next_cur(); + } + const TypeHint *type_hint = get_typehint(); bool is_varg = false; @@ -920,6 +938,7 @@ VertexAdaptor GenTree::get_func_param() { v->type_hint = type_hint; } v->is_cast_param = is_cast_param; + v->access_modifier = access_modifier; return v; } @@ -1052,7 +1071,7 @@ void GenTree::func_force_return(VertexAdaptor func, VertexPtr val) return_node = VertexAdaptor::create(); } - vector next = cmd->get_next(); + vector next = cmd->as_vector(); next.push_back(return_node); func->cmd_ref() = VertexAdaptor::create(next); } @@ -1507,7 +1526,14 @@ VertexAdaptor GenTree::parse_cur_function_param_list() { CE(expect(tok_clpar, "')'")); for (size_t i = 1; i < params_next.size(); ++i) { - if (!params_next[i]->has_default_value()) { + const auto ¶m = params_next[i]; + + // if promoted property outside constructor + if (cur_function->local_name() != ClassData::NAME_OF_CONSTRUCT && param->access_modifier != AccessModifiers::not_modifier_) { + kphp_error(0, "Cannot declare promoted property outside a constructor"); + } + + if (!param->has_default_value()) { kphp_error(!params_next[i - 1]->has_default_value(), "Optional parameter is provided before required"); } } @@ -1612,6 +1638,21 @@ VertexAdaptor GenTree::get_function(TokenType tok, vk::string_view cur_class->members.add_static_method(cur_function); } + // add all the promoted properties in constructor to the class + if (cur_function->is_constructor()) { + for (const auto &p : cur_function->get_params()) { + const auto ¶m = p.as(); + + if (param->access_modifier != AccessModifiers::not_modifier_) { + const auto field_modifiers = FieldModifiers{param->access_modifier}; + auto var = VertexAdaptor::create().set_location(auto_location()); + var->str_val = param->var()->str_val; + + cur_class->members.add_instance_field(var, {}, field_modifiers, "", param->type_hint); + } + } + } + // the function is ready, register it; // the constructor is registered later, after the entire class is parsed if (!cur_function->is_constructor()) { diff --git a/compiler/gentree.h b/compiler/gentree.h index 31fe78c097..4ac269a715 100644 --- a/compiler/gentree.h +++ b/compiler/gentree.h @@ -128,6 +128,7 @@ class GenTree { VertexAdaptor get_anonymous_function(TokenType tok = tok_function, bool is_static = false); VertexAdaptor get_function(TokenType tok, vk::string_view phpdoc_str, FunctionModifiers modifiers, std::vector> *uses_of_lambda = nullptr); + AccessModifiers try_get_visibility_modifiers(); ClassMemberModifiers parse_class_member_modifier_mask(); VertexPtr get_class_member(vk::string_view phpdoc_str); diff --git a/compiler/pipes/gen-tree-postprocess.cpp b/compiler/pipes/gen-tree-postprocess.cpp index 0b1cf0c506..dbe89fb8b8 100644 --- a/compiler/pipes/gen-tree-postprocess.cpp +++ b/compiler/pipes/gen-tree-postprocess.cpp @@ -256,9 +256,60 @@ VertexPtr GenTreePostprocessPass::on_exit_vertex(VertexPtr root) { return convert_array_with_spread_operators(array); } + if (auto fun = root.try_as()) { + process_property_promotion(fun); + } + return root; } +void GenTreePostprocessPass::process_property_promotion(VertexAdaptor &fun) const { + if (!fun->func_id->is_constructor()) { + return; + } + + auto promoted_params = std::vector>(); + + // TODO: add flag 'with_property_promotion' instead of iterating over parameters here + const auto param_list = fun->param_list(); + for (const auto &p : param_list->params()) { + const auto ¶m = p.try_as(); + + if (param->access_modifier != AccessModifiers::not_modifier_) { + promoted_params.push_back(param); + } + } + + if (promoted_params.empty()) { + return; + } + + auto func_stmts = fun->cmd()->as_vector(); + + // create an expression like "$this-> = " for each promoted property + // all such expressions are placed at the very beginning of the expression list + // inside the constructor to avoid collisions with existing expressions + for (const auto ¶m : promoted_params) { + const auto ¶m_name = param->var()->str_val; + const auto *field = current_function->class_id->get_instance_field(param_name); + if (!field) { + continue; + } + + const auto prop_var = field->var; + const auto this_vertex = ClassData::gen_vertex_this(fun->location); + + auto prop_fetch = VertexAdaptor::create(this_vertex).set_location(fun->location); + prop_fetch->set_string(param_name); + prop_fetch->var_id = prop_var; + + const auto set_vertex = VertexAdaptor::create(prop_fetch, param->var().clone()); + func_stmts.insert(func_stmts.begin(), set_vertex); + } + + fun->cmd_ref() = VertexAdaptor::create(func_stmts);; +} + VertexAdaptor array_vertex_from_slice(const VertexRange &args, size_t start, size_t end) { return VertexAdaptor::create( std::vector{args.begin() + start, args.begin() + end} diff --git a/compiler/pipes/gen-tree-postprocess.h b/compiler/pipes/gen-tree-postprocess.h index 82f262878c..ad8d2f9073 100644 --- a/compiler/pipes/gen-tree-postprocess.h +++ b/compiler/pipes/gen-tree-postprocess.h @@ -23,4 +23,5 @@ 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); + void process_property_promotion(VertexAdaptor &fun) const; }; diff --git a/compiler/pipes/preprocess-function.cpp b/compiler/pipes/preprocess-function.cpp index ea70c74417..b2d1d177c1 100644 --- a/compiler/pipes/preprocess-function.cpp +++ b/compiler/pipes/preprocess-function.cpp @@ -260,7 +260,7 @@ class PreprocessFunctionPass final : public FunctionPassBase { // just as ordinary positional argument: // $this->fun(1, 2, 3) -> fun($this, 1, [2, 3]) - const std::vector &cur_call_args = call->get_next(); + const std::vector &cur_call_args = call->as_vector(); auto positional_args_start = cur_call_args.begin(); int min_args = func_args_n - 1; // variadic param may accept 0 args, so subtract 1 diff --git a/compiler/pipes/register-variables.h b/compiler/pipes/register-variables.h index f3279e94c9..dbbbace1bc 100644 --- a/compiler/pipes/register-variables.h +++ b/compiler/pipes/register-variables.h @@ -10,7 +10,7 @@ #include "compiler/function-pass.h" /** - * 1. Function parametres (with default values) + * 1. Function parameters (with default values) * 2. Global variables * 3. Static local variables (with default values) * 4. Local variables diff --git a/compiler/pipes/resolve-self-static-parent.cpp b/compiler/pipes/resolve-self-static-parent.cpp index b8d9273a5f..6f33bac7a3 100644 --- a/compiler/pipes/resolve-self-static-parent.cpp +++ b/compiler/pipes/resolve-self-static-parent.cpp @@ -63,7 +63,7 @@ VertexPtr ResolveSelfStaticParentPass::on_enter_vertex(VertexPtr v) { this_vertex = VertexAdaptor::create(this_vertex).set_location(v); this_vertex->set_string(LambdaClassData::get_parent_this_name()); } - v = VertexAdaptor::create(this_vertex, v->get_next()).set_location(v); + v = VertexAdaptor::create(this_vertex, v->as_vector()).set_location(v); v->extra_type = op_ex_func_call_arrow; v->set_string(std::string{method->local_name()}); v.as()->func_id = method; diff --git a/compiler/vertex-desc.json b/compiler/vertex-desc.json index 1cfba930a4..a0b215739f 100644 --- a/compiler/vertex-desc.json +++ b/compiler/vertex-desc.json @@ -447,6 +447,10 @@ "is_callable": { "type": "bool", "default": "false" + }, + "access_modifier": { + "type": "AccessModifiers", + "default": "AccessModifiers::not_modifier_" } } }, diff --git a/compiler/vertex-meta_op_base.h b/compiler/vertex-meta_op_base.h index 72151cdd99..2e17cc8343 100644 --- a/compiler/vertex-meta_op_base.h +++ b/compiler/vertex-meta_op_base.h @@ -7,6 +7,7 @@ #include "common/wrappers/iterator_range.h" #include "compiler/common.h" +#include "compiler/data/class-member-modifiers.h" #include "compiler/data/data_ptr.h" #include "compiler/data/vertex-adaptor.h" #include "compiler/inferring/expr-node.h" @@ -166,7 +167,14 @@ class vertex_inner { VertexPtr &back() { return ith(size() - 1); } - std::vector get_next() { return std::vector(begin(), end()); } + /** + * as_vector returns a vector which is a set of VertexPtr that + * should be interpreted depending on the type of the vertex. + * + * For example, for 'op_seq' this will be equivalent to a list + * of vertices in the sequence. + */ + std::vector as_vector() { return {begin(), end()}; } bool empty() { return size() == 0; } From 781689f5d2cef540f94f416f464a9db456dcd8b5 Mon Sep 17 00:00:00 2001 From: i582 <51853996+i582@users.noreply.github.com> Date: Mon, 13 Sep 2021 11:20:59 +0300 Subject: [PATCH 2/5] added checks and tests --- compiler/gentree.cpp | 18 ++++++++++++----- compiler/vertex-desc.json | 2 +- .../php8/property_promotion/001_basic.php | 0 .../002_mixing_with_normal_props.php | 14 +++++++++++++ .../003_with_default_value.php | 20 +++++++++++++++++++ .../004_redeclarate_prop.php | 9 +++++++++ .../005_abstract_constructor.php | 7 +++++++ .../php8/property_promotion/006_static.php | 7 +++++++ .../007_in_free_function.php | 0 .../php8/property_promotion/008_interface.php | 7 +++++++ .../009_not_in_constructor.php | 7 +++++++ .../php8/property_promotion/010_trait.php | 12 +++++++++++ .../php8/property_promotion/011_variadic.php | 7 +++++++ 13 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 tests/phpt/php8/property_promotion/001_basic.php create mode 100644 tests/phpt/php8/property_promotion/002_mixing_with_normal_props.php create mode 100644 tests/phpt/php8/property_promotion/003_with_default_value.php create mode 100644 tests/phpt/php8/property_promotion/004_redeclarate_prop.php create mode 100644 tests/phpt/php8/property_promotion/005_abstract_constructor.php create mode 100644 tests/phpt/php8/property_promotion/006_static.php create mode 100644 tests/phpt/php8/property_promotion/007_in_free_function.php create mode 100644 tests/phpt/php8/property_promotion/008_interface.php create mode 100644 tests/phpt/php8/property_promotion/009_not_in_constructor.php create mode 100644 tests/phpt/php8/property_promotion/010_trait.php create mode 100644 tests/phpt/php8/property_promotion/011_variadic.php diff --git a/compiler/gentree.cpp b/compiler/gentree.cpp index 9388dff9fe..2e29baa6a5 100644 --- a/compiler/gentree.cpp +++ b/compiler/gentree.cpp @@ -886,9 +886,8 @@ AccessModifiers GenTree::try_get_visibility_modifiers() { VertexAdaptor GenTree::get_func_param() { auto location = auto_location(); - // if is promoted property + // possibly promoted property const auto access_modifier = try_get_visibility_modifiers(); - // if there is a modifier if (access_modifier != AccessModifiers::not_modifier_) { next_cur(); } @@ -904,7 +903,12 @@ VertexAdaptor GenTree::get_func_param() { is_varg = true; } if (is_varg) { - kphp_error(!cur_function->has_variadic_param, "Function can not have ...$variadic more than once"); + if (cur_function->has_variadic_param) { + kphp_error(0, "Function can not have ...$variadic more than once"); + } + if (access_modifier != AccessModifiers::not_modifier_) { + kphp_error(0, "Cannot declare variadic promoted property"); + } cur_function->has_variadic_param = true; } @@ -1525,7 +1529,7 @@ VertexAdaptor GenTree::parse_cur_function_param_list() { CE(!kphp_error(ok_params_next, "Failed to parse function params")); CE(expect(tok_clpar, "')'")); - for (size_t i = 1; i < params_next.size(); ++i) { + for (size_t i = 0; i < params_next.size(); ++i) { const auto ¶m = params_next[i]; // if promoted property outside constructor @@ -1533,7 +1537,11 @@ VertexAdaptor GenTree::parse_cur_function_param_list() { kphp_error(0, "Cannot declare promoted property outside a constructor"); } - if (!param->has_default_value()) { + if (cur_function->local_name() == ClassData::NAME_OF_CONSTRUCT && cur_function->modifiers.is_abstract()) { + kphp_error(0, "Cannot declare promoted property in an abstract constructor"); + } + + if (i > 0 && !param->has_default_value()) { kphp_error(!params_next[i - 1]->has_default_value(), "Optional parameter is provided before required"); } } diff --git a/compiler/vertex-desc.json b/compiler/vertex-desc.json index a0b215739f..bb4b80620a 100644 --- a/compiler/vertex-desc.json +++ b/compiler/vertex-desc.json @@ -417,7 +417,7 @@ } }, { - "comment": "var(); or var() = default_value(); type hints are stored in type_hint", + "comment": "[access_modifier] var() [= default_value()]; type hints are stored in type_hint", "name": "op_func_param", "base_name": "meta_op_base", "sons": { diff --git a/tests/phpt/php8/property_promotion/001_basic.php b/tests/phpt/php8/property_promotion/001_basic.php new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/phpt/php8/property_promotion/002_mixing_with_normal_props.php b/tests/phpt/php8/property_promotion/002_mixing_with_normal_props.php new file mode 100644 index 0000000000..bb6ed10aad --- /dev/null +++ b/tests/phpt/php8/property_promotion/002_mixing_with_normal_props.php @@ -0,0 +1,14 @@ +@ok php8 +prop2 = $prop1 . $param2; + } +} + +$test = new Test("hello ", "world"); +echo $test->prop1 . "\n"; +echo $test->prop2 . "\n"; diff --git a/tests/phpt/php8/property_promotion/003_with_default_value.php b/tests/phpt/php8/property_promotion/003_with_default_value.php new file mode 100644 index 0000000000..948dcc0e29 --- /dev/null +++ b/tests/phpt/php8/property_promotion/003_with_default_value.php @@ -0,0 +1,20 @@ +@ok php8 +x . "\n"; + echo $this->y . "\n"; + echo $this->z . "\n"; + } +} + +(new Point(10.0))->print(); +(new Point(10.0, 11.0))->print(); +(new Point(10.0, 11.0, 12.0))->print(); diff --git a/tests/phpt/php8/property_promotion/004_redeclarate_prop.php b/tests/phpt/php8/property_promotion/004_redeclarate_prop.php new file mode 100644 index 0000000000..33cc4a0f7c --- /dev/null +++ b/tests/phpt/php8/property_promotion/004_redeclarate_prop.php @@ -0,0 +1,9 @@ +@kphp_should_fail php8 +/Redeclaration of Test::\$prop/ +prop); diff --git a/tests/phpt/php8/property_promotion/011_variadic.php b/tests/phpt/php8/property_promotion/011_variadic.php new file mode 100644 index 0000000000..e2b4255ddf --- /dev/null +++ b/tests/phpt/php8/property_promotion/011_variadic.php @@ -0,0 +1,7 @@ +@kphp_should_fail php8 +/Cannot declare variadic promoted property/ + Date: Mon, 13 Sep 2021 11:34:52 +0300 Subject: [PATCH 3/5] added with_property_promotion flag to FunctionData --- compiler/data/function-data.h | 1 + compiler/gentree.cpp | 1 + compiler/pipes/gen-tree-postprocess.cpp | 15 ++++----------- compiler/pipes/gen-tree-postprocess.h | 2 +- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/compiler/data/function-data.h b/compiler/data/function-data.h index 2747a20506..d57513c6ba 100644 --- a/compiler/data/function-data.h +++ b/compiler/data/function-data.h @@ -106,6 +106,7 @@ class FunctionData { bool warn_unused_result = false; bool is_flatten = false; bool is_pure = false; + bool with_property_promotion = false; function_palette::ColorContainer colors{}; // colors specified with @kphp-color std::vector *next_with_colors{nullptr}; // next colored functions reachable via call graph diff --git a/compiler/gentree.cpp b/compiler/gentree.cpp index 2e29baa6a5..5b71ef2941 100644 --- a/compiler/gentree.cpp +++ b/compiler/gentree.cpp @@ -1657,6 +1657,7 @@ VertexAdaptor GenTree::get_function(TokenType tok, vk::string_view var->str_val = param->var()->str_val; cur_class->members.add_instance_field(var, {}, field_modifiers, "", param->type_hint); + cur_function->with_property_promotion = true; } } } diff --git a/compiler/pipes/gen-tree-postprocess.cpp b/compiler/pipes/gen-tree-postprocess.cpp index dbe89fb8b8..46a622f6d6 100644 --- a/compiler/pipes/gen-tree-postprocess.cpp +++ b/compiler/pipes/gen-tree-postprocess.cpp @@ -257,20 +257,17 @@ VertexPtr GenTreePostprocessPass::on_exit_vertex(VertexPtr root) { } if (auto fun = root.try_as()) { - process_property_promotion(fun); + if (fun->func_id->with_property_promotion) { + process_property_promotion(fun); + } } return root; } -void GenTreePostprocessPass::process_property_promotion(VertexAdaptor &fun) const { - if (!fun->func_id->is_constructor()) { - return; - } - +void GenTreePostprocessPass::process_property_promotion(VertexAdaptor fun) const { auto promoted_params = std::vector>(); - // TODO: add flag 'with_property_promotion' instead of iterating over parameters here const auto param_list = fun->param_list(); for (const auto &p : param_list->params()) { const auto ¶m = p.try_as(); @@ -280,10 +277,6 @@ void GenTreePostprocessPass::process_property_promotion(VertexAdaptorcmd()->as_vector(); // create an expression like "$this-> = " for each promoted property diff --git a/compiler/pipes/gen-tree-postprocess.h b/compiler/pipes/gen-tree-postprocess.h index ad8d2f9073..061f16da41 100644 --- a/compiler/pipes/gen-tree-postprocess.h +++ b/compiler/pipes/gen-tree-postprocess.h @@ -23,5 +23,5 @@ 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); - void process_property_promotion(VertexAdaptor &fun) const; + void process_property_promotion(VertexAdaptor fun) const; }; From a0fbc512e6b1adbbba956c74f09f48177e55f12f Mon Sep 17 00:00:00 2001 From: i582 <51853996+i582@users.noreply.github.com> Date: Mon, 13 Sep 2021 11:38:11 +0300 Subject: [PATCH 4/5] added missing tests --- tests/phpt/php8/property_promotion/001_basic.php | 10 ++++++++++ tests/phpt/php8/property_promotion/012_private.php | 11 +++++++++++ 2 files changed, 21 insertions(+) create mode 100644 tests/phpt/php8/property_promotion/012_private.php diff --git a/tests/phpt/php8/property_promotion/001_basic.php b/tests/phpt/php8/property_promotion/001_basic.php index e69de29bb2..0f1bd4dfab 100644 --- a/tests/phpt/php8/property_promotion/001_basic.php +++ b/tests/phpt/php8/property_promotion/001_basic.php @@ -0,0 +1,10 @@ +@kphp_should_fail php8 +/assign string to Point::\$x/ +x = "foo"; diff --git a/tests/phpt/php8/property_promotion/012_private.php b/tests/phpt/php8/property_promotion/012_private.php new file mode 100644 index 0000000000..86baa1e7af --- /dev/null +++ b/tests/phpt/php8/property_promotion/012_private.php @@ -0,0 +1,11 @@ +@kphp_should_fail php8 +/Can't access private field z/ +x; +echo $point->z; From e8f5a6d96990fdfd34f93abf5035c1f088fd3154 Mon Sep 17 00:00:00 2001 From: i582 <51853996+i582@users.noreply.github.com> Date: Mon, 13 Sep 2021 11:44:00 +0300 Subject: [PATCH 5/5] removed unnecessary semicolon --- compiler/pipes/gen-tree-postprocess.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/pipes/gen-tree-postprocess.cpp b/compiler/pipes/gen-tree-postprocess.cpp index 46a622f6d6..780a4f9801 100644 --- a/compiler/pipes/gen-tree-postprocess.cpp +++ b/compiler/pipes/gen-tree-postprocess.cpp @@ -300,7 +300,7 @@ void GenTreePostprocessPass::process_property_promotion(VertexAdaptorcmd_ref() = VertexAdaptor::create(func_stmts);; + fun->cmd_ref() = VertexAdaptor::create(func_stmts); } VertexAdaptor array_vertex_from_slice(const VertexRange &args, size_t start, size_t end) {