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

Fixed issue #167 - removed operator ValueType() condition for VS2015 #188

Merged
merged 1 commit into from
Jan 24, 2016

Conversation

twelsby
Copy link
Contributor

@twelsby twelsby commented Jan 23, 2016

Removed the condition and not std::is_same<ValueType, std::initializer_list<typename string_t::value_type>>::value from the operator ValueType() const template for VS2015 only. This is what has been causing AppVeyor build errors since it was introduced in commit 457bfc2 to address a problem with implicit conversion to strings (issue #144). While this worked for gcc it causes operator<< ambiguity under VS2015 for reasons that are still unclear (to me, but may be obvious to others). It would be more satisfying to know why this caused VS2015 to break but not gcc but at least it solves the problem. The fix for implicit conversion to strings still works after this change.

@Cleroth
Copy link

Cleroth commented Jul 4, 2016

This causes VS2015 to not be able to type cast into a string.
ie. this won't compile:

basic_json<> j;
auto s = (std::string)j["test"];

Putting back the condition fixes it. I'm not sure about operator<< ambiguity. Can you post the code that makes it break?

@nlohmann
Copy link
Owner

nlohmann commented Jul 4, 2016

I cannot recall right now... I'll have a look.

@nlohmann
Copy link
Owner

nlohmann commented Jul 4, 2016

@Cleroth, the change originated a post by @twelsby (#167 (comment)) who also made this PR. As I cannot test MSVC myself, I do not know whether this change is still needed.

@aholzinger
Copy link

Unfortunately this 9de14a4 (the #ifndef _MSC_VER) "fix" is bouncing back with Visual Studio 2019 16.5:

Compiling https://github.com/sony/nmos-cpp/blob/master/Development/third_party/nlohmann/json-validator.cpp gives this:

1>------ Build started: Project: json_schema_validator_static, Configuration: Debug x64 ------
1>json-validator.cpp
1>C:\nmos-cpp\Development\third_party\nlohmann\json-validator.cpp(594,1): error C2593: 'operator =' is ambiguous
1>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.25.28610\include\xstring(2591,19): message : could be 'std::basic_string<char,std::char_traits<char>,std::allocator<char>> &std::basic_string<char,std::char_traits<char>,std::allocator<char>>::operator =(std::initializer_list<_Elem>)'
1>        with
1>        [
1>            _Elem=char
1>        ]
1>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.25.28610\include\xstring(2492,19): message : or       'std::basic_string<char,std::char_traits<char>,std::allocator<char>> &std::basic_string<char,std::char_traits<char>,std::allocator<char>>::operator =(std::basic_string<char,std::char_traits<char>,std::allocator<char>> &&) noexcept(<expr>)'
1>C:\nmos-cpp\Development\third_party\nlohmann\json-validator.cpp(594,1): message : while trying to match the argument list '(std::string, nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer>)'

With the line

                 and not std::is_same<ValueType, std::initializer_list<typename string_t::value_type>>::value

not #ifdefed out in Visual Studio 2019 16.5 (_MSC_VER == 1925) it's compiling correctly.

So I propose this patch:

- #ifndef _MSC_VER  // Fix for issue #167 operator<< ambiguity under VS2015
+ #if !defined _MSC_VER || _MSC_VER >= 1925  // fix for issue #167 operator<< ambiguity under VS2015 and operator= ambiguity for VS2019 >= 16.5

@garethsb
Copy link
Contributor

garethsb commented Mar 23, 2020

I agree something like the change @aholzinger suggests is required. I suggest the preprocessor condition two lines down should also be revisited, since the _MSCVER bit can't possibly have applied given the outer #ifndef _MSC_VER/#endif. Looking at the GitHub blame it would appear these were two separate cases originally, not nested (specifically, see bf348ca#r38013102).

@garethsb
Copy link
Contributor

#167 (comment) includes some minimal code that failed to compile with VS2015. I tested this myself (with Version 14.0.25431.01 Update 3) and it compiles fine whether or not the outer #ifndef _MSC_VER/#endif is present or not.

On the other hand, I found that similar code using nlohmann::json_pointer<nlohmann::json>::operator< still fails whether I keep or remove that preprocessor condition.

And that applies in VS 2019 (Version 16.5.0) also.

Demo: https://godbolt.org/z/vaksWb

@aholzinger, probably these issues with VS 2019 need to be raised as a new issue, maybe referring to this, in order to get any traction.

@aholzinger
Copy link

@garethsb-sony, I just searched to find this comment/issue and it was hard to find, so I guess you're right. You made the example also, do you want to add a new issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants