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

2.1.1+ breaks compilation of shared_ptr<json> == 0 #610

Closed
vslavik opened this issue Jun 7, 2017 · 3 comments
Closed

2.1.1+ breaks compilation of shared_ptr<json> == 0 #610

vslavik opened this issue Jun 7, 2017 · 3 comments
Labels
confirmed solution: wontfix the issue will not be fixed (either it is impossible or deemed out of scope)

Comments

@vslavik
Copy link

vslavik commented Jun 7, 2017

v2.1.1 introduced regression that breaks compilation of existing code that compiled with up to v2.1.0. The breakage was introduced in #415 (fixing #414) and relates to over-reaching template operator==.

Code that uses std::shared_ptr<json> and compares it with zero (both == 0 and != 0) no longer compiles due to operator ambiguity that is not immediately (or even no so immediately...) apparent to a mere human like me, but nevertheless I confirmed was caused by #415.

This is enough to reproduce:

#include <json.hpp>

int main()
{
    std::shared_ptr<nlohmann::json> p;
    if (p==0) {
    }
    return 0;
}

This doesn't make the compiler happy:

$ clang++ —std=c++14 test.cpp
test.cpp:16:10: error: use of overloaded operator '==' is ambiguous (with operand types
      'std::shared_ptr<nlohmann::json>' (aka 'shared_ptr<basic_json<> >') and 'int')
    if (p==0) {
        ~^ ~
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/memory:4922:1: note: 
      candidate function [with _Tp = nlohmann::basic_json<std::map, std::vector, std::__1::basic_string<char>, bool,
      long long, unsigned long long, double, std::allocator, adl_serializer>]
operator==(const shared_ptr<_Tp>& __x, nullptr_t) _NOEXCEPT
^
json.hpp:6334:17: note: candidate function
      [with ScalarType = int, $1 = 0]
    friend bool operator==(const_reference lhs, const ScalarType rhs) noexcept
                ^
json.hpp:6253:17: note: candidate function
    friend bool operator==(const_reference lhs, const_reference rhs) noexcept
                ^
1 error generated.
$

Yes, it would be better to compare with nullptr or use implicit conversion to bool, and it would be trivial to fix the above example, but you don't always have a choice… such as when using another library where this happens internally to it. To name a prominent example, Boost.Thread is affected:

#define BOOST_THREAD_VERSION 4
#include <boost/thread/future.hpp>
#include <json.hpp>

int main()
{
    boost::future<nlohmann::json> f;
    f.get();
    return 0;
}

results in

boost/thread/future.hpp:1683:31: error: use of overloaded operator '==' is
      ambiguous (with operand types 'future_ptr' (aka 'shared_ptr<detail::shared_state<basic_json<std::map, std::vector,
      basic_string<char>, bool, long long, unsigned long long, double, std::allocator, adl_serializer> > >') and 'int')
            if (this->future_ == 0)
                ~~~~~~~~~~~~~ ^  ~
test.cpp:11:7: note: in instantiation of member function 'boost::future<nlohmann::basic_json<std::map, std::vector,
      std::__1::basic_string<char>, bool, long long, unsigned long long, double, std::allocator, adl_serializer> >::get'
      requested here
    f.get();
      ^
boost/smart_ptr/shared_ptr.hpp:814:31: note: candidate function [with T =
      boost::detail::shared_state<nlohmann::basic_json<std::map, std::vector, std::__1::basic_string<char>, bool, long
      long, unsigned long long, double, std::allocator, adl_serializer> >]
template<class T> inline bool operator==( shared_ptr<T> const & p, boost::detail::sp_nullptr_t ) BOOST_NOEXCEPT
                              ^
json.hpp:6334:17: note: candidate function
      [with ScalarType = int, $1 = 0]
    friend bool operator==(const_reference lhs, const ScalarType rhs) noexcept
                ^
json.hpp:6253:17: note: candidate function
    friend bool operator==(const_reference lhs, const_reference rhs) noexcept
                ^
1 error generated.

Observed with both clang-802.0.42 and Visual Studio 2015, so this isn't compiler-specific.

vslavik added a commit to vslavik/poedit that referenced this issue Jul 7, 2017
Fixes incorrect parsing of numbers with decimal point in locales that
use a decimal period natively.

The latest version cannot be used, because it breaks compilation with
Boost.Thread, see nlohmann/json#610
@nlohmann
Copy link
Owner

nlohmann commented Jul 8, 2017

I have no idea how to fix this from my side.

@nlohmann nlohmann added the state: help needed the issue needs help to proceed label Jul 8, 2017
@vslavik
Copy link
Author

vslavik commented Jul 8, 2017

I have no idea how to fix this from my side.

Me neither, trivially, otherwise I would have submitted a PR instead. Generally speaking, the over-eager implicit conversions (it was the recent custom types support that caused this) is causing the trouble, so more explicitness in that are would likely help.

I think this issue is a useful canary in a way: it's not entirely unlikely to cause similar issues elsewhere. If it were Boost.Thread only, well, that's only a matter of time until Boost-side fix bubbles up into a release and Linux distros.

@nlohmann
Copy link
Owner

nlohmann commented Jul 9, 2017

I think there is nothing I can do about this. Good to have this issue if someone has the same problem.

@nlohmann nlohmann added solution: wontfix the issue will not be fixed (either it is impossible or deemed out of scope) and removed state: help needed the issue needs help to proceed labels Jul 9, 2017
@nlohmann nlohmann closed this as completed Jul 9, 2017
vslavik added a commit to vslavik/poedit that referenced this issue Apr 21, 2019
This is forced by incompatiblity between JSON for Modern C++ and
Boost.Threads up to 1.64, as used by Poedit's code. See
nlohmann/json#610
vslavik added a commit to vslavik/poedit that referenced this issue Apr 21, 2019
This is forced by incompatiblity between JSON for Modern C++ and
Boost.Threads up to 1.64, as used by Poedit's code. See
nlohmann/json#610
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed solution: wontfix the issue will not be fixed (either it is impossible or deemed out of scope)
Projects
None yet
Development

No branches or pull requests

2 participants