stephenberry / glaze

Extremely fast, in memory, JSON and interface library for modern C++
MIT License
1.22k stars 121 forks source link

beve reads empty value, that should be skipped #1326

Open fzuuzf opened 1 month ago

fzuuzf commented 1 month ago

Hi Stephen,

this code:

#include "glaze/glaze.hpp"
#include "glaze/beve.hpp"

struct test_skip {
     std::optional<char> o_;  
};

int main() {
    std::vector<test_skip> a{{},{}};
    std::string        buffer;
    std::vector<char>      beve_buffer;

    auto err{glz::write_json(a, buffer)};
    auto beve_err{glz::write_beve(a, beve_buffer)};
    assert(!err && !beve_err);

    std::array<test_skip, 2> b{{{false}, {true}}};
    auto beve_b{b};

    err = glz::read_json(b, buffer);
    beve_err = glz::read_beve(beve_b, beve_buffer);
    assert(!err && !beve_err);

    // triggers:
    assert(b[0].o_ == beve_b[0].o_);

    return 0;
}

triggers the last assert here for gcc-14, clang-1[89] on ubuntu 24.04.

Thanks, Karsten

stephenberry commented 1 month ago

Thanks for bringing this up. Right now BEVE doesn't adhere to skip_null_members, which it should. Will keep this issue alive until support has been added.

fzuuzf commented 1 month ago

A fix:

diff --git a/include/glaze/beve/read.hpp b/include/glaze/beve/read.hpp
index a3abe46..c21ef1e 100644
--- a/include/glaze/beve/read.hpp
+++ b/include/glaze/beve/read.hpp
@@ -1108,15 +1108,8 @@ namespace glz
             GLZ_END_CHECK();
             const auto tag = uint8_t(*it);

-            if (tag == tag::null) {
+            if (tag == tag::null)
                ++it;
-               if constexpr (is_specialization_v<T, std::optional>)
-                  value = std::nullopt;
-               else if constexpr (is_specialization_v<T, std::unique_ptr>)
-                  value = nullptr;
-               else if constexpr (is_specialization_v<T, std::shared_ptr>)
-                  value = nullptr;
-            }
             else {
                if (!value) {
                   if constexpr (is_specialization_v<T, std::optional>)
stephenberry commented 1 month ago

So, we actually want to set the value to null if a null value is read in. The reason why your BEVE and JSON do not align right now is that by default JSON serialization uses the option skip_null_members, which means your JSON message will not set the elements to null. If you set skip_null_members to false then the JSON and BEVE would act the same.

The code you posted isn't really a fix. What is needed is skip_null_members support for BEVE so that null fields don't get written and thus don't set your optionals to null when read in.