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

Visual Studio 2017 15.8.5 "conditional expression is constant" warning on Line 1851 in json.hpp #1268

Closed
cugone opened this issue Oct 1, 2018 · 6 comments
Assignees
Milestone

Comments

@cugone
Copy link

cugone commented Oct 1, 2018

There is a warning "conditional expression is constant" issued for VIsual Studio 2017 version 15.8.5 on Line 1851 of the json.hpp single-include header. I have Treat Warnings As Errors turned on as well as the Warning Level set to 4 so this prevents successful compilation. I do not use Clang nor GCC so I do not know if a similar issue shows up and this is just an edge case and can be safely pragma'd out.

(Immediate surrounding code provided for context)

it can be fixed by changing the if-statement to a constexpr-if:
if constexpr (sizeof(typename WideStringType::value_type) == 2) { //...

template<typename WideStringType>
class wide_string_input_adapter : public input_adapter_protocol {
public:
    explicit wide_string_input_adapter(const WideStringType& w) : str(w) {}

    std::char_traits<char>::int_type get_character() noexcept override {
        // check if buffer needs to be filled
        if(utf8_bytes_index == utf8_bytes_filled) {
//Line 1851 WARNING ISSUED Here VV
            if(sizeof(typename WideStringType::value_type) == 2) {
                fill_buffer_utf16();
            } else {
                fill_buffer_utf32();
            }

            assert(utf8_bytes_filled > 0);
            assert(utf8_bytes_index == 0);
        }

        // use buffer
        assert(utf8_bytes_filled > 0);
        assert(utf8_bytes_index < utf8_bytes_filled);
        return utf8_bytes[utf8_bytes_index++];
    }
@gregmarr
Copy link
Contributor

gregmarr commented Oct 1, 2018

This library only requires a C++11 compiler, so if constexpr can't be used here.

@theodelrieu
Copy link
Contributor

We could add a template fill_buffer function taking the sizeof as a parameter.

@cugone Would you like to open a PR?

@nlohmann
Copy link
Owner

nlohmann commented Oct 1, 2018

Maybe a duplicate of #1204.

@nlohmann
Copy link
Owner

nlohmann commented Oct 1, 2018

And, of course, a PR would be greatly appreciated!

@cugone
Copy link
Author

cugone commented Oct 1, 2018

@nlohmann Attempted PR but Visual Studio's support of makefiles is lacking. Using nmake via VS developer console to try to run the makefile as per CONTRIBUTING.md complains: "makefile(33) : fatal error U1000: syntax error : ')' missing in macro invocation". :(

Anyway, after looking at the most recent push to json.hpp prior to Aug 20, yes, it is an exact duplicate of #1204.

Visual Studio has been fully C++14 compliant since or before VS2017 RTM (VS 15.0) was released. If your holdout to moving to at least support C++14 was visual studio, I think it would be a good time to at least support that for now.

See: https://blogs.msdn.microsoft.com/vcblog/2018/05/07/announcing-msvc-conforms-to-the-c-standard/

Sadly, yes, if constexpr statements are C++17, so the true fix will have to be a few years away when C++20 is released and C++17 is fully adopted.

@nlohmann
Copy link
Owner

nlohmann commented Oct 4, 2018

Fixed by merging #1272.

@nlohmann nlohmann closed this as completed Oct 4, 2018
@nlohmann nlohmann self-assigned this Oct 4, 2018
@nlohmann nlohmann added this to the Release 3.3.0 milestone Oct 4, 2018
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

4 participants