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

explicit constructor with default does not compile #3077

Closed
bpmckinnon opened this issue Oct 12, 2021 · 7 comments · Fixed by #3079
Closed

explicit constructor with default does not compile #3077

bpmckinnon opened this issue Oct 12, 2021 · 7 comments · Fixed by #3079
Assignees
Labels
confirmed kind: bug release item: 🐛 bug fix solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@bpmckinnon
Copy link

bpmckinnon commented Oct 12, 2021

From include/nlohmann/json.hpp
image
The change from ValueType ret; to ValueType ret{}; is causing a compiler error for the construction of a type with an explicit default constructor of the form.

explicit Foo(const alloc_context& allocContext = alloc_context())

In vs2019 it gives the following errors, using c++17
error C2512: '###': no appropriate default constructor available
note: Constructor for class '###' is declared 'explicit'

This is happening in a from_json call to my custom container type

Worked in 3.9.1, does not work in 3.10.2

@nlohmann
Copy link
Owner

Do you have a complete example?

@nlohmann nlohmann added platform: visual studio related to MSVC state: needs more info the author of the issue needs to provide more details labels Oct 12, 2021
@bpmckinnon
Copy link
Author

bpmckinnon commented Oct 12, 2021

This will repro the issue. It seems to be related to when a vector of a class contains a class that has an explicit constructor. I can probably work around this... but removing the {} from ValueType ret{} also fixes the problem, but I'm sure it's needed in some cases.

#include "nlohmann/json.hpp"

class FooAlloc
{};

class Foo
{
public:
    explicit Foo(const FooAlloc& = FooAlloc()) : value(false) {}

    bool value;
};

class FooBar
{
public:
    Foo foo;
};

inline void from_json(const nlohmann::json& j, FooBar& fb) { j.at("value").get_to(fb.foo.value); }

void test()
{
    nlohmann::json j;
    std::vector<FooBar> foo;
    j.at("foo").get_to(foo);
}

@nlohmann nlohmann added confirmed and removed platform: visual studio related to MSVC state: needs more info the author of the issue needs to provide more details labels Oct 13, 2021
@nlohmann
Copy link
Owner

I can confirm the issue with Xcode on macOS, so it's not an MSVC issue.

@nlohmann
Copy link
Owner

Interestingly, the {} is not mentioned in the documentation:

json/include/nlohmann/json.hpp

Lines 3035 to 3040 in 4b1cb9e

The function is equivalent to executing
@code {.cpp}
ValueType ret;
JSONSerializer<ValueType>::from_json(*this, ret);
return ret;
@endcode

The change was made in #2561 with 1101f0e, apparently to silence a warning from Clang-Tidy. I was not aware that this could break code like this.

I will add your example as regression test and revert the change and check if the CI passes.

@nlohmann nlohmann self-assigned this Oct 13, 2021
nlohmann added a commit that referenced this issue Oct 13, 2021
@nlohmann nlohmann linked a pull request Oct 13, 2021 that will close this issue
@nlohmann
Copy link
Owner

In fact, this is the Clang-Tidy warning that led to the (breaking) change:

error: uninitialized record type: 'ret' [cppcoreguidelines-pro-type-member-init,hicpp-member-init,-warnings-as-errors]
        ValueType ret;
        ^
                     {}
´´`

@bpmckinnon
Copy link
Author

Can you use
auto ret = ValueType(); ?

@nlohmann nlohmann added release item: 🐛 bug fix solution: proposed fix a fix for the issue has been proposed and waits for confirmation labels Oct 14, 2021
@nlohmann nlohmann added this to the Release 3.10.4 milestone Oct 14, 2021
@nlohmann
Copy link
Owner

PR #3079 is ready for review. Feedback greatly appreciated!

nlohmann added a commit that referenced this issue Oct 14, 2021
falbrechtskirchinger added a commit to falbrechtskirchinger/json that referenced this issue May 1, 2022
nlohmann added a commit that referenced this issue May 1, 2022
* ⬆️ Doctest 2.4.7

* 👷 add CI step for ICPC

* 👷 add CI step for ICPC

* 👷 add CI step for ICPC

* ⬇️ downgrade to Doctest 2.4.6

* 👷 add CI step for ICPC

* 👷 add CI step for ICPC

* 👷 add CI step for ICPC

* 👷 add CI step for ICPC

* 👷 add CI step for ICPC

* 🔇 suppress warning #2196: routine is both "inline" and "noinline"

* Re-enable <filesystem> detection on ICPC

* Limit regression test for #3070 to Clang and GCC >=8.4

* Disable deprecation warnings on ICPC

* Disable regression test for #1647 on ICPC (C++20)

* Fix compilation failure of regression test for #3077 on ICPC

* Disable wstring unit test on ICPC

Fixes:
  error 913: invalid multibyte character sequence

* Add ICPC to README

Co-authored-by: Niels Lohmann <mail@nlohmann.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed 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