WIP: use templated is_negative to avoid warning on some compilers

[nickaein]

Related to #2676, #1390, #755, #2712

[Describe your pull request here. Please read the text below the line, and make sure you follow the checklist.]

---

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.

[coveralls]

Coverage remained the same at 100.0% when pulling da50013 on nickaein:fix/check-is-negative into fe23033 on nlohmann:develop.

[nlohmann]

Thanks for the PR! I think it would be a good idea to also add the Intel compiler to the CI. Any proposal for this?

[nickaein]

Sure! I wanted to play around and see what could be done for this issue.

Adding Intel Compiler(s) is great idea. I will look into it.

[nlohmann]

@nickaein If you can point me to instructions how to install it in Ubuntu, I can add it to the Docker image I'm using in the CI. I just wanted to make sure these warnings don't come back...

[nickaein]

As an update, I installed Intel C++ compiler inside the existing json-ci docker image:

FROM nlohmann/json-ci

RUN wget https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB && \
    APT_KEY_DONT_WARN_ON_DANGEROUS_USAGE=1 apt-key add GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB && \
    rm GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB && \
    add-apt-repository -y "deb https://apt.repos.intel.com/oneapi all main" && \
    apt-get update

RUN apt-get install -y --no-install-recommends \
        intel-oneapi-compiler-dpcpp-cpp-and-cpp-classic

And add it to the CI of my fork in this commit: nickaein@7cc2c69. The pipline for this compiler passes without any issues (link).

Unfortunately, the infamous sign warning (e.g. #755) is not reproduced on this compiler version. I'm using icpx as the C++ compiler that seems to be Clang-based and is recently released by Intel. There is also an icpc (classic Intel C++ compiler), but I couldn't get it working after few attempts. It might worth the try in the hope that the issue will be reproduced.

Nevertheless, there are other environments that Intel compiler(s) can be installed (e.g. Windows/MacOS). The GitHub Workflow for their setup is available at: oneapi-ci project. While their setup is different from CI of this project, I think it might worth adapting it. This will bifurcate the current CI architecture, but that could be a plus to have these configuration isolated (e.g. not polluting json-ci, having a dedicated cmake/ci-intel.cmake).

[nlohmann]

Thanks for looking into it! I could reproduce your findings that everything works fine (i.e., without warning on develop) with icpx. (Note to self: do not forget

source /opt/intel/oneapi/setvars.sh

I tried icpc and learned the following:

• Out of the box, cmake .. -DCMAKE_C_COMPILER=icc -DCMAKE_CXX_COMPILER=icpc does not work, because system headers are not found.
• I could make it work by making gcc point to a system GCC (not the experimental/latest version) with export PATH=/usr/bin:$PATH.

Now I'm stuck with this error message:

/root/json/test/thirdparty/doctest/doctest.h(1238): internal error: assertion failed at: "exprutil.c", line 5535 in make_cast_rescan_operands

          DOCTEST_DO_BINARY_EXPRESSION_COMPARISON(==, " == ", DOCTEST_CMP_EQ) //!OCLINT bitwise operator in conditional
          ^

compilation aborted for /root/json/test/src/unit-user_defined_input.cpp (code 4)

Next, I will check if this may be a doctest issue.

Anyway, I'm thinking of adding the Intel compilers to my Docker.

[nlohmann]

@nickaein You are 2 hours ahead of me :-) doctest/doctest#502 - and you also linked the issue. Uff, if I had seen that, I could have saved some time ;)

[nickaein]

Oops, I should've mentioned earlier that I'm going to continue working on this. My bad.

Yeah it was not a straightforward task to setup these compilers. I had to use the following weird compiler flags to get it working. Yours is easier to grasp.

icpc -std=c++11 -gnu-prefix=x86_64-linux-gnu- -gxx-name=g++-9 example.cpp

Having the experience on configuring these compilers, you probably can give your opinion on last point of my previous comment #2736 (comment). Basically, I think adapting oneapi-ci's Actions workflow may be a good approach as we can cover different variants of Intel compilers (LLVM-based, Classic, Data Parallel C++ aka. icx, icc, dpcpp) on Windows/MacOS/Linux platforms, without polluting json-ci image. Another plus is it would take less effort to adapt their changes in future.

A proof of concept is available on ci/intel-compilers branch of my fork. I copied the scripts from oneapi-ci project and a few changes which can be seen here: nickaein#1. A lot of these codes can be easily removed though (e.g. jobs and scripts related to testing different compiler install methods).

[nlohmann]

Yes, I agree that it may be good to have all the Intel compilers combined at one place - especially since there are also Windows/macOS versions. I'll be happy about PRs!

[philbucher]

@nickaein thanks, this solves the problem for me 👍

I tried sth similar but without success, so kudos to you :)

[nlohmann]

Hey @nickaein, I took your changes and added them in #3227 together with a CI step for NVCC. I am currently also trying to add a CI step for ICPC. In any case, this PR can be closed now. Thanks a lot for the contribution!

[nickaein]

That's awesome! Sorry I didn't have time to finalize this and integrate Intel compiler. Thanks for carrying the torch and finishing this up :)