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

json({}) gives null instead of empty object with GCC and -std=c++17 #2046

Closed
misos1 opened this issue Apr 17, 2020 · 27 comments
Closed

json({}) gives null instead of empty object with GCC and -std=c++17 #2046

misos1 opened this issue Apr 17, 2020 · 27 comments
Labels
kind: bug state: please discuss please discuss the issue or vote for your favorite option state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated state: waiting for PR

Comments

@misos1
Copy link

misos1 commented Apr 17, 2020

  • What is the issue you have?

Readme says:

// ways to express the empty object {}
json empty_object_implicit = json({});
json empty_object_explicit = json::object();

But seems when using GCC with -std=c++17 json({}) actually gives null. Without -std=c++17 or with -std=c++14 or any other version it works. On clang it works with any version.

Seems there is some problem with overload resolution because basic_json(std::nullptr_t = nullptr) is called instead of basic_json(initializer_list_t init, bool type_deduction = true.... It works after I explicitly set second parameter to true.

  • Please describe the steps to reproduce the issue. Can you provide a small but working code example?
#include "json.hpp"
int main()
{
	nlohmann::json j = nlohmann::json({});
	printf("%s: %s\n", j.type_name(), j.dump().c_str());
	return 0;
}
g++ -std=c++17 main.cpp
  • What is the expected behavior?
object: {}
  • And what is the actual behavior instead?
null: null

g++ (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0

  • Did you use a released version of the library or the version from the develop branch?

version 3.7.0

@misos1
Copy link
Author

misos1 commented Apr 17, 2020

Seems this bug is related only to constructors with default arguments (and it does not must be nullptr_t) or without arguments (with A() and void a() it is same):

#include <stdio.h>
#include <initializer_list>

struct A
{
	A(std::nullptr_t = nullptr) { printf("A nullptr\n"); }
	A(std::initializer_list<int>) { printf("A initializer_list\n"); }
	void a(std::nullptr_t = nullptr) { printf("a nullptr\n"); }
	void a(std::initializer_list<int>) { printf("a initializer_list\n"); }
};

struct B
{
	B(std::nullptr_t) { printf("B nullptr\n"); }
	B(std::initializer_list<int>) { printf("B initializer_list\n"); }
	void b(std::nullptr_t) { printf("b nullptr\n"); }
	void b(std::initializer_list<int>) { printf("b initializer_list\n"); }
};


int main()
{
	A({}).a({});
	B({}).b({});
	return 0;
}
A nullptr
a initializer_list
B initializer_list
b initializer_list

Maybe GCC with -std=c++17 treats json({}) as json{} (which gives null on clang and GCC with or without -std=c++17).

@ArtemSarmini
Copy link
Contributor

Or it treats it as json(json{}). Either way, I suppose one need to do json{{}} to call init list overload.

@nlohmann
Copy link
Owner

Thanks for reporting. This is a known issue, and it seems that the compilers behave differently on how to cope with an empty initializer list.

@nlohmann
Copy link
Owner

If that fixes the issue, this would be great! However, I am afraid that it would also break existing code as the behavior of the constructors was like this since version 1.0.0. Maybe we could add a preprocessor define to switch between old and new behavior, similar to policies in CMake?

@ArtemSarmini
Copy link
Contributor

Sorry I deleted that comment because realized we can't do much to provide compatible fix, I don't see an option. But documentation shoud be modified for sure. It would be nice if compiler guys come here for a bit to explain this.

@misos1
Copy link
Author

misos1 commented Apr 19, 2020

Or it treats it as json(json{}). Either way, I suppose one need to do json{{}} to call init list overload.

But this gives either [{}] or [null].

@mikeloomisgg
Copy link

Here to confirm that there is different behavior on msvc as well.
Clang and MSVC both evaluate nlohmann::json{nlohmann::json{}}.empty() as true, but gcc reports as false.
json{json{}} is null on gcc but empty on other compilers.

@Xeverous
Copy link

Xeverous commented May 2, 2020

So the issue basically is that {} is creating a value of type std::nullptr_t and calling the json(std::nullptr_t = nullptr) overload instead of json(initializer_list<T>). Could the case @mikeloomisgg mentioned be solved by splitting the overload set into 3 overloads?

  • json() (make object)
  • json(std::nullptr_t) (make null)
  • json(std::initializer_list<T>) (make object)

Then the nlohmann::json{} should call the 0-argument constructor. Am I right? I think the = nullptr default value is completely unnecessary for type std::nullptr_t - it already can hold only 1 value. The null should always be specified explicitly and the default argument thing is very likely unneeded. Correct me if I'm wrong.

@dota17
Copy link
Contributor

dota17 commented May 11, 2020

During the test, I found that default constructor basic_json()(if implemented) and basic_json(std::nullptr_t) are ambiguous when call of overloaded basic_json(). so we couldn't set 3 overloads as you mentioned above @Xeverous. I tried to replace basic_json(std::nullptr_t) with basic_json(), finally, It was terminated due to some problems. So it doesn't seem to work.

In my tests on GCC-7.5, json{{}} json{json{}} are the same array type, and same value [null](not empty), but json{} is null type and null value in C++14 and C++17.

json(json{}) is null type and null value both in C++14 and C++17 too,
json({}) is object type, and will call basic_json(initializer_list_t) in C++14,
but is null type, null value, and will call basic_json(value_t), basic_json(nullptr_t) in C++17,

Anyway, the behavior of the constructors is a little weird.

@Xeverous
Copy link

Anyway, the behavior of the constructors is a little weird.

It is not weird. C++17 changed some rules regarding 1-element and 0-element braced init lists. This is likely a compiler bug (as different ones give different results) but the usage of the API might have a different meaning in different standards too.

@dota17
Copy link
Contributor

dota17 commented May 13, 2020

@Xeverous At first, I wanted to avoid this problem by designing more sophisticated basic_json constructors, but you're right, it's caused by different compiler rules. Maybe, as nlohmann said, we can solve this problem by predefined macros.

@stale
Copy link

stale bot commented Jun 12, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Jun 12, 2020
@stale stale bot closed this as completed Jun 20, 2020
@nlohmann nlohmann reopened this Jul 25, 2020
@stale stale bot removed the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Jul 25, 2020
@not-a-user
Copy link

A nullptr
a initializer_list
B initializer_list
b initializer_list

@misos1 Looks like this is an issue with gcc 7 only, not older, not newer: https://godbolt.org/z/z59adb

@stale
Copy link

stale bot commented Oct 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Oct 4, 2020
@stale stale bot closed this as completed Oct 12, 2020
@tawmoto
Copy link

tawmoto commented Jun 17, 2021

Hello, any news on this? I don't know how to correctly handle this in my code, because sometimes it returns null and sometimes {}.
Thanks

@SumuduLansakara
Copy link

SumuduLansakara commented Jun 26, 2021

I had a similar problem related to dumping. At least there, the value json::value_t::null set in default initialization caused the problem. Initializing the json explicitly as json j(json::value_t::object); solves this.

@davidkennedydev
Copy link

Thanks, @SumuduLansakara, to me seems that this should be the default value for a JSON instance initialized with an empty initializer list constructor.

Is a lot more reasonable if:

json({})

had the same well-defined meaning that:

json(json::value_t::object)

@tawmoto
Copy link

tawmoto commented Nov 11, 2021

I had a similar problem related to dumping. At least there, the value json::value_t::null set in default initialization caused the problem. Initializing the json explicitly as json j(json::value_t::object); solves this.

Sorry this does not work for me, it returns [{}], instead of {}

@nlohmann
Copy link
Owner

I had a similar problem related to dumping. At least there, the value json::value_t::null set in default initialization caused the problem. Initializing the json explicitly as json j(json::value_t::object); solves this.

Sorry this does not work for me, it returns [{}], instead of {}

No, this should work. Are you sure you wrote

json j(json::value_t::object);

rather than

json j{json::value_t::object};

?

@tawmoto
Copy link

tawmoto commented Nov 11, 2021

I had a similar problem related to dumping. At least there, the value json::value_t::null set in default initialization caused the problem. Initializing the json explicitly as json j(json::value_t::object); solves this.

Sorry this does not work for me, it returns [{}], instead of {}

No, this should work. Are you sure you wrote

json j(json::value_t::object);

rather than

json j{json::value_t::object};

?

Wow, indeed I was using the second method, such a small difference but such a huge difference in behaviour.
I hope it works across all compilers.
Thanks a lot.

@SylvainCorlay
Copy link

Beyond the discussion on default initialisation of nlohmann::json, I think that nlohmann::json::object() should ensure that the returned value is an object and not null.

@Xeverous
Copy link

Xeverous commented Jan 7, 2022

100% agree. nlohmann::json::object implies in the name it is an object and under no circumstances it should create nulls. One of key points of named constructor pattern (in C++ a public static method) is that you clearly express what you want and I think any sane person would agree about ::object name purpose.

@gregmarr
Copy link
Contributor

gregmarr commented Jan 7, 2022

I think that nlohmann::json::object() should ensure that the returned value is an object and not null.

It does.

@SylvainCorlay
Copy link

I think that nlohmann::json::object() should ensure that the returned value is an object and not null.

It does.

It does not seem so. That's what brought us here (cf jupyter-xeus/xeus#309 (comment)).

@gregmarr
Copy link
Contributor

gregmarr commented Jan 7, 2022

It does, at least in 3.6.0: https://godbolt.org/z/z8qWEoj34

The original report here was about

 nlohmann::json j({});

not about

auto j = nlohmann::json::object();

@DerThorsten
Copy link

@gregmarr the report from @SylvainCorlay was based on wrong info from me. We had still a few nlohmann::json() in our code. Sry for the noise

@gregmarr
Copy link
Contributor

gregmarr commented Jan 7, 2022

I saw, thanks for the confirmation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug state: please discuss please discuss the issue or vote for your favorite option state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated state: waiting for PR
Projects
None yet
Development

No branches or pull requests