Serialize inheritance (#1067)

Allow serialize of inherited classes.

compiler/code-gen/declarations.cpp

@@ -1147,16 +1147,29 @@ void ClassMembersDefinition::compile_msgpack_serialize(CodeGenerator &W, ClassPt
   //   packer.pack(field_1);
   //   ...
   //}
+
   std::vector<std::string> body;
   uint16_t cnt_fields = 0;
 
-  klass->members.for_each([&](ClassMemberInstanceField &field) {
-    if (field.serialization_tag != -1) {
-      auto func_name = fmt_format("vk::msgpack::packer_float32_decorator::pack_value{}", field.serialize_as_float32 ? "_float32" : "");
-      body.emplace_back(fmt_format("packer.pack({}); {}(packer, ${});", field.serialization_tag, func_name, field.var->name));
-      cnt_fields += 2;
-    }
-  });
+  std::vector<ClassPtr> klasses;
+  ClassPtr the_klass = klass;
+
+  while (the_klass) {
+    klasses.push_back(the_klass);
+    the_klass = the_klass->parent_class;
+  }
+
+  std::reverse(klasses.begin(), klasses.end());
+
+  for (auto &k : klasses) {
+    k->members.for_each([&](ClassMemberInstanceField &field) {
+      if (field.serialization_tag != -1) {
+        auto func_name = fmt_format("vk::msgpack::packer_float32_decorator::pack_value{}", field.serialize_as_float32 ? "_float32" : "");
+        body.emplace_back(fmt_format("packer.pack({}); {}(packer, ${});", field.serialization_tag, func_name, field.var->name));
+        cnt_fields += 2;
+      }
+    });
+  }
 
   FunctionSignatureGenerator(W).set_const_this()
     << "void " << klass->src_name << "::msgpack_pack(vk::msgpack::packer<string_buffer> &packer)" << BEGIN
@@ -1184,12 +1197,25 @@ void ClassMembersDefinition::compile_msgpack_deserialize(CodeGenerator &W, Class
   //
 
   std::vector<std::string> cases;
-  klass->members.for_each([&](ClassMemberInstanceField &field) {
-    if (field.serialization_tag != -1) {
-      cases.emplace_back(fmt_format("case {}: elem.convert(${}); break;", field.serialization_tag, field.var->name));
-    }
-  });
+  std::vector<ClassPtr> klasses;
+
+  ClassPtr the_klass = klass;
+
+  // Put class' fields along with all parent's fields.
+  while (the_klass) {
+    klasses.push_back(the_klass);
+    the_klass = the_klass->parent_class;
+  }
 
+  std::reverse(klasses.begin(), klasses.end());
+
+  for (auto &k : klasses) {
+    k->members.for_each([&](ClassMemberInstanceField &field) {
+      if (field.serialization_tag != -1) {
+        cases.emplace_back(fmt_format("case {}: elem.convert(${}); break;", field.serialization_tag, field.var->name));
+      }
+    });
+  }
   cases.emplace_back("default: break;");
 
   W << "void " << klass->src_name << "::msgpack_unpack(const vk::msgpack::object &msgpack_o) " << BEGIN

compiler/pipes/check-classes.cpp

@@ -48,10 +48,6 @@ inline void CheckClassesPass::analyze_class(ClassPtr klass) {
                fmt_format("class {} can be autoloaded, but its file contains some logic (maybe, require_once files with global vars?)\n",
                           klass->as_human_readable()));
   }
-  if (klass->is_serializable) {
-    kphp_error(!klass->parent_class || !klass->parent_class->members.has_any_instance_var(),
-               "You may not serialize classes which has a parent with fields");
-  }
 }
 
 /*
@@ -90,7 +86,6 @@ void CheckClassesPass::check_serialized_fields(ClassPtr klass) {
       return;
     }
 
-    kphp_error_return(klass->is_serializable, fmt_format("you may not use @kphp-serialized-field inside non-serializable klass: {}", klass->name));
     if (kphp_serialized_field_tag->value.starts_with("none")) {
       return;
     }

compiler/pipes/final-check.cpp

@@ -16,6 +16,7 @@
 #include "compiler/data/var-data.h"
 #include "compiler/vertex-util.h"
 #include "compiler/type-hint.h"
+#include "compiler/phpdoc.h"
 
 namespace {
 void check_class_immutableness(ClassPtr klass) {
@@ -570,6 +571,7 @@ void FinalCheckPass::on_start() {
 
   if (current_function->type == FunctionData::func_class_holder) {
     check_class_immutableness(current_function->class_id);
+    check_serialized_fields_hierarchy(current_function->class_id);
     process_job_worker_class(current_function->class_id);
   }
 
@@ -1063,3 +1065,40 @@ void FinalCheckPass::check_magic_clone_method(FunctionPtr fun) {
   kphp_error(!fun->is_resumable, fmt_format("{} method has to be not resumable", fun->as_human_readable()));
   kphp_error(!fun->can_throw(), fmt_format("{} method should not throw exception", fun->as_human_readable()));
 }
+
+
+void FinalCheckPass::check_serialized_fields_hierarchy(ClassPtr klass) {
+  auto the_klass = klass;
+  // This loop finishes unconditionally since there is NULL klass->parent_class if there is no base class.
+  while (the_klass) {
+
+    // Inheritance with serialization is allowed if
+    // * parent class has ho instance field
+    // * if there are instance fields, class should be marked with kphp-serializable
+    if (klass->is_serializable) {
+      kphp_error_return(
+        (the_klass->members.has_any_instance_var() && the_klass->is_serializable) || !the_klass->members.has_any_instance_var(),
+        fmt_format("Class {} and all its ancestors must be @kphp-serializable if there are instance fields. Class {} is not.", klass->name, the_klass->name));
+
+    }
+    the_klass = the_klass->parent_class;
+  }
+
+  klass->members.for_each([&](ClassMemberInstanceField &f) {
+    // Check if there is a field with the same number available above in hierarchy
+    auto f_tag = f.serialization_tag;
+    the_klass = klass->parent_class;
+
+    while (the_klass) {
+      auto same_numbered_field = the_klass->members.find_member([&f_tag](const ClassMemberInstanceField &f) {
+        return (f.serialization_tag == f_tag) && (f_tag != -1);
+      });
+
+      if (same_numbered_field) {
+        kphp_error_return(false, fmt_format("kphp-serialized-field: field with number {} found in both classes {} and {}", f_tag, the_klass->name, klass->name));
+      }
+
+      the_klass = the_klass->parent_class;
+    }
+  });
+}

compiler/pipes/final-check.h

@@ -44,4 +44,5 @@ class FinalCheckPass final : public FunctionPassBase {
   static void check_instanceof(VertexAdaptor<op_instanceof> instanceof_vertex);
   static void check_indexing(VertexPtr array, VertexPtr key) noexcept;
   static void check_array_literal(VertexAdaptor<op_array> vertex) noexcept;
+  static void check_serialized_fields_hierarchy(ClassPtr klass);
 };

docs/kphp-language/howto-by-kphp/serialization-msgpack.md

@@ -105,7 +105,13 @@ After having deleted the field, you must specify its index in `@kphp-reserved-fi
 * Deleting is ok, but don't forget about *@kphp-reserved-fields*
 * **Types of fields can't be changed!** Old binary data would not map to the new structure
 ```
+## Inheritance
+It is allowed to inherit classes with `@kphp-serializable` annotation, but there are restrictions:
 
+```tip
+* Class with instance field must be marked with `@kphp-serializable`
+* Within annotation `@kphp-serialized-field {index}` index must be unique across whole inheritance hierarchy.
+```
 
 ## Serializing floats
 

tests/phpt/msgpack_serialize/026_serialize_with_base.php

@@ -0,0 +1,30 @@
+@ok k2_skip
+<?php
+require_once 'kphp_tester_include.php';
+
+/** @kphp-serializable */
+class Base {
+    /**
+     * @kphp-serialized-field 1
+     * @var int
+     */
+    public $b = 10;
+}
+
+/** @kphp-serializable */
+class A extends Base {
+    /**
+     * @kphp-serialized-field 2
+     * @var int
+     */
+    public $x = 10;
+
+    /**
+     * @kphp-serialized-field 3
+     * @var int
+     */
+    public $y = 10;
+}
+
+$a = new A();
+instance_serialize($a);

tests/phpt/msgpack_serialize/027_serialize_intermediate.php

@@ -0,0 +1,34 @@
+@ok k2_skip
+<?php
+require_once 'kphp_tester_include.php';
+
+class Base {
+    public function foo() { var_dump("OK"); }
+}
+
+/** @kphp-serializable */
+class A extends Base {
+    /**
+     * @kphp-serialized-field 1
+     * @var int
+     */
+    public $x = 10;
+}
+
+class B extends A {
+}
+
+/** @kphp-serializable */
+class C extends B {
+    /**
+     * @kphp-serialized-field 2
+     * @var int
+     */
+    public $y = 12;
+}
+
+$c = new C();
+$str = instance_serialize($c);
+$c->foo();
+$c_new = instance_deserialize($str, C::class);
+$c_new->foo();

tests/phpt/msgpack_serialize/028_deserialize_inherited.php

@@ -0,0 +1,50 @@
+@ok k2_skip
+<?php
+require_once 'kphp_tester_include.php';
+
+class Base {
+    public function foo() { var_dump("OK"); }
+}
+
+/** @kphp-serializable */
+class A extends Base {
+    /**
+     * @kphp-serialized-field 1
+     * @var int
+     */
+    public $x = 10;
+}
+
+/** @kphp-serializable */
+class B extends A {
+    /**
+     * @kphp-serialized-field 2
+     * @var int
+     */
+    public $y = 10;
+}
+
+/** @kphp-serializable */
+class C extends B {
+    /**
+     * @kphp-serialized-field 3
+     * @var int
+     */
+    public $z = 12;
+}
+
+$c0 = new C();
+$str = instance_serialize($c0);
+$c0->foo();
+
+$c = instance_deserialize($str, C::class);
+$b = instance_deserialize($str, B::class);
+$a = instance_deserialize($str, A::class);
+
+var_dump(instance_to_array($a));
+var_dump(instance_to_array($b));
+var_dump(instance_to_array($c));
+
+$a->foo();
+$b->foo();
+$c->foo();

tests/phpt/msgpack_serialize/103_serialize_with_base.php

@@ -1,5 +1,5 @@
 @kphp_should_fail k2_skip
-/You may not serialize classes which has a parent with fields/
+/Class A and all its ancestors must be @kphp-serializable if there are instance fields. Class Base is not./
 <?php
 
 require_once 'kphp_tester_include.php';

tests/phpt/msgpack_serialize/113_duplicated_inherited_tags.php

@@ -0,0 +1,27 @@
+@kphp_should_fail k2_skip
+/kphp-serialized-field: field with number 1 found in both classes A and B/
+<?php
+
+require_once 'kphp_tester_include.php';
+
+/** @kphp-serializable */
+class A {
+    /**
+     * @kphp-serialized-field 1
+     * @var int
+     */
+    public $x = 10;
+
+}
+
+/** @kphp-serializable */
+class B extends A {
+    /**
+     * @kphp-serialized-field 1
+     * @var int
+     */
+    public $y = 10;
+}
+
+$b = new B();
+instance_serialize($b);