Fix warning C4127: conditional expression is constant

[antonioborondo]

#1268

---

Pull request checklist

Read the Contribution Guidelines for detailed information.

• Changes are described in the pull request, or an existing issue is referenced.
• The test suite compiles and runs without error.
• Code coverage is 100%. Test cases can be added by editing the test suite.
• The source code is amalgamated; that is, after making changes to the sources in the include/nlohmann directory, run make amalgamate to create the single-header file single_include/nlohmann/json.hpp. The whole process is described here.

Please don't

• The C++11 support varies between different compilers and versions. Please note the list of supported compilers. Some compilers like GCC 4.7 (and earlier), Clang 3.3 (and earlier), or Microsoft Visual Studio 13.0 and earlier are known not to work due to missing or incomplete C++11 support. Please refrain from proposing changes that work around these compiler's limitations with #ifdefs or other means.
• Specifically, I am aware of compilation problems with Microsoft Visual Studio (there even is an issue label for these kind of bugs). I understand that even in 2016, complete C++11 support isn't there yet. But please also understand that I do not want to drop features or uglify the code just to make Microsoft's sub-standard compiler happy. The past has shown that there are ways to express the functionality such that the code compiles with the most recent MSVC - unfortunately, this is not the main objective of the project.
• Please refrain from proposing changes that would break JSON conformance. If you propose a conformant extension of JSON to be supported by the library, please motivate this extension.
• Please do not open pull requests that address multiple issues.

[theodelrieu]

I was thinking of something like this (to not add runtime overhead):

template <std::size_t N>
void fill_buffer() {
  fill_buffer_utf32();
}

template <>
void fill_buffer<2>() {
  fill_buffer_utf16();
}

Also, do not forget to edit files in include/nlohmann as well.

[antonioborondo]

@theodelrieu I just edited the include files and generated the header 👍

About the implementation, I can change it if you want to use templates, but I am not sure how much overhead I am introducing with my implementation. I would say none actually 🤔

[theodelrieu]

Well, you are performing a runtime check (with the if (sizeof(...))), which should not silence the warning.

Unfortunately the templates way is the pre-C++17 way of doing if constexpr.

[nlohmann]

Yes, please use templates. The change right now makes little sense.

[antonioborondo]

@theodelrieu @nlohmann The warning is silenced with the current implementation, but I will use templates if you prefer.

[coveralls]

Coverage remained the same at 100.0% when pulling b6fdad9 on antonioborondo:fix_warning into 9f18e17 on nlohmann:develop.

[antonioborondo]

Commit 9ba3f79 fixes the following error in LGTM:

[2018-10-03 10:49:06] [build] /opt/src/single_include/nlohmann/json.hpp:2026:14: error: explicit specialization in non-namespace scope ‘class nlohmann::detail::wide_string_input_adapter<WideStringType>’
[2018-10-03 10:49:06] [build]      template<>
[2018-10-03 10:49:06] [build]               ^
[2018-10-03 10:49:06] [build] /opt/src/single_include/nlohmann/json.hpp:2027:25: error: template-id ‘fill_buffer<2>’ in declaration of primary template
[2018-10-03 10:49:06] [build]      void fill_buffer<2>()

[antonioborondo]

Commit 7c385a4 fixes the following error in LGTM:

[2018-10-03 11:31:51] [build] /opt/src/single_include/nlohmann/json.hpp: In member function ‘void nlohmann::detail::wide_string_input_adapter<WideStringType>::fill_buffer()’:
[2018-10-03 11:31:51] [build] /opt/src/single_include/nlohmann/json.hpp:2023:9: error: ‘wide_string_input_helper’ was not declared in this scope
[2018-10-03 11:31:51] [build]          wide_string_input_helper<WideStringType, T>::fill_buffer(str, current_wchar, utf8_bytes, utf8_bytes_index, utf8_bytes_filled);
[2018-10-03 11:31:51] [build]          ^~~~~~~~~~~~~~~~~~~~~~~~
[2018-10-03 11:31:51] [build] /opt/src/single_include/nlohmann/json.hpp:2023:9: note: suggested alternative: ‘wide_string_input_adapter’
[2018-10-03 11:31:51] [build]          wide_string_input_helper<WideStringType, T>::fill_buffer(str, current_wchar, utf8_bytes, utf8_bytes_index, utf8_bytes_filled);
[2018-10-03 11:31:51] [build]          ^~~~~~~~~~~~~~~~~~~~~~~~
[2018-10-03 11:31:51] [build]          wide_string_input_adapter
[2018-10-03 11:31:51] [build] /opt/src/single_include/nlohmann/json.hpp:2023:48: error: expected primary-expression before ‘,’ token
[2018-10-03 11:31:51] [build]          wide_string_input_helper<WideStringType, T>::fill_buffer(str, current_wchar, utf8_bytes, utf8_bytes_index, utf8_bytes_filled);
[2018-10-03 11:31:51] [build]                                                 ^
[2018-10-03 11:31:51] [build] /opt/src/single_include/nlohmann/json.hpp:2023:54: error: ‘::fill_buffer’ has not been declared
[2018-10-03 11:31:51] [build]          wide_string_input_helper<WideStringType, T>::fill_buffer(str, current_wchar, utf8_bytes, utf8_bytes_index, utf8_bytes_filled);
[2018-10-03 11:31:51] [build]                                                       ^~~~~~~~~~~
[2018-10-03 11:31:52] [build] /opt/src/single_include/nlohmann/json.hpp:2023:54: note: suggested alternative: ‘setbuffer’
[2018-10-03 11:31:52] [build]          wide_string_input_helper<WideStringType, T>::fill_buffer(str, current_wchar, utf8_bytes, utf8_bytes_index, utf8_bytes_filled);
[2018-10-03 11:31:52] [build]                                                       ^~~~~~~~~~~
[2018-10-03 11:31:52] [build]                                                       setbuffer

[nlohmann]

This pull request fixes 1 alert when merging 7c385a4 into 9f18e17 - view on LGTM.com

fixed alerts:

• 1 for Comparison result is always the same

---

Comment posted by LGTM.com

[nlohmann]

This pull request fixes 1 alert when merging b6fdad9 into 9f18e17 - view on LGTM.com

fixed alerts:

• 1 for Comparison result is always the same

---

Comment posted by LGTM.com

[antonioborondo]

@theodelrieu PR ready to merge 👍

[nlohmann]

Looks good to me.

[nlohmann]

Thanks!