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

warning C4706: assignment within conditional expression #295

Closed
Mighty-Professional opened this issue Aug 16, 2016 · 12 comments
Closed

warning C4706: assignment within conditional expression #295

Mighty-Professional opened this issue Aug 16, 2016 · 12 comments
Assignees
Milestone

Comments

@Mighty-Professional
Copy link

Mighty-Professional commented Aug 16, 2016

There is a few lines like:

if (keep and (not callback or (keep = callback(depth++, parse_event_t::object_start, result))))

That are causing warnings and make projects that treat warnings as errors unable to compile without fixing it or disabling warnings in the code.

Microsoft Visual Studio Compiler 2015

@nlohmann
Copy link
Owner

I understand the warnings, but they seem to be uncritical and rather pedantic. A solution would be to either restructure the code by moving the assignment out of the conditional and make it less readable or to add #pragmas and make the code even less readable. Any other suggestions?

@nlohmann
Copy link
Owner

Just to better understand MSVC at warning level 4: Would

if (keep and (not callback or ((keep = callback(depth++, parse_event_t::object_start, result))) != 0))

fix this issue?

@Mighty-Professional
Copy link
Author

Yes that fixes the issue.

It could be rewritten I do not think it makes it any less clear, (I personally find that line with increment depth in the if a bit confusing to begin with. (But I am not a super experienced coder). )

For a easy fix you could just add:

#ifdef _MSC_VER
#pragma warning(push)
#pragma warning(disable:4706) // assignment within conditional expression
#endif
...

#ifdef _MSC_VER
#pragma warning(pop)
#endif

There is already some code that ignores specific warnings for clang.

But its really up to you, as you said its not a critical problem. Just requires a tiny bit more work for someone integrating it.

Awesome library btw, easy to use and great features.

@nlohmann
Copy link
Owner

nlohmann commented Aug 16, 2016

Ok. I think adding some != 0 is a bit nicer than adding another pragma in an ifdef. I wonder why MSVC is not silenced by another pair of parentheses like Clang.

Thanks for reporting! For a MSVC beginner - can you tell me how to compile a CMake project with this level of warnings?

Added: https://msdn.microsoft.com/en-us/library/7hw7c1he%28v=vs.80%29.aspx

@Mighty-Professional
Copy link
Author

Mighty-Professional commented Aug 16, 2016

Sorry to say I do not use CMake nor know how. I just included the source file in my visual studio project.

That being said Warning level 4 is /W4 or even better EnableAllWarnings is /Wall.

@nlohmann
Copy link
Owner

Thanks! I shall add this to the AppVeyor build to see what else MSVC is warning about.

@nlohmann nlohmann added this to the Release 2.0.3 milestone Aug 17, 2016
@nlohmann nlohmann self-assigned this Aug 17, 2016
@nlohmann
Copy link
Owner

nlohmann commented Aug 17, 2016

I ran an AppVeyor build with /Wall flags. All output is in the log.txt file.

There are two error types: C4706 (assignment within conditional expression) and C4710 (function not inlined). We can ignore C4710. The remaining warnings are:

[00:02:49]   c:\projects\json\src\json.hpp(8895): warning C4706: assignment within conditional expression [C:\projects\json\test\json_unit.vcxproj]
[00:02:49] n\src\json.hpp(8973): warning C4706: assignment within conditional expression [C:\projects\json\test\json_unit.vcxproj]
[00:02:49]   c:\projects\json\src\json.hpp(8895): warning C4706: assignment within conditional expression [C:\projects\json\test\json_unit.vcxproj]
[00:02:49]   c:\projects\json\src\json.hpp(8973): warning C4706: assignment within conditional expression [C:\projects\json\test\json_unit.vcxproj]

The two cases can be fixed with additional parenthesis and != 0.

nlohmann added a commit that referenced this issue Aug 17, 2016
@nlohmann
Copy link
Owner

@marsh0 Could you please check if the warnings disappeared?

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Aug 18, 2016
@Mighty-Professional
Copy link
Author

Yup, looks good now! Thanks!

@nlohmann nlohmann removed the state: please discuss please discuss the issue or vote for your favorite option label Aug 18, 2016
@nlohmann
Copy link
Owner

Thanks for checking!

@eliasdaler
Copy link

Don't want to create a new thread, but I still have this warning on -W4 in VS 2017. Probably new expressions which should use the same pattern?

Or maybe #pragma will really be needed.

@nlohmann
Copy link
Owner

nlohmann commented Oct 2, 2017

@eliasdaler Could you open an issue and paste the warnings? Please use the develop version.

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

No branches or pull requests

3 participants