Specialize char_traits for std::byte to fix from_msgpack (fixes #4756)#4760
Merged
Conversation
nlohmann
requested changes
Apr 26, 2025
🔴 Amalgamation check failed! 🔴The source code has not been amalgamated. @XueSongTap |
Contributor
Author
|
Hi, @nlohmann This PR adds a missing comment for #include (to clarify that it is needed for std::byte) and improves the English comments in the unit-msgpack.cpp tests for better clarity. Could you please help take another look to make sure the added comments look good? Thanks! |
🔴 Amalgamation check failed! 🔴The source code has not been amalgamated. @XueSongTap |
nlohmann
requested changes
Apr 27, 2025
Owner
|
…ann#4756) Provide a char_traits<std::byte> specialization under __cpp_lib_byte to allow parsing MessagePack data from containers of std::byte. Signed-off-by: xuesongtap <tap91624@gmail.com> Signed-off-by: yexiaochuan <tap91624@gmail.com>
Signed-off-by: xuesongtap <tap91624@gmail.com> Signed-off-by: yexiaochuan <tap91624@gmail.com>
Signed-off-by: yexiaochuan <tap91624@gmail.com>
Signed-off-by: yexiaochuan <tap91624@gmail.com>
Signed-off-by: yexiaochuan <tap91624@gmail.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
Ensure
from_msgpackproperly handles containers ofstd::byteby mappingstd::bytetounsigned char.Changes
char_traits<std::byte>when__cpp_lib_byte >= 201603L.to_int_type,to_char_type, andeof()forstd::byteusingstd::char_traits<char>as a base.#if defined(__cpp_lib_byte) && __cpp_lib_byte >= 201603Lto maintain compatibility with C++11.Reason
Without
char_traits<std::byte>, usingfrom_msgpackonstd::vector<std::byte>leads to a appleclang compilation failure, becausechar_traits<T>is required by the input adapter.Fixes #4756
Testing Done
ctest --test-dir build -j 10).tests/src/unit-msgpack.cppChecklist
make amalgamate.Additional Note on Test Behavior
The newly added unit test verifying
from_msgpackwithstd::bytecontainers is only strictly required and triggered on stricter compilers such as:On most GCC and older Clang versions, this specific issue does not manifest because of more relaxed type deduction and
char_traitsusage, so the corresponding test would pass without triggering a compilation error even without the fix.However, maintaining the specialization ensures consistent behavior and future-proof compatibility across all supported compilers.