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

C++17's ambiguous conversion #586

Closed
Krysme opened this issue May 16, 2017 · 6 comments
Closed

C++17's ambiguous conversion #586

Krysme opened this issue May 16, 2017 · 6 comments
Assignees
Labels
confirmed kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@Krysme
Copy link

Krysme commented May 16, 2017

In c++ 17, std::string_view can be converted to std::string implicitly which leads to a problem that in c++17 mode , a json object cannot be assigned to a string due to ambiguous assigning conversion path ( json --> string and json --> string_view --> string).

In gcc 7.1 such code produces a compilation error width -std=c++1z.

code:

#include "json.hpp"
#include <string>


int main()
{

    nlohmann::json j;
    j = "12345";
    std::string s;
    s = j;

    return 0;
}

So, is it possible to remove the conversion to string specifically in c++17 mode to let the compiler select the json--> string_view --> string path ?

@nlohmann
Copy link
Owner

This seems related to #464.

@nlohmann
Copy link
Owner

Checked with the latest GCC trunk version:

In file included from /Users/niels/Documents/projects/gcc/include/c++/8.0.0/string:52:0,
                 from /Users/niels/Documents/projects/gcc/include/c++/8.0.0/stdexcept:39,
                 from /Users/niels/Documents/projects/gcc/include/c++/8.0.0/array:39,
                 from src/json.hpp:33,
                 from c17.cpp:1:
/Users/niels/Documents/projects/gcc/include/c++/8.0.0/bits/basic_string.h: In instantiation of ‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::_If_sv<_Tp, std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&> std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator=(_Tp) [with _Tp = nlohmann::basic_json<>; _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>; std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::_If_sv<_Tp, std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&> = std::__cxx11::basic_string<char>&]’:
c17.cpp:11:9:   required from here
/Users/niels/Documents/projects/gcc/include/c++/8.0.0/bits/basic_string.h:764:28: error: call of overloaded ‘assign(nlohmann::basic_json<>&)’ is ambiguous
  { return this->assign(__sv); }
                            ^
/Users/niels/Documents/projects/gcc/include/c++/8.0.0/bits/basic_string.h:1313:7: note: candidate: ‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::assign(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]’
       assign(const basic_string& __str)
       ^~~~~~
/Users/niels/Documents/projects/gcc/include/c++/8.0.0/bits/basic_string.h:1329:7: note: candidate: ‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::assign(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]’
       assign(basic_string&& __str)
       ^~~~~~
/Users/niels/Documents/projects/gcc/include/c++/8.0.0/bits/basic_string.h:1440:7: note: candidate: ‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::assign(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::__sv_type) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>; std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::__sv_type = std::basic_string_view<char>]’
       assign(__sv_type __sv)
       ^~~~~~

@Krysme
Copy link
Author

Krysme commented May 16, 2017

Thanks for the replying.This error message you posted is exactly what I had.

@nlohmann
Copy link
Owner

This fixes the issue:

diff --git a/src/json.hpp b/src/json.hpp
index 85d559d7..7bfb7359 100644
--- a/src/json.hpp
+++ b/src/json.hpp
@@ -3742,7 +3742,7 @@ class basic_json
 #ifndef _MSC_VER  // fix for issue #167 operator<< ambiguity under VS2015
                    and not std::is_same<ValueType, std::initializer_list<typename string_t::value_type>>::value
 #endif
-#if defined(_MSC_VER) && _MSC_VER >1900 && defined(_HAS_CXX17) && _HAS_CXX17 == 1 // fix for issue #464
+#if (defined(__cplusplus) && __cplusplus == 201703L) || (defined(_MSC_VER) && _MSC_VER >1900 && defined(_HAS_CXX17) && _HAS_CXX17 == 1) // fix for issue #464
                    and not std::is_same<ValueType, typename std::string_view>::value
 #endif
                    , int >::type = 0 >

nlohmann added a commit that referenced this issue May 20, 2017
Also added a Travis builder with -std=c++1z
@nlohmann nlohmann self-assigned this May 20, 2017
@nlohmann nlohmann added this to the Release 3.0.0 milestone May 20, 2017
@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label May 20, 2017
@Krysme
Copy link
Author

Krysme commented May 20, 2017

thanks for the fix.
btw is it better to change __cplusplus == 201703L to __cplusplus >= 201703L for future version of c++?

@nlohmann
Copy link
Owner

Very good point. I shall fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

2 participants