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.
I'm actively working on a RESP 3 implementation for fun and figured I'd share it for comment before I get too deep into it. I am trying to maintain an interface as close as possible to your RESP 2 implementation, although I've had to make a few compromises to make the recursiveness work without use of boost::variant (although from what I've read, boost variant would have involved more heap allocations than is strictly necessary).
I had a couple specific concerns I want to bring up:
sizeof(std::map<...>)
etc doesn't change based on the data type, but it still feels a bit hacky to do this. Are there any better ideas that don't involve an extra heap allocation? Or do you think that extra heap allocation is fine?double_float
if you have better suggestions I am 100% willing to consider them.`
and|
on the RESP documentation page at different locations, and I can't seem to do a full search of the redis interface documentation to find a message that is responded to with data that has attributes, do you know a way I might be able to confirm which is actually used?get_marker
functions I did not realize how close it was in name toget_mark
. But I can't quite think of a name that is sufficiently different.I am not quite ready to call this ready for merge, I'd like to add resp3 to serialization side and get some tests written for both. But any and all comments would be appreciated.