Call clearTerseFields during the Thrift object construction

Summary: Call the method after the object is constructed using `withDefaultValues` before deserialization. This should populate terse fields with intrinsic defaults.

Reviewed By: rmakheja, thedavekwon

Differential Revision: D39779291

fbshipit-source-id: cd92a614c1b47fb757ddaa59371e754199226dda

Author: Andrii Korotkov

hphp/runtime/ext/thrift/binary.cpp

@@ -440,6 +440,7 @@ Object binary_deserialize_struct(const String& clsName,
   SpecHolder specHolder;
   auto const& spec = specHolder.getSpec(cls);
   Object dest = spec.newObject(cls);
+  spec.clearTerseFields(cls, dest);
 
   auto const& fields = spec.fields;
   const size_t numFields = fields.size();

hphp/runtime/ext/thrift/compact.cpp

@@ -704,6 +704,7 @@ struct CompactReader {
       SpecHolder specHolder;
       auto const& spec = specHolder.getSpec(cls);
       Object dest = spec.newObject(cls);
+      spec.clearTerseFields(cls, dest);
 
       readStructBegin();
       int16_t fieldNum;

hphp/runtime/ext/thrift/spec-holder.cpp

@@ -147,6 +147,25 @@ const Func* lookupWithDefaultValuesFunc(const Class* cls) {
   return func;
 }
 
+const StaticString s_clearTerseFields("clearTerseFields");
+
+const Func* lookupClearTerseFieldsFunc(const Class* cls) {
+  auto const func = cls->lookupMethod(s_clearTerseFields.get());
+  if (func == nullptr) {
+    thrift_error(
+      folly::sformat("Method {}::clearTerseFields() not found", cls->name()),
+      ERR_INVALID_DATA
+    );
+  }
+  if (func->isStatic()) {
+    thrift_error(
+      folly::sformat("Method {}::clearTerseFields() is static", cls->name()),
+      ERR_INVALID_DATA
+    );
+  }
+  return func;
+}
+
 StructSpec compileSpec(const Array& spec, const Class* cls) {
   // A union field also writes to a property named __type. If one exists, we
   // need to also verify that it accepts integer values. We only need to do
@@ -197,10 +216,13 @@ StructSpec compileSpec(const Array& spec, const Class* cls) {
 
   // We can precompute the cls::withDefaultValues() func pointer only if the
   // underlying class cannot change.
-  auto const func = cls != nullptr && cls->isPersistent()
+  auto const withDefaultValuesFunc = cls != nullptr && cls->isPersistent()
     ? lookupWithDefaultValuesFunc(cls) : nullptr;
 
-  return StructSpec{HPHP::FixedVector(std::move(temp)), func};
+  auto const clearTerseFieldsFunc = cls != nullptr && cls->isPersistent()
+    ? lookupClearTerseFieldsFunc(cls) : nullptr;
+
+  return StructSpec{HPHP::FixedVector(std::move(temp)), withDefaultValuesFunc, clearTerseFieldsFunc};
 }
 
 } // namespace
@@ -303,7 +325,9 @@ Object StructSpec::newObject(Class* cls) const {
 
   auto obj = g_context->invokeFuncFew(
     func, cls, 0, nullptr, RuntimeCoeffects::pure(), false /* dynamic */);
-  if (tvIsObject(obj)) return Object::attach(obj.m_data.pobj);
+  if (tvIsObject(obj)) {
+    return Object::attach(obj.m_data.pobj);
+  }
 
   SCOPE_EXIT { tvDecRefGen(obj); };
   thrift_error(
@@ -315,4 +339,11 @@ Object StructSpec::newObject(Class* cls) const {
   );
 }
 
+void StructSpec::clearTerseFields(Class* cls, const Object& obj) const {
+  auto const func = clearTerseFieldsFunc != nullptr
+    ? clearTerseFieldsFunc : lookupClearTerseFieldsFunc(cls);
+  g_context->invokeFuncFew(
+    func, obj.get(), 0, nullptr, RuntimeCoeffects::write_props());
+}
+
 }

hphp/runtime/ext/thrift/spec-holder.h

@@ -62,8 +62,10 @@ struct FieldSpec {
 struct StructSpec {
   FixedVector<FieldSpec> fields;
   const Func* withDefaultValuesFunc;
+  const Func* clearTerseFieldsFunc;
 
   Object newObject(Class* cls) const;
+  void clearTerseFields(Class* cls, const Object& obj) const;
 };
 
 // Provides safe access to specifications.

hphp/runtime/vm/coeffects.cpp

@@ -62,7 +62,8 @@ RuntimeCoeffects RuntimeCoeffects::globals_leak_safe() {
   X(zoned_with)        \
   X(zoned)             \
   X(leak_safe_shallow) \
-  X(write_this_props)
+  X(write_this_props)  \
+  X(write_props)
 
 #define X(x)                                                             \
 RuntimeCoeffects RuntimeCoeffects::x() {                                 \

hphp/runtime/vm/coeffects.h

@@ -44,6 +44,7 @@ struct RuntimeCoeffects {
   static RuntimeCoeffects zoned_with();
   static RuntimeCoeffects zoned();
   static RuntimeCoeffects write_this_props();
+  static RuntimeCoeffects write_props();
   static RuntimeCoeffects globals_leak_safe();
   static RuntimeCoeffects leak_safe_shallow();
 

hphp/test/slow/class-ptr/lazyclass-suppress.inc

@@ -40,27 +40,31 @@ class ThriftStruct {
   public static function withDefaultValues()[]: this {
     return new static("");
   }
+
+  public function clearTerseFields()[write_props]: void {}
 }
-    
+
 class NestedStruct {
   public string $str;
-  
+
   public function __construct(string $str) {
     $this->str = $str;
   }
-  
+
   public function withDefaultValues()[]: this {
     return new static("");
   }
+
+  public function clearTerseFields()[write_props]: void {}
 };
 
 class NestedStructAdapter {
   const type THackType = NestedStruct;
-   
+
   public static function toThrift(NestedStruct $hack_value): ThriftStruct {
     return new ThriftStruct(HH\Lib\Str\reverse($hack_value->str));
   }
-  
+
   public static function fromThrift(ThriftStruct $thrift_struct): NestedStruct {
     return new NestedStruct(HH\Lib\Str\reverse($thrift_struct->str));
   }
@@ -77,10 +81,12 @@ class Foo {
   ];
 
   public ?NestedStructAdapter::THackType $nested = null;
-  
+
   public static function withDefaultValues()[]: this {
     return new static();
   }
+
+  public function clearTerseFields()[write_props]: void {}
 };
 
 class DummyProtocol {

hphp/test/slow/hack_arr_compat/thrift_list.php

@@ -62,6 +62,8 @@ class Listish {
   public static function withDefaultValues()[]: this {
     return new static();
   }
+
+  public function clearTerseFields()[write_props]: void {}
 }
 
 function test() {

hphp/test/slow/hack_arr_compat/thrift_map.php

@@ -66,6 +66,8 @@ class Mappish {
   public static function withDefaultValues()[]: this {
     return new static();
   }
+
+  public function clearTerseFields()[write_props]: void {}
 }
 
 class TestStruct {
@@ -82,6 +84,8 @@ class TestStruct {
   public static function withDefaultValues()[]: this {
     return new static();
   }
+
+  public function clearTerseFields()[write_props]: void {}
 }
 
 function test() {

hphp/test/slow/hack_arr_compat/thrift_map_2.php

@@ -66,6 +66,8 @@ class Mappish {
   public static function withDefaultValues()[]: this {
     return new static();
   }
+
+  public function clearTerseFields()[write_props]: void {}
 }
 
 class Settish {
@@ -84,6 +86,8 @@ class Settish {
   public static function withDefaultValues()[]: this {
     return new static();
   }
+
+  public function clearTerseFields()[write_props]: void {}
 }
 
 class TestStruct {
@@ -100,6 +104,8 @@ class TestStruct {
   public static function withDefaultValues()[]: this {
     return new static();
   }
+
+  public function clearTerseFields()[write_props]: void {}
 }
 
 function test() {