Support serialization as another type without casting #12731
Conversation
src/serialize.h
Outdated
There was a problem hiding this comment.
As x is X&&, is std::move necessary?
There was a problem hiding this comment.
x is a variable, and thus can't be a temporary.
The type of the expression x is X. The X&& in the function signature makes that particular instance match cases where it's invoked with a temporary as argument, but once assigned to the variable x that is gone.
src/serialize.h
Outdated
There was a problem hiding this comment.
I think this may be broken in the case where x binds to the temporary result of an implicit conversion, e.g. in deserialization of ServiceFlags nServices above as int64_t, or in a more straightforward case of a char variable being deserialized as an int. In these cases the deserialization will just happen into a temporary variable, and the actual obj argument passed to READWRITEAS won't get updated.
This could be fixed with a serialization wrapper class that can deserialize into a temporary and then save the results on destruction. But a simpler alternative might just be to drop the two rvalue reference overloads of ReadWriteAsHelper so it would be an error to call READWRITEAS anywhere where deserialization would need to happen into a temporary variable. It seems like the only place where this happens is with nServices
This adds a READWRITEAS(type, obj) macro which serializes obj as if it were casted to (const type&) when const, and to (type&) when non-const. This makes it usable in serialization code that uses a single implementation for both serialization and deserializing, which doesn't know the constness of the object involved.
|
@ryanofsky Scary. I've removed the |
|
utACK 818dc74 |
818dc74 Support serialization as another type without casting (Pieter Wuille) Pull request description: This adds a `READWRITEAS(type, obj)` macro which serializes `obj` as if it were converted to `const type&` when `const`, and to `type&` when non-`const`. No actual cast is involved, so this only works when this conversion can be done automatically. This makes it usable in serialization code that uses a single implementation for both serialization and deserializing, which doesn't know the constness of the object involved. This is a redo of #12712, using a slightly different interface. Tree-SHA512: 262f0257284ff99b5ffaec9b997c194e221522ba35c3ac8eaa9bb344449d7ea0a314de254dc77449fa7aaa600f8cd9a24da65aade8c1ec6aa80c6e9a7bba5ca7
…sting 818dc74 Support serialization as another type without casting (Pieter Wuille) Pull request description: This adds a `READWRITEAS(type, obj)` macro which serializes `obj` as if it were converted to `const type&` when `const`, and to `type&` when non-`const`. No actual cast is involved, so this only works when this conversion can be done automatically. This makes it usable in serialization code that uses a single implementation for both serialization and deserializing, which doesn't know the constness of the object involved. This is a redo of bitcoin#12712, using a slightly different interface. Tree-SHA512: 262f0257284ff99b5ffaec9b997c194e221522ba35c3ac8eaa9bb344449d7ea0a314de254dc77449fa7aaa600f8cd9a24da65aade8c1ec6aa80c6e9a7bba5ca7
172fe15 Add missing locks and locking annotations for CAddrMan (practicalswift) 9271ace Support serialization as another type without casting (Pieter Wuille) dc37dd9 Remove unnecessary NONNEGATIVE_SIGNED (Russell Yanofsky) ddd2ab1 Add static_assert to prevent VARINT(<signed value>) (Russell Yanofsky) 539db35 Support deserializing into temporaries (Pieter Wuille) 36db7fd Merge READWRITEMANY into READWRITE (Pieter Wuille) 08ebd5b Remove old pre C++11 functions begin_ptr/end_ptr. (furszy) 9c84665 Prevent integer overflow in ReadVarInt. (Gregory Maxwell) Pull request description: Back ported some pretty concise PRs from upstream over the data serialization area. * bitcoin#9305. --> removal of pre c++11 compatibility functions. * bitcoin#9693. --> prevent integer overflow in `ReadVarInt`. * bitcoin#9753. --> compile error if VARINT is called with a signed value. * bitcoin#12683. --> fixing constness violations * bitcoin#12731. --> support for `READWRITEAS` macro. ACKs for top commit: random-zebra: ACK 172fe15 Fuzzbawls: ACK 172fe15 Tree-SHA512: 1e1e697761b885dcc1aed8a2132bed693b1c76f1f2ed22ae5c074dfb4c353b81d307f71a4c12ed71fc39fd2207c1403881bd699e32b85a167bee57b4f0946130
6 participants
This adds a
READWRITEAS(type, obj)macro which serializesobjas if it were converted toconst type&whenconst, and totype&when non-const. No actual cast is involved, so this only works when this conversion can be done automatically.This makes it usable in serialization code that uses a single implementation for both serialization and deserializing, which doesn't know the constness of the object involved.
This is a redo of #12712, using a slightly different interface.