Change Packer and Unpacker to support any byte type (#3) #5
Conversation
…converting iterator
|
I've put some work into the branch to make it buildable again. It would be nice if we find a way to make: auto [int8] = msgpack23::unpack<std::byte, UInt8Struct>(data);look like: auto [int8] = msgpack23::unpack<UInt8Struct>(data); |
…ted object packing tests
|
@igromanru do you have an idea about my last comment? If not you can set the branch to Ready for review then I can merge it. |
|
I'll look at it later. |
|
No idea. I'm not a C++ templating Wizard. |
There was a problem hiding this comment.
Pull Request Overview
This PR extends the Packer and Unpacker classes to support any byte type (not just std::byte) through C++20 concepts and deduction guides, enabling more flexible usage with containers of char, unsigned char, std::uint8_t, std::int8_t, and std::byte.
- Introduced a
byte_typeconcept to constrain supported byte types - Added deduction guides for
PackerandUnpackerto automatically infer the byte type from constructor arguments - Refactored pack/unpack functions to support generic byte type containers
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| include/msgpack23/msgpack23.h | Core implementation: added byte_type concept, templatized Packer and Unpacker on byte type, added deduction guides, and updated all byte operations to use generic type parameter |
| tests/byte_type_tests.cpp | New comprehensive test file covering packing/unpacking with unsigned char, std::uint8_t, char, and std::int8_t |
| tests/CMakeLists.txt | Added new byte_type_tests.cpp to the test build |
| tests/object_packing_tests.cpp | Minor const correctness improvement for unpacked object |
| tests/exception_tests.cpp | Updated to explicitly specify template parameter for default-constructed Unpacker instances |
| examples/first_example.cpp | Simplified by removing custom converting_insert_iterator, now using deduction guides directly |
| examples/second_example.cpp | Simplified by removing custom converting_insert_iterator, now using deduction guides directly |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… better maintainability
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…e_t for improved clarity and consistency
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2 participants
Here is a possible implementation of what I was talking about in #3.
To allow
PackerandUnpackerto useconcept ByteTypewithout explicitly specify the type, you have to add a "deduction guide" or multiple, if needed.