Semantics of `read_into()` and `read_sequence_into()` with respect to destruction

In custom serialization, read_into<T>() and read_sequence_into<T>() expect uninitialized storage as input, and they do not destruct any existing objects in that storage. However, this is not explicit in the specification, which just states the storage “must point to a location with appropriate space and alignment for the object(s)”. We should clarify that destructors are not invoked.

Furthermore, we should consider whether these semantics match how users are likely to use them. I suspect that the common case is reading into a member variable of an already constructed outer object, in which case the member has also been already constructed. This is problematic if the type is not TriviallyDestructible. I scanned the upcxx source and test cases for this pattern. Most involved TriviallyDestructible types and so are not problematic, but I did find an egregious violation in test/serialization.cpp:

struct array_write_read_into {
  int arr1[4];
  std::string arr2[2];

  struct upcxx_serialization {
    template<typename Writer>
    static void serialize(Writer &w, array_write_read_into const &x) {
      w.write(x.arr1);
      w.write(x.arr2);
    }

    template<typename Reader>
    static array_write_read_into* deserialize(Reader &r, void *spot) {
      auto result = new(spot) array_write_read_into;
      r.template read_into<int[4]>(result->arr1);   // OK - int is trivial
      r.template read_into<std::string[2]>(result->arr2);   // ERROR - string is not TriviallyDestructible
      return result;
    }
  };
};

One more in test/neg/neg-serial12.cpp:

struct A { // non-Serializable
  ~A() {}
};

struct B {
  A a;
  struct upcxx_serialization {
    template<typename Writer>
    static void serialize(Writer &w, const B &x) {
    }
    template<typename Reader>
    static B* deserialize(Reader &r, void *spot) {
      B* result = new(spot) B;
      r.template read_into<A>(&result->a);    // ERROR
      return result;
    }
  };
};

The actual call is erroneous because A is not Serializable, but if A were Serializable, the call would still be problematic, and I don’t think that was the intent of this test case.

Perhaps we should provide overloads of read_into<T>() and read_sequence_into<T>() that do destruction? Maybe if a T* is provided, the corresponding overload does destruction, but if a void* is provided, it does not? We’d need to carefully explain this in the spec.

Comments (3)