[k2] pass const coroutine parameters by const ref where it's safe (#1427)

In certain scenarios, it is safe for coroutines to accept arguments by reference. This commit allows the code generator to produce user-defined coroutines with reference parameters. The rationale behind this decision is that all await points are encapsulated within runtime functions, which are implemented with safety in mind.

This commit also updates several runtime functions to accept arguments by const &. An additional // SAFETY comment has been added to explain why this change is considered safe.

Author: apolyakov

compiler/code-gen/declarations.cpp

@@ -116,8 +116,7 @@ void FunctionParams::declare_cpp_param(CodeGenerator &W, VertexAdaptor<op_var> v
   auto var_ptr = var->var_id;
   if (var->ref_flag) {
     W << "&";
-  } else if (!function->is_interruptible && (var_ptr->marked_as_const || (!function->has_variadic_param && var_ptr->is_read_only))) {
-    // interruptible function must take arguments by value (see C++ avoid reference parameters in coroutines)
+  } else if (!function->is_k2_fork && (var_ptr->marked_as_const || (!function->has_variadic_param && var_ptr->is_read_only))) {
     W << (!type.type->is_primitive_type() ? "const &" : "");
   }
   W << VarName(var_ptr);

compiler/code-gen/vertex-compiler.cpp

@@ -920,7 +920,6 @@ void compile_func_call(VertexAdaptor<op_func_call> root, CodeGenerator &W, func_
       W << "SAVE_BUILTIN_CALL_STATS(\"" << func->name << "\", (";
     }
 
-
     if (mode == func_call_mode::fork_call) {
       if (func->is_interruptible) {
         W << "(kphp::forks::start(" << FunctionName(func);

compiler/data/function-data.h

@@ -118,6 +118,7 @@ class FunctionData {
   bool cpp_variadic_call = false;
   bool is_resumable = false;
   bool is_interruptible = false;
+  bool is_k2_fork = false;
   bool is_stub = false;
   bool need_generated_stub = false;
   bool can_be_implicitly_interrupted_by_other_resumable = false;

compiler/pipes/calc-bad-vars.cpp

@@ -6,10 +6,12 @@
 
 #include <algorithm>
 #include <queue>
+#include <utility>
 #include <vector>
 
 #include "compiler/compiler-core.h"
 #include "compiler/data/class-data.h"
+#include "compiler/data/data_ptr.h"
 #include "compiler/data/src-file.h"
 #include "compiler/function-pass.h"
 #include "compiler/kphp_assert.h"
@@ -558,10 +560,16 @@ class CalcBadVars {
     }
   }
 
-  static void calc_k2_fork(const FuncCallGraph &call_graph, const std::vector<DepData> &dep_data) {
+  static void calc_k2_fork(const FuncCallGraph& call_graph, const std::vector<DepData>& dep_data) {
     for (int i = 0; i < call_graph.n; ++i) {
-      for (const auto &fork : dep_data[i].forks) {
+      for (const auto& fork : dep_data[i].forks) {
         fork->is_interruptible = true;
+        if (!std::exchange(fork->is_k2_fork, true)) { // check only once
+          for (VarPtr param : fork->param_ids) {
+            kphp_error(!param->is_reference, fmt_format("Function '{}' cannot be forked since it has a reference parameter '{}'\n", fork->as_human_readable(),
+                                                        param->as_human_readable()));
+          }
+        }
       }
     }
   }

runtime-light/stdlib/array/array-functions.h

@@ -209,7 +209,7 @@ template<class T, std::invocable<T> F>
 requires convertible_to_php_bool<kphp::coro::async_function_return_type_t<F, T>>
 kphp::coro::task<array<T>> array_filter_impl(array<T> a, F f) noexcept {
   array<T> result{a.size()};
-  for (const auto& it : a) {
+  for (const auto& it : std::as_const(a)) {
     bool condition{};
     if constexpr (kphp::coro::is_async_function_v<F, T>) {
       condition = f$boolval(co_await std::invoke(f, it.get_value()));
@@ -227,7 +227,7 @@ template<class T, std::invocable<typename array<T>::const_iterator::key_type> F>
 requires convertible_to_php_bool<kphp::coro::async_function_return_type_t<F, typename array<T>::const_iterator::key_type>>
 kphp::coro::task<array<T>> array_filter_by_key_impl(array<T> a, F f) noexcept {
   array<T> result{a.size()};
-  for (const auto& it : a) {
+  for (const auto& it : std::as_const(a)) {
     bool condition{};
     if constexpr (kphp::coro::is_async_function_v<F, typename array<T>::const_iterator::key_type>) {
       condition = f$boolval(co_await std::invoke(f, it.get_key()));
@@ -245,7 +245,7 @@ template<class T, class F>
 requires convertible_to_php_bool<kphp::coro::async_function_return_type_t<F, typename array<T>::const_iterator::value_type>>
 kphp::coro::task<std::tuple<typename array<T>::const_iterator::key_type, typename array<T>::const_iterator::value_type>> array_find_impl(array<T> a,
                                                                                                                                          F f) noexcept {
-  for (const auto& it : a) {
+  for (const auto& it : std::as_const(a)) {
     bool condition{};
     if constexpr (kphp::coro::is_async_function_v<F, typename array<T>::const_iterator::value_type>) {
       condition = co_await std::invoke(f, it.get_value());
@@ -339,7 +339,7 @@ mixed f$array_rand(const array<T>& a, int64_t num) noexcept {
 template<class A, std::invocable<A> F, class R = kphp::coro::async_function_return_type_t<F, A>>
 kphp::coro::task<array<R>> f$array_map(F f, array<A> a) noexcept {
   array<R> result{a.size()};
-  for (const auto& it : a) {
+  for (const auto& it : std::as_const(a)) {
     if constexpr (kphp::coro::is_async_function_v<F, A>) {
       result.set_value(it.get_key(), co_await std::invoke(f, it.get_value()));
     } else {
@@ -353,7 +353,7 @@ template<class R, class T, std::invocable<R, T> F, class I>
 requires std::constructible_from<R, std::add_rvalue_reference_t<I>>
 kphp::coro::task<R> f$array_reduce(array<T> a, F f, I init) noexcept {
   R result{R(std::move(init))}; // explicit constructor call to avoid implicit cast
-  for (const auto& it : a) {
+  for (const auto& it : std::as_const(a)) {
     if constexpr (kphp::coro::is_async_function_v<F, R, T>) {
       result = co_await std::invoke(f, result, it.get_value());
     } else {
@@ -365,12 +365,9 @@ kphp::coro::task<R> f$array_reduce(array<T> a, F f, I init) noexcept {
 
 template<class T, class Comparator>
 requires(std::invocable<Comparator, T, T>)
-kphp::coro::task<> f$usort(array<T>& a, Comparator compare) {
+kphp::coro::task<> f$usort(array<T>& a, Comparator compare) noexcept {
   if constexpr (kphp::coro::is_async_function_v<Comparator, T, T>) {
-    /* ATTENTION: temporary copy is necessary since functions is coroutine and sort is inplace */
-    array<T> tmp{a};
-    co_await array_functions_impl_::async_sort<kphp::coro::task<>>(tmp, std::move(compare), true);
-    a = tmp;
+    co_await array_functions_impl_::async_sort<kphp::coro::task<>>(a, std::move(compare), true);
     co_return;
   } else {
     co_return a.sort(std::move(compare), true);
@@ -379,25 +376,19 @@ kphp::coro::task<> f$usort(array<T>& a, Comparator compare) {
 
 template<class T, class Comparator>
 requires(std::invocable<Comparator, T, T>)
-kphp::coro::task<> f$uasort(array<T>& a, Comparator compare) {
+kphp::coro::task<> f$uasort(array<T>& a, Comparator compare) noexcept {
   if constexpr (kphp::coro::is_async_function_v<Comparator, T, T>) {
-    /* ATTENTION: temporary copy is necessary since functions is coroutine and sort is inplace */
-    array<T> tmp{a};
-    co_await array_functions_impl_::async_sort<kphp::coro::task<>>(tmp, std::move(compare), false);
-    a = tmp;
+    co_await array_functions_impl_::async_sort<kphp::coro::task<>>(a, std::move(compare), false);
   } else {
     co_return a.sort(std::move(compare), false);
   }
 }
 
 template<class T, class Comparator>
 requires(std::invocable<Comparator, typename array<T>::key_type, typename array<T>::key_type>)
-kphp::coro::task<> f$uksort(array<T>& a, Comparator compare) {
+kphp::coro::task<> f$uksort(array<T>& a, Comparator compare) noexcept {
   if constexpr (kphp::coro::is_async_function_v<Comparator, typename array<T>::key_type, typename array<T>::key_type>) {
-    /* ATTENTION: temporary copy is necessary since functions is coroutine and sort is inplace */
-    array<T> tmp{a};
-    co_await array_functions_impl_::async_ksort<kphp::coro::task<>>(tmp, std::move(compare));
-    a = tmp;
+    co_await array_functions_impl_::async_ksort<kphp::coro::task<>>(a, std::move(compare));
   } else {
     co_return a.ksort(std::move(compare));
   }

runtime-light/stdlib/confdata/confdata-functions.cpp

@@ -44,7 +44,7 @@ mixed extract_confdata_value(const tl::confdataValue& confdata_value) noexcept {
 
 } // namespace
 
-kphp::coro::task<mixed> f$confdata_get_value(string key) noexcept {
+kphp::coro::task<mixed> f$confdata_get_value(const string& key) noexcept {
   tl::ConfdataGet confdata_get{.key = {.value = {key.c_str(), key.size()}}};
   tl::storer tls{confdata_get.footprint()};
   confdata_get.store(tls);
@@ -70,7 +70,7 @@ kphp::coro::task<mixed> f$confdata_get_value(string key) noexcept {
   co_return extract_confdata_value(*maybe_confdata_value.opt_value); // the key exists
 }
 
-kphp::coro::task<array<mixed>> f$confdata_get_values_by_any_wildcard(string wildcard) noexcept {
+kphp::coro::task<array<mixed>> f$confdata_get_values_by_any_wildcard(const string& wildcard) noexcept {
   static constexpr size_t CONFDATA_GET_WILDCARD_STREAM_CAPACITY = 1 << 20;
 
   tl::ConfdataGetWildcard confdata_get_wildcard{.wildcard = {.value = {wildcard.c_str(), wildcard.size()}}};

runtime-light/stdlib/confdata/confdata-functions.h

@@ -14,11 +14,14 @@ inline bool f$is_confdata_loaded() noexcept {
   return k2::access(kphp::confdata::COMPONENT_NAME) == k2::errno_ok;
 }
 
-kphp::coro::task<mixed> f$confdata_get_value(string key) noexcept;
+// SAFETY: `key` is only used before await point
+kphp::coro::task<mixed> f$confdata_get_value(const string& key) noexcept;
 
-kphp::coro::task<array<mixed>> f$confdata_get_values_by_any_wildcard(string wildcard) noexcept;
+// SAFETY: `wildcard` is only used before await point
+kphp::coro::task<array<mixed>> f$confdata_get_values_by_any_wildcard(const string& wildcard) noexcept;
 
-inline kphp::coro::task<array<mixed>> f$confdata_get_values_by_predefined_wildcard(string wildcard) noexcept {
+// SAFETY: `f$confdata_get_values_by_any_wildcard` is coro-pass-by-ref safe
+inline kphp::coro::task<array<mixed>> f$confdata_get_values_by_predefined_wildcard(const string& wildcard) noexcept {
   kphp::log::info("k2-confdata doesn't support predefined wildcard optimization. wildcard: {}", wildcard.c_str());
-  co_return co_await f$confdata_get_values_by_any_wildcard(std::move(wildcard));
+  co_return co_await f$confdata_get_values_by_any_wildcard(wildcard);
 }