- changed status to resolved
Clarify MoveConstructibility requirements on Serializable types
Discussion copied from implementation PR 251.
@Dan Bonachea wrote:
IIUC this PR actually enables this program to work even if A is non-movable/non-copyable.
@Amir Kamil wrote:
I’m afraid that’s not the case. Deserializing a bound function requires both the function object and the arguments to be movable. The function object is deserialized into temporary storage, and the arguments are deserialized as a tuple. The latter in turn deserializes each argument into temporary storage before moving them into the result tuple. The deserialized function object and argument tuple are then moved into the deserialized bound function.
Changing this would likely be quite ugly. We wouldn’t be able to use a normal tuple to hold the arguments, otherwise we’d run into the same UB discussed in PR #240. We’d need something like a tuple where each element is a union/variant holding either raw storage or a real argument. We’d then be able to construct the tuple and then deserialize into each element. Changes would probably be required elsewhere too (e.g.
command.hpp
).I move (pun intended) that we keep the implementation as is and require MoveConstructibility in RPC and friends.
@Dan Bonachea wrote:
Hmm ok. Thanks for looking into that. I agree that sounds orthogonal to the goal of this PR.
FWIW I don't think we actually specify that RPC arguments need to be MoveConstructible, but it sounds like we probably should.
@Amir Kamil wrote:
I think the issue is much more expansive than RPC. We need
deserialized_type_t<T>
to be MoveConstructible to deserialize aT
, not just in RPC, but also in deserializing astd::pair
orstd::tuple
that contains aT
, and likely other places as well.The safest solution would probably be to require MoveConstructibility for
T
for any of the serialization mechanisms forT
and fordeserialized_type_t<T>
when it is not the same asT
.We should fork off this discussion into a spec issue.
Comments (2)
-
reporter -
reporter Merged in akamil/upcxx-spec/issue166 (pull request #55)
Clarify MoveConstructible requirement on serialization. Fixes
#166.Approved-by: Dan Bonachea dobonachea@lbl.gov
→ <<cset b40c72c80e92>>
- Log in to comment
Clarify MoveConstructible requirement on serialization. Fixes
#166.→ <<cset d567a3da279f>>