From efdc3151b25c261f4c02ee98430d3439ec39bc7c Mon Sep 17 00:00:00 2001 From: Mikhail Kornaukhov Date: Tue, 22 Nov 2022 23:00:35 +0300 Subject: [PATCH 1/4] [php8] Implement nullsafe operator --- compiler/debug.cpp | 1 + compiler/gentree.cpp | 9 +- compiler/gentree.h | 2 +- compiler/lexer.cpp | 13 +- compiler/pipes/gen-tree-postprocess.cpp | 75 ++++++++ compiler/token.h | 1 + compiler/vertex-desc.json | 8 + tests/phpt/php8/001_nullsafe_op_simple.php | 172 ++++++++++++++++++ .../php8/002_nullsafe_op_special_cases.php | 65 +++++++ .../phpt/php8/101_nullsafe_op_check_null.php | 31 ++++ tests/phpt/php8/102_nullsafe_op_crazy.php | 7 + tests/phpt/php8/103_nullsafe_op_absent.php | 34 ++++ 12 files changed, 412 insertions(+), 6 deletions(-) create mode 100644 tests/phpt/php8/001_nullsafe_op_simple.php create mode 100644 tests/phpt/php8/002_nullsafe_op_special_cases.php create mode 100644 tests/phpt/php8/101_nullsafe_op_check_null.php create mode 100644 tests/phpt/php8/102_nullsafe_op_crazy.php create mode 100644 tests/phpt/php8/103_nullsafe_op_absent.php diff --git a/compiler/debug.cpp b/compiler/debug.cpp index 795ed12fd9..075303b05e 100644 --- a/compiler/debug.cpp +++ b/compiler/debug.cpp @@ -152,6 +152,7 @@ std::string debugTokenName(TokenType t) { {tok_double_arrow, "tok_double_arrow"}, {tok_double_colon, "tok_double_colon"}, {tok_arrow, "tok_arrow"}, + {tok_nullsafe_arrow, "tok_nullsafe_arrow"}, {tok_class_c, "tok_class_c"}, {tok_file_c, "tok_file_c"}, {tok_file_relative_c, "tok_file_relative_c"}, diff --git a/compiler/gentree.cpp b/compiler/gentree.cpp index 11b70840e5..68cf24d35e 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" @@ -332,12 +333,12 @@ VertexPtr GenTree::get_postfix_expression(VertexPtr res, bool parenthesized) { } res.set_location(location); need = true; - } else if (tp == tok_arrow) { + } else if (tp == tok_arrow || tp == tok_nullsafe_arrow) { auto location = auto_location(); next_cur(); VertexPtr rhs = get_expr_top(true); CE (!kphp_error(rhs, "Failed to parse right argument of '->'")); - res = process_arrow(res, rhs); + res = process_arrow(res, rhs, tp == tok_nullsafe_arrow); CE(res); res.set_location(location); need = true; @@ -1782,10 +1783,11 @@ VertexAdaptor GenTree::auto_capture_this_in_lambda(FunctionPtr f_lambda) return v_captured_this; } -VertexPtr GenTree::process_arrow(VertexPtr lhs, VertexPtr rhs) { +VertexPtr GenTree::process_arrow(VertexPtr lhs, VertexPtr rhs, bool is_null_safe) { if (rhs->type() == op_func_name) { auto inst_prop = VertexAdaptor::create(lhs); inst_prop->str_val = rhs->get_string(); + inst_prop->is_null_safe = is_null_safe; return inst_prop; } else if (auto as_func_call = rhs.try_as()) { @@ -1793,6 +1795,7 @@ VertexPtr GenTree::process_arrow(VertexPtr lhs, VertexPtr rhs) { new_root->extra_type = op_ex_func_call_arrow; new_root->str_val = as_func_call->str_val; new_root->reifiedTs = as_func_call->reifiedTs; + new_root->is_null_safe = is_null_safe; return new_root; } else { diff --git a/compiler/gentree.h b/compiler/gentree.h index 31e5fe6314..462fd6f486 100644 --- a/compiler/gentree.h +++ b/compiler/gentree.h @@ -121,7 +121,7 @@ class GenTree { VertexPtr get_class(const PhpDocComment *phpdoc, ClassType class_type); void parse_extends_implements(); - static VertexPtr process_arrow(VertexPtr lhs, VertexPtr rhs); + static VertexPtr process_arrow(VertexPtr lhs, VertexPtr rhs, bool is_null_safe = false); private: const TypeHint *get_typehint(); diff --git a/compiler/lexer.cpp b/compiler/lexer.cpp index aee442b81a..14cf6c47d5 100644 --- a/compiler/lexer.cpp +++ b/compiler/lexer.cpp @@ -204,9 +204,9 @@ void LexerData::hack_last_tokens() { } /** - * For a case when we encounter a keyword after the '->' it should be a tok_func_name, + * For a case when we encounter a keyword after the '->' and '?->' it should be a tok_func_name, * not tok_array, tok_try, etc. - * For example: $c->array, $c->try + * For example: $c->array, $c?->try */ if (are_last_tokens(tok_arrow, any_token_tag{})) { if (!tokens.back().str_val.empty() && is_alpha(tokens.back().str_val[0])) { @@ -215,6 +215,13 @@ void LexerData::hack_last_tokens() { } } + if (are_last_tokens(tok_nullsafe_arrow, any_token_tag{})) { + if (!tokens.back().str_val.empty() && is_alpha(tokens.back().str_val[0])) { + tokens.back().type_ = tok_func_name; + return; + } + } + // replace elseif with else+if, but not if the previous token can cause elseif // to be interpreted as identifier (class const and method) if (are_last_tokens(tok_elseif)) { @@ -1112,6 +1119,7 @@ void TokenLexerCommon::init() { add_rule(h, "=>", tok_double_arrow); add_rule(h, "::", tok_double_colon); add_rule(h, "->", tok_arrow); + add_rule(h, "?->", tok_nullsafe_arrow); add_rule(h, "...", tok_varg); } @@ -1180,6 +1188,7 @@ void TokenLexerPHPDoc::init() { add_rule(h, "::", tok_double_colon); add_rule(h, "=>", tok_double_arrow); add_rule(h, "->", tok_arrow); + add_rule(h, "?->", tok_nullsafe_arrow); add_rule(h, "...", tok_varg); add_rule(h, "=", tok_eq1); } diff --git a/compiler/pipes/gen-tree-postprocess.cpp b/compiler/pipes/gen-tree-postprocess.cpp index ddd9b2a94b..6681d49fa3 100644 --- a/compiler/pipes/gen-tree-postprocess.cpp +++ b/compiler/pipes/gen-tree-postprocess.cpp @@ -8,6 +8,8 @@ #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-util.h" namespace { @@ -230,6 +232,41 @@ VertexPtr GenTreePostprocessPass::on_enter_vertex(VertexPtr root) { } VertexPtr GenTreePostprocessPass::on_exit_vertex(VertexPtr root) { + if (auto as_instance_prop = root.try_as(); as_instance_prop && as_instance_prop->is_null_safe) { + if (as_instance_prop->instance().try_as() || as_instance_prop->instance().try_as() || + (as_instance_prop->instance().try_as() && + as_instance_prop->instance().try_as()->false_expr()->type() == op_func_call) + ) { + auto before_arrow = as_instance_prop->instance(); + auto tmp_var = VertexAdaptor::create().set_location(root); + tmp_var->str_val = gen_unique_name("tmp_before_arrow"); + tmp_var->extra_type = op_ex_var_superlocal_inplace; + + auto set_call_to_tmp = VertexAdaptor::create(tmp_var, before_arrow).set_location(before_arrow); + + auto cond = VertexAdaptor::create(VertexAdaptor::create(), tmp_var).set_location(before_arrow); + + auto new_as_inst = VertexAdaptor::create(tmp_var).set_location(before_arrow); + new_as_inst->str_val = as_instance_prop->str_val; + + auto ternary = VertexAdaptor::create(VertexAdaptor::create(cond), + VertexAdaptor::create(), + new_as_inst).set_location(root); + + return VertexAdaptor::create(set_call_to_tmp, ternary).set_location(root); + } + + auto cond = VertexAdaptor::create(VertexAdaptor::create(), + as_instance_prop->instance().clone()).set_location(root); + VertexPtr reconstructed = as_instance_prop; + if (auto as_ternary = as_instance_prop->instance().try_as()) { + reconstructed = VertexAdaptor::create(as_ternary->false_expr()).set_location(root); + reconstructed.as()->str_val = as_instance_prop->str_val; + } + return VertexAdaptor::create(VertexAdaptor::create(cond), + VertexAdaptor::create(), + reconstructed).set_location(root); + } if (root->type() == op_var) { if (VertexUtil::is_superglobal(root->get_string())) { root->extra_type = op_ex_var_superglobal; @@ -237,6 +274,44 @@ VertexPtr GenTreePostprocessPass::on_exit_vertex(VertexPtr root) { } if (auto call = root.try_as()) { + if (call->is_null_safe) { + if (call->begin()->try_as() || call->begin()->try_as() || + (call->begin()->try_as() && + call->begin()->try_as()->false_expr()->type() == op_func_call) + ) { + auto before_arrow = *call->begin(); + + auto tmp_var = VertexAdaptor::create().set_location(before_arrow); + tmp_var->str_val = gen_unique_name("tmp_before_arrow"); + tmp_var->extra_type = op_ex_var_superlocal_inplace; + + auto set_call_to_tmp = VertexAdaptor::create(tmp_var, before_arrow).set_location(before_arrow); + + auto cond = VertexAdaptor::create(VertexAdaptor::create(), + tmp_var).set_location(before_arrow); + auto new_call = call.clone(); + *new_call->begin() = tmp_var; + auto ternary = VertexAdaptor::create(VertexAdaptor::create(cond), + VertexAdaptor::create(), + new_call).set_location(before_arrow); + + return VertexAdaptor::create(set_call_to_tmp, ternary).set_location(before_arrow); + } + + auto cond = VertexAdaptor::create(VertexAdaptor::create(), + call->begin()->clone()).set_location(root); + VertexPtr reconstructed = call; + if (auto as_ternary = call->begin()->try_as()) { + reconstructed = VertexAdaptor::create(as_ternary->false_expr(), + VertexRange(call->begin() + 1, call->end())); + reconstructed->extra_type = call->extra_type; + reconstructed.as()->str_val = call->str_val; + reconstructed.as()->reifiedTs = call->reifiedTs; + } + return VertexAdaptor::create(VertexAdaptor::create(cond), + VertexAdaptor::create(), + reconstructed).set_location(root); + } if (!G->settings().profiler_level.get() && call->size() == 1 && vk::any_of_equal(call->get_string(), "profiler_set_log_suffix", "profiler_set_function_label")) { return VertexAdaptor::create(VertexAdaptor::create(), VertexUtil::embrace(call)).set_location_recursively(root); diff --git a/compiler/token.h b/compiler/token.h index 8303527035..ea372104a3 100644 --- a/compiler/token.h +++ b/compiler/token.h @@ -138,6 +138,7 @@ enum TokenType { tok_double_arrow, tok_double_colon, tok_arrow, + tok_nullsafe_arrow, tok_class_c, tok_file_c, diff --git a/compiler/vertex-desc.json b/compiler/vertex-desc.json index 0db7e0733c..fce9220886 100644 --- a/compiler/vertex-desc.json +++ b/compiler/vertex-desc.json @@ -314,6 +314,10 @@ "reifiedTs": { "type": "GenericsInstantiationMixin *", "default": "nullptr" + }, + "is_null_safe": { + "type": "bool", + "default": "false" } } }, @@ -519,6 +523,10 @@ "access_type": { "type": "InstancePropAccessType", "default": "InstancePropAccessType::Default" + }, + "is_null_safe": { + "type": "bool", + "default": "false" } } }, diff --git a/tests/phpt/php8/001_nullsafe_op_simple.php b/tests/phpt/php8/001_nullsafe_op_simple.php new file mode 100644 index 0000000000..a99e87fbd0 --- /dev/null +++ b/tests/phpt/php8/001_nullsafe_op_simple.php @@ -0,0 +1,172 @@ +@ok php8 + +dd = 42; + } + function null() { + var_dump('C::null()'); + if (2 > 1) { + return $this; + } + return null; + } + + function self() { + var_dump('C::self()'); + return $this; + } + + function d() { + var_dump('C::d()'); + return $this->dd; + } + + function d_param(int $tmp, int $tmp2, int $tmp3) { + var_dump('C::d_param(int, int, int)'); + return $this->dd; + } +} + +class B { + public C $cc; + function __construct() { + $this->cc = new C; + } + function null() { + var_dump('B::null()'); + if (2 > 1) { + return $this; + } + return null; + } + + function self() { + var_dump('B::self()'); + return $this; + } + + function c() { + var_dump('B::c()'); + return $this->cc; + } + + function c_param(int $tmp, int $tmp2) { + var_dump('B::c_param(int, int)'); + return $this->cc; + } +} + +class A { + public B $bb; + function __construct() { + $this->bb = new B; + } + function null() { + var_dump('A::null()'); + if (2 > 1) { + return $this; + } + return null; + } + + function self() { + var_dump('A::self()'); + return $this; + } + + function b() { + var_dump('A::b()'); + return $this->bb; + } + + function b_param(int $tmp) { + var_dump('A::b_param(int)'); + return $this->bb; + } +} + +function getA() { + return new A; +} + +/// Start testing + +function field_chain(?A $a) { + return $a?->bb?->cc?->dd; +} + +function field_chain_starts_with_fun_call() { + return getA()?->bb?->cc?->dd; +} + +var_dump(field_chain(new A)); +var_dump(field_chain(NULL)); +var_dump(field_chain_starts_with_fun_call()); + +function method_chain(?A $a) { + return $a?->self()?->b()?->self()?->c()?->self()?->d(); +} + +function method_chain_starts_with_fun_call() { + return getA()?->self()?->b()?->self()?->c()?->self()?->dd; +} + +var_dump(method_chain(new A)); +var_dump(method_chain(NULL)); +var_dump(method_chain_starts_with_fun_call()); + + +function method_params_chain(?A $a) { + return $a?->self()?->b_param(1)?->self()?->c_param(42, 146)?->self()?->d_param(42, 146, 42); +} + +var_dump(method_params_chain(new A)); +var_dump(method_params_chain(NULL)); + +function mixed_chain(?A $a) { + return $a?->self()?->bb?->self()?->cc?->self()?->d(); +} + +function mixed_chain_starts_with_fun_call() { + return getA()?->bb?->self()?->cc?->self()?->dd; +} + +var_dump(mixed_chain(new A)); +var_dump(mixed_chain(NULL)); +var_dump(mixed_chain_starts_with_fun_call()); + +function null_chain_1(?A $a) { + return $a?->null()?->bb?->self()?->cc?->self()?->d(); +} + +function null_chain_2(?A $a) { + return $a?->self()?->b()?->null()?->c()?->self()?->d(); +} + +function null_chain_3(?A $a) { + return $a?->self()?->bb?->self()?->c()?->null()?->d(); +} + +var_dump(null_chain_1(new A)); +var_dump(null_chain_1(null)); +var_dump(null_chain_2(new A)); +var_dump(null_chain_2(null)); +var_dump(null_chain_3(new A)); +var_dump(null_chain_3(null)); + +function or_null(?A $a) { + return $a?->self()?->bb?->self()?->c(); +} +$res = or_null(getA()); +if ($res) { + var_dump($res->d()); +} + +?> \ No newline at end of file diff --git a/tests/phpt/php8/002_nullsafe_op_special_cases.php b/tests/phpt/php8/002_nullsafe_op_special_cases.php new file mode 100644 index 0000000000..f0ccb16d5d --- /dev/null +++ b/tests/phpt/php8/002_nullsafe_op_special_cases.php @@ -0,0 +1,65 @@ +@ok php8 + +data = 42; + } + function NULL() { + var_dump('A::NULL()'); + if (2 > 1) { + return $this; + } + return NULL; + } + + function self() { + var_dump('A::self()'); + return $this; + } + + function data() { + var_dump('A::data()'); + return $this->data; + } + + function data_param(int $tmp) { + var_dump('A::data_param(int)'); + return $this->data; + } +} + +function getA() { + return new A; +} + +function condA(int $val) { + if ($val % 2 == 1) { + return new A(); + } + return NULL; +} + +/// Start testing +/** @var A[] $arr */ +$arr = array(NULL, getA(), new A, NULL); +var_dump($arr[0]?->data); +var_dump($arr[1]?->data()); +var_dump($arr[2]?->data); +var_dump($arr[3]?->data()); + +$a = condA(0); +var_dump(($a === NULL ? getA() : condA(1))?->data); +var_dump(($a === NULL ? getA() : condA(0))?->data); +$a = condA(1); +var_dump(($a === NULL ? getA() : condA(1))?->data); +var_dump(($a === NULL ? getA() : condA(0))?->data); + +?> \ No newline at end of file diff --git a/tests/phpt/php8/101_nullsafe_op_check_null.php b/tests/phpt/php8/101_nullsafe_op_check_null.php new file mode 100644 index 0000000000..b37198084b --- /dev/null +++ b/tests/phpt/php8/101_nullsafe_op_check_null.php @@ -0,0 +1,31 @@ +@kphp_should_fail php8 + +/Invalid property ...->dd: Can not parse: maybe, too deep nesting/ +dd = 42; + } + + function d() { + var_dump('C::d()'); + return $this->dd; + } +} + +class B { + public C $cc; + function __construct() { + $this->cc = new C; + } + + function c() { + var_dump('B::c()'); + return $this->cc; + } +} + +var_dump((new B())?->c()->dd); // should use ?->dd +?> \ No newline at end of file diff --git a/tests/phpt/php8/102_nullsafe_op_crazy.php b/tests/phpt/php8/102_nullsafe_op_crazy.php new file mode 100644 index 0000000000..3e381507a2 --- /dev/null +++ b/tests/phpt/php8/102_nullsafe_op_crazy.php @@ -0,0 +1,7 @@ +@kphp_should_fail @kphp8 + +absent()); +var_dump(return?->property); +?> + diff --git a/tests/phpt/php8/103_nullsafe_op_absent.php b/tests/phpt/php8/103_nullsafe_op_absent.php new file mode 100644 index 0000000000..33bafc671f --- /dev/null +++ b/tests/phpt/php8/103_nullsafe_op_absent.php @@ -0,0 +1,34 @@ +@kphp_should_fail kphp8 + +/Method absent() not found in class B/ +/Method absent() not found in class C/ + +dd = 42; + } + + function d() { + var_dump('C::d()'); + return $this->dd; + } +} + +class B { + public C $cc; + function __construct() { + $this->cc = new C; + } + + function c() { + var_dump('B::c()'); + return $this->cc; + } +} + +var_dump((new B)?->absent()); +var_dump((new B)?->c()?->absent()); +?> \ No newline at end of file From daf30e67e6dd3bcb7c733ea4667d538e7ad46c78 Mon Sep 17 00:00:00 2001 From: Mikhail Kornaukhov Date: Tue, 24 Jan 2023 08:55:31 +0300 Subject: [PATCH 2/4] Add tests --- tests/cpp/compiler/lexer-test.cpp | 3 ++ .../001_nullsafe_op_simple.php | 13 ++++++--- .../002_nullsafe_op_special_cases.php | 0 .../101_nullsafe_op_check_null.php | 0 .../102_nullsafe_op_crazy.php | 0 .../103_nullsafe_op_absent.php | 0 .../nullsafe_op/104_prohibit_lhs_property.php | 29 +++++++++++++++++++ .../nullsafe_op/105_prohibit_lhs_method.php | 29 +++++++++++++++++++ 8 files changed, 70 insertions(+), 4 deletions(-) rename tests/phpt/php8/{ => nullsafe_op}/001_nullsafe_op_simple.php (92%) rename tests/phpt/php8/{ => nullsafe_op}/002_nullsafe_op_special_cases.php (100%) rename tests/phpt/php8/{ => nullsafe_op}/101_nullsafe_op_check_null.php (100%) rename tests/phpt/php8/{ => nullsafe_op}/102_nullsafe_op_crazy.php (100%) rename tests/phpt/php8/{ => nullsafe_op}/103_nullsafe_op_absent.php (100%) create mode 100644 tests/phpt/php8/nullsafe_op/104_prohibit_lhs_property.php create mode 100644 tests/phpt/php8/nullsafe_op/105_prohibit_lhs_method.php diff --git a/tests/cpp/compiler/lexer-test.cpp b/tests/cpp/compiler/lexer-test.cpp index ef2cd8e8b9..bf0b4a4803 100644 --- a/tests/cpp/compiler/lexer-test.cpp +++ b/tests/cpp/compiler/lexer-test.cpp @@ -19,6 +19,9 @@ TEST(lexer_test, test_php_tokens) { {"$obj->exit", {"tok_var_name($obj)", "tok_arrow(->)", "tok_func_name(exit)"}}, {"$obj->exit()", {"tok_var_name($obj)", "tok_arrow(->)", "tok_func_name(exit)", "tok_oppar(()", "tok_clpar())"}}, {"$obj->throw", {"tok_var_name($obj)", "tok_arrow(->)", "tok_func_name(throw)"}}, + {"$obj?->exit", {"tok_var_name($obj)", "tok_nullsafe_arrow(?->)", "tok_func_name(exit)"}}, + {"$obj?->exit()", {"tok_var_name($obj)", "tok_nullsafe_arrow(?->)", "tok_func_name(exit)", "tok_oppar(()", "tok_clpar())"}}, + {"$obj?->throw", {"tok_var_name($obj)", "tok_nullsafe_arrow(?->)", "tok_func_name(throw)"}}, {"Example::for", {"tok_func_name(Example::for)"}}, {"Example::for()", {"tok_func_name(Example::for)", "tok_oppar(()", "tok_clpar())"}}, diff --git a/tests/phpt/php8/001_nullsafe_op_simple.php b/tests/phpt/php8/nullsafe_op/001_nullsafe_op_simple.php similarity index 92% rename from tests/phpt/php8/001_nullsafe_op_simple.php rename to tests/phpt/php8/nullsafe_op/001_nullsafe_op_simple.php index a99e87fbd0..d484a36fe1 100644 --- a/tests/phpt/php8/001_nullsafe_op_simple.php +++ b/tests/phpt/php8/nullsafe_op/001_nullsafe_op_simple.php @@ -2,9 +2,6 @@ bb; } + + public static B $b_static; + public static function get_b_static() { + return self::$b_static; + } + } function getA() { @@ -169,4 +172,6 @@ function or_null(?A $a) { var_dump($res->d()); } -?> \ No newline at end of file +A::$b_static = new B; +var_dump(A::$b_static?->null()?->c()?->self()?->d()); +var_dump(A::get_b_static()?->null()?->c()?->self()?->d()); \ No newline at end of file diff --git a/tests/phpt/php8/002_nullsafe_op_special_cases.php b/tests/phpt/php8/nullsafe_op/002_nullsafe_op_special_cases.php similarity index 100% rename from tests/phpt/php8/002_nullsafe_op_special_cases.php rename to tests/phpt/php8/nullsafe_op/002_nullsafe_op_special_cases.php diff --git a/tests/phpt/php8/101_nullsafe_op_check_null.php b/tests/phpt/php8/nullsafe_op/101_nullsafe_op_check_null.php similarity index 100% rename from tests/phpt/php8/101_nullsafe_op_check_null.php rename to tests/phpt/php8/nullsafe_op/101_nullsafe_op_check_null.php diff --git a/tests/phpt/php8/102_nullsafe_op_crazy.php b/tests/phpt/php8/nullsafe_op/102_nullsafe_op_crazy.php similarity index 100% rename from tests/phpt/php8/102_nullsafe_op_crazy.php rename to tests/phpt/php8/nullsafe_op/102_nullsafe_op_crazy.php diff --git a/tests/phpt/php8/103_nullsafe_op_absent.php b/tests/phpt/php8/nullsafe_op/103_nullsafe_op_absent.php similarity index 100% rename from tests/phpt/php8/103_nullsafe_op_absent.php rename to tests/phpt/php8/nullsafe_op/103_nullsafe_op_absent.php diff --git a/tests/phpt/php8/nullsafe_op/104_prohibit_lhs_property.php b/tests/phpt/php8/nullsafe_op/104_prohibit_lhs_property.php new file mode 100644 index 0000000000..7ed2df4a38 --- /dev/null +++ b/tests/phpt/php8/nullsafe_op/104_prohibit_lhs_property.php @@ -0,0 +1,29 @@ +@kphp_should_fail kphp8 +dd = 42; + } + + function d() { + var_dump('C::d()'); + return $this->dd; + } +} + +class B { + public C $cc; + function __construct() { + $this->cc = new C; + } + + function c() { + var_dump('B::c()'); + return $this->cc; + } +} + +$b = new B; +$b?->cc = new C; diff --git a/tests/phpt/php8/nullsafe_op/105_prohibit_lhs_method.php b/tests/phpt/php8/nullsafe_op/105_prohibit_lhs_method.php new file mode 100644 index 0000000000..0b1437e806 --- /dev/null +++ b/tests/phpt/php8/nullsafe_op/105_prohibit_lhs_method.php @@ -0,0 +1,29 @@ +@kphp_should_fail kphp8 +dd = 42; + } + + function d() { + var_dump('C::d()'); + return $this->dd; + } +} + +class B { + public C $cc; + function __construct() { + $this->cc = new C; + } + + function c() { + var_dump('B::c()'); + return $this->cc; + } +} + +$b = new B; +$b?->c() = new C; From d9848d9239fc6eee449b32f75ae1d990cd824a42 Mon Sep 17 00:00:00 2001 From: mkornaukhov03 Date: Wed, 8 Feb 2023 14:00:43 +0300 Subject: [PATCH 3/4] Fix tests --- tests/phpt/php8/nullsafe_op/001_nullsafe_op_simple.php | 3 +-- .../php8/nullsafe_op/002_nullsafe_op_special_cases.php | 6 ------ tests/phpt/php8/nullsafe_op/101_nullsafe_op_check_null.php | 2 -- tests/phpt/php8/nullsafe_op/102_nullsafe_op_crazy.php | 3 --- tests/phpt/php8/nullsafe_op/103_nullsafe_op_absent.php | 7 ++----- 5 files changed, 3 insertions(+), 18 deletions(-) diff --git a/tests/phpt/php8/nullsafe_op/001_nullsafe_op_simple.php b/tests/phpt/php8/nullsafe_op/001_nullsafe_op_simple.php index d484a36fe1..8b20e19f0b 100644 --- a/tests/phpt/php8/nullsafe_op/001_nullsafe_op_simple.php +++ b/tests/phpt/php8/nullsafe_op/001_nullsafe_op_simple.php @@ -1,5 +1,4 @@ @ok php8 - null()?->c()?->self()?->d()); -var_dump(A::get_b_static()?->null()?->c()?->self()?->d()); \ No newline at end of file +var_dump(A::get_b_static()?->null()?->c()?->self()?->d()); diff --git a/tests/phpt/php8/nullsafe_op/002_nullsafe_op_special_cases.php b/tests/phpt/php8/nullsafe_op/002_nullsafe_op_special_cases.php index f0ccb16d5d..9a49eae851 100644 --- a/tests/phpt/php8/nullsafe_op/002_nullsafe_op_special_cases.php +++ b/tests/phpt/php8/nullsafe_op/002_nullsafe_op_special_cases.php @@ -1,12 +1,8 @@ @ok php8 - data); var_dump(($a === NULL ? getA() : condA(0))?->data); - -?> \ No newline at end of file diff --git a/tests/phpt/php8/nullsafe_op/101_nullsafe_op_check_null.php b/tests/phpt/php8/nullsafe_op/101_nullsafe_op_check_null.php index b37198084b..41f3c2bd9b 100644 --- a/tests/phpt/php8/nullsafe_op/101_nullsafe_op_check_null.php +++ b/tests/phpt/php8/nullsafe_op/101_nullsafe_op_check_null.php @@ -1,5 +1,4 @@ @kphp_should_fail php8 - /Invalid property ...->dd: Can not parse: maybe, too deep nesting/ c()->dd); // should use ?->dd -?> \ No newline at end of file diff --git a/tests/phpt/php8/nullsafe_op/102_nullsafe_op_crazy.php b/tests/phpt/php8/nullsafe_op/102_nullsafe_op_crazy.php index 3e381507a2..29c1719c6f 100644 --- a/tests/phpt/php8/nullsafe_op/102_nullsafe_op_crazy.php +++ b/tests/phpt/php8/nullsafe_op/102_nullsafe_op_crazy.php @@ -1,7 +1,4 @@ @kphp_should_fail @kphp8 - absent()); var_dump(return?->property); -?> - diff --git a/tests/phpt/php8/nullsafe_op/103_nullsafe_op_absent.php b/tests/phpt/php8/nullsafe_op/103_nullsafe_op_absent.php index 33bafc671f..e337a9ab4f 100644 --- a/tests/phpt/php8/nullsafe_op/103_nullsafe_op_absent.php +++ b/tests/phpt/php8/nullsafe_op/103_nullsafe_op_absent.php @@ -1,8 +1,6 @@ @kphp_should_fail kphp8 - -/Method absent() not found in class B/ -/Method absent() not found in class C/ - +/Method absent\(\) not found in class B/ +/Method absent\(\) not found in class C/ absent()); var_dump((new B)?->c()?->absent()); -?> \ No newline at end of file From 082e2566b0fdf4e8ed360bc2fbd332b700a6c513 Mon Sep 17 00:00:00 2001 From: mkornaukhov03 Date: Wed, 8 Feb 2023 17:42:34 +0300 Subject: [PATCH 4/4] Fix review notes; simplify code --- compiler/gentree.cpp | 2 +- compiler/pipes/gen-tree-postprocess.cpp | 129 +++++++++++------------- 2 files changed, 58 insertions(+), 73 deletions(-) diff --git a/compiler/gentree.cpp b/compiler/gentree.cpp index 68cf24d35e..823facdce0 100644 --- a/compiler/gentree.cpp +++ b/compiler/gentree.cpp @@ -333,7 +333,7 @@ VertexPtr GenTree::get_postfix_expression(VertexPtr res, bool parenthesized) { } res.set_location(location); need = true; - } else if (tp == tok_arrow || tp == tok_nullsafe_arrow) { + } else if (vk::any_of_equal(tp, tok_arrow, tok_nullsafe_arrow)) { auto location = auto_location(); next_cur(); VertexPtr rhs = get_expr_top(true); diff --git a/compiler/pipes/gen-tree-postprocess.cpp b/compiler/pipes/gen-tree-postprocess.cpp index 6681d49fa3..e0bbbd41ab 100644 --- a/compiler/pipes/gen-tree-postprocess.cpp +++ b/compiler/pipes/gen-tree-postprocess.cpp @@ -4,13 +4,16 @@ #include "compiler/pipes/gen-tree-postprocess.h" +#include "auto/compiler/vertex/vertex-types.h" #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/kphp_assert.h" #include "compiler/name-gen.h" #include "compiler/vertex-util.h" +#include "runtime/php_assert.h" namespace { template @@ -232,86 +235,68 @@ VertexPtr GenTreePostprocessPass::on_enter_vertex(VertexPtr root) { } VertexPtr GenTreePostprocessPass::on_exit_vertex(VertexPtr root) { - if (auto as_instance_prop = root.try_as(); as_instance_prop && as_instance_prop->is_null_safe) { - if (as_instance_prop->instance().try_as() || as_instance_prop->instance().try_as() || - (as_instance_prop->instance().try_as() && - as_instance_prop->instance().try_as()->false_expr()->type() == op_func_call) - ) { - auto before_arrow = as_instance_prop->instance(); - auto tmp_var = VertexAdaptor::create().set_location(root); - tmp_var->str_val = gen_unique_name("tmp_before_arrow"); - tmp_var->extra_type = op_ex_var_superlocal_inplace; - - auto set_call_to_tmp = VertexAdaptor::create(tmp_var, before_arrow).set_location(before_arrow); - - auto cond = VertexAdaptor::create(VertexAdaptor::create(), tmp_var).set_location(before_arrow); - - auto new_as_inst = VertexAdaptor::create(tmp_var).set_location(before_arrow); - new_as_inst->str_val = as_instance_prop->str_val; - - auto ternary = VertexAdaptor::create(VertexAdaptor::create(cond), - VertexAdaptor::create(), - new_as_inst).set_location(root); - - return VertexAdaptor::create(set_call_to_tmp, ternary).set_location(root); - } - - auto cond = VertexAdaptor::create(VertexAdaptor::create(), - as_instance_prop->instance().clone()).set_location(root); - VertexPtr reconstructed = as_instance_prop; - if (auto as_ternary = as_instance_prop->instance().try_as()) { - reconstructed = VertexAdaptor::create(as_ternary->false_expr()).set_location(root); - reconstructed.as()->str_val = as_instance_prop->str_val; - } - return VertexAdaptor::create(VertexAdaptor::create(cond), - VertexAdaptor::create(), - reconstructed).set_location(root); - } if (root->type() == op_var) { if (VertexUtil::is_superglobal(root->get_string())) { root->extra_type = op_ex_var_superglobal; } } - if (auto call = root.try_as()) { - if (call->is_null_safe) { - if (call->begin()->try_as() || call->begin()->try_as() || - (call->begin()->try_as() && - call->begin()->try_as()->false_expr()->type() == op_func_call) - ) { - auto before_arrow = *call->begin(); - - auto tmp_var = VertexAdaptor::create().set_location(before_arrow); - tmp_var->str_val = gen_unique_name("tmp_before_arrow"); - tmp_var->extra_type = op_ex_var_superlocal_inplace; - - auto set_call_to_tmp = VertexAdaptor::create(tmp_var, before_arrow).set_location(before_arrow); - - auto cond = VertexAdaptor::create(VertexAdaptor::create(), - tmp_var).set_location(before_arrow); - auto new_call = call.clone(); - *new_call->begin() = tmp_var; - auto ternary = VertexAdaptor::create(VertexAdaptor::create(cond), - VertexAdaptor::create(), - new_call).set_location(before_arrow); - - return VertexAdaptor::create(set_call_to_tmp, ternary).set_location(before_arrow); - } - - auto cond = VertexAdaptor::create(VertexAdaptor::create(), - call->begin()->clone()).set_location(root); - VertexPtr reconstructed = call; - if (auto as_ternary = call->begin()->try_as()) { - reconstructed = VertexAdaptor::create(as_ternary->false_expr(), - VertexRange(call->begin() + 1, call->end())); - reconstructed->extra_type = call->extra_type; - reconstructed.as()->str_val = call->str_val; - reconstructed.as()->reifiedTs = call->reifiedTs; + // Transformation of expr?->field_or_func_call to + // { + // $tmp_unique_name = expr; + // $tmp_unique_name === null ? null : $tmp_unique_name->field_or_func_call; + // } + // We use op_seq_rval for State Exprs extension (the last statement in the block + // is used as value of whole block) + // Naive implementation could look like + // $expr === null? null : $expr->field_or_func_call + // But in case of foo()?->field_or_func_call it could call "foo()" twice + // That is why we store "expr" to temporary variable + auto transform_nullsufe = [](VertexPtr root) { + const auto before_nullsafe_arrow = [&root]() { + if (root->type() == op_instance_prop) { + return root.as()->instance(); } - return VertexAdaptor::create(VertexAdaptor::create(cond), - VertexAdaptor::create(), - reconstructed).set_location(root); + kphp_assert_msg(root->type() == op_func_call, "Internal compiler error: transformation of nullsafe operator failf"); + return *root.as()->begin(); + }(); + auto tmp_var = VertexAdaptor::create().set_location(root); + tmp_var->str_val = gen_unique_name("tmp_before_nullsafe_arrow"); + tmp_var->extra_type = op_ex_var_superlocal_inplace; + + auto assignment = VertexAdaptor::create(tmp_var, before_nullsafe_arrow).set_location(root); + auto cond = VertexAdaptor::create(VertexAdaptor::create(), tmp_var).set_location(root); + + VertexPtr transformed_vertex {}; + if (auto as_instance_prop = root.try_as()) { + transformed_vertex = VertexAdaptor::create(tmp_var).set_location(root); + transformed_vertex.as()->str_val = as_instance_prop->str_val; } + else if (auto as_call = root.try_as()) { + transformed_vertex = as_call.clone(); + *transformed_vertex->begin() = tmp_var; + } + else { + kphp_fail_msg("Internal compiler error: transformation of nullsafe operator fail"); + } + + auto ternary = VertexAdaptor::create(VertexAdaptor::create(cond), + VertexAdaptor::create(), + transformed_vertex).set_location(root); + + return VertexAdaptor::create(assignment, ternary).set_location(root); + }; + + auto is_nullsafe_construction = [](VertexPtr vertex) { + return (vertex->type() == op_instance_prop && vertex.as()->is_null_safe)|| + (vertex->type() == op_func_call && vertex.as()->is_null_safe); + }; + + if (is_nullsafe_construction(root)) { + return transform_nullsufe(root); + } + + if (auto call = root.try_as()) { if (!G->settings().profiler_level.get() && call->size() == 1 && vk::any_of_equal(call->get_string(), "profiler_set_log_suffix", "profiler_set_function_label")) { return VertexAdaptor::create(VertexAdaptor::create(), VertexUtil::embrace(call)).set_location_recursively(root);