Fix more constness violations in serialization code #12683
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently, the READWRITE macro cannot be passed any non-const temporaries, as the SerReadWrite function only accepts lvalue references. Deserializing into a temporary is very common, however. See for example things like 's >> VARINT(n)'. The VARINT macro produces a temporary wrapper that holds a reference to n. Fix this by accepting non-const rvalue references instead of lvalue references. We don't propagate the rvalue-ness down, as there are no useful optimizations that only apply to temporaries. Then use this new functionality to get rid of many (but not all) uses of the 'REF' macro (which casts away constness).
|
utACK 172f5fa |
eklitzke
approved these changes
Mar 14, 2018
There was a problem hiding this comment.
utACK 172f5fa
I want to test this later (mostly for my own curiosity), but technically on x86 a variadic function call is not the same as std::forward. The max difference is a loss of one register (that the compiler can probably optimize away anyway).
I have no idea what the context for this is or what you're trying to say. This PRs removes some |
sipa
added a commit
that referenced
this pull request
Mar 15, 2018
172f5fa Support deserializing into temporaries (Pieter Wuille) 2761bca Merge READWRITEMANY into READWRITE (Pieter Wuille) Pull request description: This is another fragment of improvements from #10785. The current serialization code does not support serializing/deserializing from/to temporaries (like `s >> CFlatData(script)`). As a result, there are many invocations of the `REF` macro which in addition to changing the reference type also changes the constness. This is unnecessary in C++11 as we can use rvalue references now instead. The first commit is an extra simplification we can make that removes the duplication of code between `READWRITE` and `READWRITEMANY` (and related functions). Tree-SHA512: babfa9cb268cc3bc39917e4f0a90e4651c33d85032161e16547a07f3b257b7ca7940e0cbfd69f09439d26fafbb1a6cf6359101043407e2c7aeececf7f20b6eed
|
utACK 172f5fa. |
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Dec 16, 2020
172f5fa Support deserializing into temporaries (Pieter Wuille) 2761bca Merge READWRITEMANY into READWRITE (Pieter Wuille) Pull request description: This is another fragment of improvements from bitcoin#10785. The current serialization code does not support serializing/deserializing from/to temporaries (like `s >> CFlatData(script)`). As a result, there are many invocations of the `REF` macro which in addition to changing the reference type also changes the constness. This is unnecessary in C++11 as we can use rvalue references now instead. The first commit is an extra simplification we can make that removes the duplication of code between `READWRITE` and `READWRITEMANY` (and related functions). Tree-SHA512: babfa9cb268cc3bc39917e4f0a90e4651c33d85032161e16547a07f3b257b7ca7940e0cbfd69f09439d26fafbb1a6cf6359101043407e2c7aeececf7f20b6eed
UdjinM6
pushed a commit
to UdjinM6/dash
that referenced
this pull request
Dec 17, 2020
172f5fa Support deserializing into temporaries (Pieter Wuille) 2761bca Merge READWRITEMANY into READWRITE (Pieter Wuille) Pull request description: This is another fragment of improvements from bitcoin#10785. The current serialization code does not support serializing/deserializing from/to temporaries (like `s >> CFlatData(script)`). As a result, there are many invocations of the `REF` macro which in addition to changing the reference type also changes the constness. This is unnecessary in C++11 as we can use rvalue references now instead. The first commit is an extra simplification we can make that removes the duplication of code between `READWRITE` and `READWRITEMANY` (and related functions). Tree-SHA512: babfa9cb268cc3bc39917e4f0a90e4651c33d85032161e16547a07f3b257b7ca7940e0cbfd69f09439d26fafbb1a6cf6359101043407e2c7aeececf7f20b6eed
furszy
added a commit
to PIVX-Project/PIVX
that referenced
this pull request
Apr 8, 2021
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is another fragment of improvements from #10785.
The current serialization code does not support serializing/deserializing from/to temporaries (like
s >> CFlatData(script)). As a result, there are many invocations of theREFmacro which in addition to changing the reference type also changes the constness. This is unnecessary in C++11 as we can use rvalue references now instead.The first commit is an extra simplification we can make that removes the duplication of code between
READWRITEandREADWRITEMANY(and related functions).