Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assertion failure when serializing array with JSON_DIAGNOSTICS set #2926

Closed
nlohmann opened this issue Aug 12, 2021 · 6 comments · Fixed by #3037
Closed

Assertion failure when serializing array with JSON_DIAGNOSTICS set #2926

nlohmann opened this issue Aug 12, 2021 · 6 comments · Fixed by #3037
Assignees
Labels
kind: bug release item: 🐛 bug fix solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@nlohmann
Copy link
Owner

I fixed the issue and also found some more member functions that could have triggered the assertion, see #2866. Feedback welcome!

@nlohmann Do you also need to track capacity in the external construct functions, e.g.

    template < typename BasicJsonType, typename CompatibleArrayType,
               enable_if_t < !std::is_same<CompatibleArrayType, typename BasicJsonType::array_t>::value,
                             int > = 0 >
    static void construct(BasicJsonType& j, const CompatibleArrayType& arr)

?

I'm getting a very hard to reproduce write access violation with JSON_DIAGNOSTICS 1 when implicitly serializing an std::array of serializable structs. Can't get a minimal example to throw it though, not sure why. It seems related to this bug though.

Thrown here:

image

One stack up:

image

Edit: It's actually an array of std::optionals with a custom serializer per https://nlohmann.github.io/json/features/arbitrary_types/#how-do-i-convert-third-party-types. This works fine with JSON_DIAGNOSTICS 0.

Originally posted by @MHebes in #2838 (comment)

@nlohmann
Copy link
Owner Author

I currently fail to reproduce the issue. This is what I have so far:

#include <iostream>

#define JSON_DIAGNOSTICS 1
#include "json.hpp"
using json = nlohmann::json;

struct widget
{
    std::string name;
    int age;
    NLOHMANN_DEFINE_TYPE_INTRUSIVE(widget, name, age);
};

void to_json(json& j, const std::optional<widget>& opt)
 {
     if (opt.has_value())
     {
         j = *opt;
     }
     else
     {
         j = nullptr;
     }
 }

int main()
{
    std::array<std::optional<widget>, 3> widgets {{
        std::nullopt, widget{"Jane", 23}, widget{"John", 42}
    }};
    
    json j = widgets;
    std::cout << j << std::endl;
}

Can you share more information on your example?

@nlohmann nlohmann added the state: needs more info the author of the issue needs to provide more details label Aug 12, 2021
@nlohmann
Copy link
Owner Author

@MHebes In case your code uses ordered_json - #2963 may have fixed the issue.

@MHebes
Copy link

MHebes commented Aug 20, 2021

@nlohmann Sadly I am using the normal json. Thank you for migrating this issue! I have still tried and failed to reproduce this with a minimal example, but it is very reproducible in my larger project. I'll keep puttering away to try and come up with a minimal example. There's also always the possibility that it's my own code :)

@nlohmann
Copy link
Owner Author

nlohmann commented Oct 7, 2021

@MHebes We fixed another related issue, see #3037. Can you please try with the latest develop version?

@MHebes
Copy link

MHebes commented Oct 7, 2021

@MHebes We fixed another related issue, see #3037. Can you please try with the latest develop version?

That fixes it! Thanks for divining and fixing the root cause @AnthonyVH / @carlsmedstad

@nlohmann nlohmann added solution: proposed fix a fix for the issue has been proposed and waits for confirmation release item: 🐛 bug fix and removed state: needs more info the author of the issue needs to provide more details labels Oct 7, 2021
@nlohmann nlohmann added this to the Release 3.10.3 milestone Oct 7, 2021
@nlohmann nlohmann linked a pull request Oct 7, 2021 that will close this issue
4 tasks
@nlohmann nlohmann self-assigned this Oct 7, 2021
@nlohmann
Copy link
Owner Author

nlohmann commented Oct 7, 2021

Good to know this is fixed now!

@nlohmann nlohmann closed this as completed Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug release item: 🐛 bug fix solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants