Enc, Dec: add support for additional standard empty types (std::monostate and std::nullopt_t)
#90
Conversation
This file contains 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
CuriousGeorgiy
force-pushed
the
gh-82-variant-monostate
branch
3 times, most recently
from
647c24d to
495480e
Compare
drewdzzz
approved these changes
Jan 23, 2024
CuriousGeorgiy
force-pushed
the
gh-82-variant-monostate
branch
3 times, most recently
from
9ad1a75 to
8ffd21d
Compare
|
@drewdzzz I refactored the patch a bit, please take another look. |
|
The leftover linting error is a FP fixed in your patch. |
drewdzzz
reviewed
Jan 24, 2024
src/Utils/Traits.hpp
Outdated
| @@ -638,6 +638,15 @@ template <class T> | |||
| constexpr bool is_uni_member_ptr_v = | |||
| std::is_member_object_pointer_v<uni_integral_base_t<T>>; | |||
|
|
|||
| struct empty_type { | |||
| constexpr empty_type() = default; | |||
There was a problem hiding this comment.
Nit: Let's mark it as noexcept as well.
drewdzzz
approved these changes
Jan 24, 2024
drewdzzz
reviewed
Jan 24, 2024
CuriousGeorgiy
changed the title
std::monostatestd::monostate and std::nullopt_t (_empty_ types)
CuriousGeorgiy
changed the title
std::monostate and std::nullopt_t (_empty_ types)std::monostate and std::nullopt_t (empty types)
CuriousGeorgiy
changed the title
std::monostate and std::nullopt_t (empty types)std::monostate and std::nullopt_t)
CuriousGeorgiy
force-pushed
the
gh-82-variant-monostate
branch
5 times, most recently
from
23a3de3 to
32d4186
Compare
alyapunov
suggested changes
Jan 29, 2024
src/mpp/Constants.hpp
Outdated
| std::nullptr_t, bool, uint64_t, int64_t, float, double, | ||
| StrValue, BinValue, ArrValue, MapValue, ExtValue | ||
| >; | ||
| std::nullptr_t, std::monostate, bool, uint64_t, int64_t, float, double, |
There was a problem hiding this comment.
What about nullopt here?. also I'm afraid of the comment.. I seems that we should not change this.
There was a problem hiding this comment.
This is actually dead code (used in previous codec version), so let's just drop it in a separate commit.
CuriousGeorgiy
force-pushed
the
gh-82-variant-monostate
branch
from
32d4186 to
ae4dacb
Compare
The commit makes clang-format config more suitable to our codebase.
If we are reading a nested type, the return value of `read_item` indicates the number of children the nested type has. However, `read_item` has completely different semantics in case of decoding other types and can even return a completely different non-integer type (the return type is `auto`). Let's put the code in which we rely on the semantics of `read_item` under `if constexpr`.
The `Value_t` definition was used in the previous codec version for abstract value decoding, but now it is unused. Let's drop it so that we don't have to maintain it.
CuriousGeorgiy
force-pushed
the
gh-82-variant-monostate
branch
from
ae4dacb to
4f6b979
Compare
alyapunov
approved these changes
Jan 30, 2024
In addition to `std::nullptr_t`, there are at least 2 more standard empty types, namely `std::monostate` and `std::nullopt_t`, which should correspond to `MP_NIL` in the MsgPack codec. Refactor the codec to support an empty type abstraction, and add `std::monostate` and `std::nullopt_t` to this abstraction. Closes tarantool#82
CuriousGeorgiy
force-pushed
the
gh-82-variant-monostate
branch
from
4f6b979 to
19dc32a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 patch set brings support for additional standard empty types (
std::monostateandstd::nullopt_t) to the MsgPack codec.Closes #82