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

To fix compilation issue for intel OSX compiler #682

Merged
merged 1 commit into from
Aug 9, 2017

Conversation

kbthomp1
Copy link
Contributor

@kbthomp1 kbthomp1 commented Aug 8, 2017

o To prevent the compilation issue on OSX with the intel compiler suite. The
error was found with icpc version 15.0.3.187 where the "clang_version" was
not defined correctly, while "clang" was.

Files to change

There are currently two files which need to be edited:

  1. src/json.hpp

  2. test/src/unit.cpp - This contains the Catch unit tests which currently cover 100 % of the library's code.

    If you add or change a feature, please also add a unit test to this file. The unit tests can be compiled and executed with

    make check

    The test cases are also executed with several different compilers on Travis once you open a pull request.

Note

  • If you open a pull request, the code will be automatically tested with Valgrind's Memcheck tool to detect memory leaks. Please be aware that the execution with Valgrind may in rare cases yield different behavior than running the code directly. This can result in failing unit tests which run successfully without Valgrind.

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.8 (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.

o To prevent the compilation issue on OSX with the intel compiler suite.  The
error was found with icpc version 15.0.3.187 where the "__clang_version__" was
not defined correctly, while "__clang__" was.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 4b55f0e on kbthomp1:fix-intel-osx into c90bf5e on nlohmann:develop.

Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@nlohmann nlohmann merged commit 6e5b895 into nlohmann:develop Aug 9, 2017
@nlohmann
Copy link
Owner

nlohmann commented Aug 9, 2017

Thanks a lot - I haven't tried ICC so far - does the test suite compile without problems? Are there warnings?

@nlohmann nlohmann self-assigned this Aug 9, 2017
@nlohmann nlohmann added kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation labels Aug 9, 2017
@nlohmann nlohmann added this to the Release 3.0.0 milestone Aug 9, 2017
@kbthomp1
Copy link
Contributor Author

kbthomp1 commented Aug 9, 2017

Actually, the test suite doesn't compile with OS X icc. I'm getting both errors and warnings from icpc when running:

CC=icc CXX=icpc make check

make_check.txt

nlohmann added a commit that referenced this pull request Aug 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants