Added public target_compile_features for auto and constexpr

[ktonon]

For projects which consume this header-only library using CMake, this adds public dependencies on C++ features to the library target. Requires an update to CMake 3.1.0, as target_compile_features was not available before that version.

Also, I'm not sure that the list of features is exhaustive. I only notice the use of auto and constexpr, and including these two was sufficient to get my example building without having to manually specify C++ 11.

[ktonon]

For reference:

• https://cmake.org/cmake/help/v3.1/prop_gbl/CMAKE_CXX_KNOWN_FEATURES.html
• https://cmake.org/cmake/help/v3.1/release/3.1.0.html#commands

[nlohmann]

What is the benefit compared to using

set(CMAKE_CXX_STANDARD 11)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

?

[OvermindDL1]

@nlohmann

It allows it to work on older compilers that have support for auto and so forth without all of C++11. Honestly though I'd probably just set it to C++11, it's quite literally a decade that 'most' compilers have had 'most' of its features anyway; even VS support C++11 (and C++14 and even most of C++17 too) and if it supports it then just about everything else does too.

[nlohmann]

I am not sure such a compiler exists :) We need to exclude GCC 4.8 due to bugs in the implementation, and I fear that it still would pass the feature checks.

[ktonon]

CMake 3.8 adds compiler features for C++ standards: cxx_std_98, cxx_std_11, cxx_std_14, cxx_std_17 (https://cmake.org/cmake/help/v3.8/prop_gbl/CMAKE_CXX_KNOWN_FEATURES.html)

My aim with this PR is to export the dependency on C++ 11 features as an interface requirement of using this library, so that clients can just target link the library and not worry about the standard. The use of fine grained features, or just cxx_std_11, is less important.

I'm pretty new to CMake, so maybe we don't need target_compile_features to achieve this.

[ktonon]

I wrapped this up as a Conan package, and consumed it like this. Without the changes in this PR, the compilation fails because C++ 11 is not manually specified.

PROJECT(TestJson)
cmake_minimum_required(VERSION 3.9)

include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup(TARGETS)

find_package(nlohmann_json)

add_executable(main main.cpp)

target_link_libraries(main nlohmann_json)

[ktonon]

The test is dead simple:

#include <nlohmann/json.hpp>
#include <iostream>

int main() {
	nlohmann::json j;
	j["pi"] = 3.14159;
	j["happy"] = true;
	std::cout << j.dump(4) << std::endl;
	return 0;
}

and there's nothing in there that is obviously C++ 11. So my point is, if I'm consuming this library, the dependency on auto, constexpr, ... should be explicit

[OvermindDL1]

I do agree there, the minimum language feature requirements should be exported from the CMake definition.

[nlohmann]

I am not sure whether listing all C++11 features make sense here - we just use too much of them. Requiring C++11 is much saner here. The snippet from #1026 (comment) is already used for parts of the test suite. The PR should be adjusted toward something similar, and also touch the other CMakeList files.

[ktonon]

@nlohmann that's fair. I think when you are ready to require a minimum version of CMake 3.8, then we can do just

target_compile_features(${NLOHMANN_JSON_TARGET_NAME} INTERFACE cxx_std_11)

Other than that, I'm not sure how to export CMAKE_CXX_STANDARD as a target requirement. Maybe set_target_properties? Or maybe not, since I don't see PUBLIC or INTERFACE modifies for that function.

Feel free to close this PR, for now I'm just patching the CMakeLists.txt file when I build with Conan.

[coveralls]

Coverage remained the same at 100.0% when pulling a35d414 on ktonon:develop into d2dd27d on nlohmann:develop.

[nlohmann]

@ktonon I would be happy with a PR implementing the changes you describe in #1026 (comment).

[ktonon]

Replaced the commit. Using CMake 3.8 and cxx_std_11 now. For consistency I updated the test and benchmark CMakeLists.txt files too

[nlohmann]

Some jobs fail because of a too old CMake version. I need to adjust the .travis.yml file for that.

[nlohmann]

@ktonon Could you please add the line brew upgrade cmake to the .travis.yml file? Here is a patch:

diff --git a/.travis.yml b/.travis.yml
index 8ce38cc8..68a16db5 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -277,6 +277,7 @@ script:
      if [[ (-x $(which brew)) ]]; then
        brew update
        brew install cmake ninja
+       brew upgrade cmake
        cmake --version
      fi

[nlohmann]

Looks good to me.

[nlohmann]

Thanks a lot!