CMake handling of the C++ standard suggestions

@hodoulp : regarding this discussion https://github.com/AcademySoftwareFoundation/aswf-sample-project/pull/8#issuecomment-952962520, I prefer to answer your question here:

To smooth integrations, do you think that we can improve the C++ version support in OpenColorIO? -- @hodoulp

I think CMAKE_CXX_STANDARD (and things many things starting with CMAKE_ like CMAKE_CXX_STANDARD_REQUIRED and CMAKE_CXX_EXTENSIONS) should not be set, or even checked in the CMake files, as it is done here:

https://github.com/AcademySoftwareFoundation/OpenColorIO/blob/2d4701e5abc4c95594faae384c6d54fd8b6e3a6b/share/cmake/utils/CppVersion.cmake#L8-L19

I think the quoted code has these 2 issues:

The project will not compile with a C++20-capable compiler, it's looks like an unnecessary limitation, a minimum standard seems more future-proof than a hardcoded list of standards.
```
$ cmake -DCMAKE_CXX_STANDARD=20 ..
CMake Error at CMakeLists.txt:21 (message):
 CMAKE_CXX_STANDARD=20 is unsupported.  Supported standards are: 11, 14, 17.
```
CMake is smart enough to detect the C++ standard the compiler support by defaults or that was set in the CXXFLAGS/CMAKE_CXX_FLAGS, as explained in the top-level message of my PR https://github.com/AcademySoftwareFoundation/aswf-sample-project/pull/8 and in my CMake notes here https://sarcasm.github.io/notes/dev/cmake.html#c-standard.

However, we can see the code will not respect the compiler standard and will instead overwrite with C++11.
```
$ cmake -G Ninja -DCMAKE_CXX_FLAGS=-std=c++20 ..
-- The CXX compiler identification is GNU 11.1.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Setting C++ version to '11' as none was specified.
...
$ ninja -v
[1/2] /usr/bin/c++   -std=c++20 -std=gnu++11 -MD -MT ...
```

So I think the target_compile_features(OpenColorIO PUBLIC cxx_std_11) is the correct an sufficient code mostly. I wouldn't set the CMAKE_CXX_STANDARD bits in the CMake files at all, just in CI.

Regarding the issues with dependencies requiring C++14, if they use the same method, e.g. target_compile_features(oslcomp PUBLIC cxx_std_14), the CMake transitive "contamination" should work ok.

Thanks @Sarcasm , once again I learned something on that topic.

The project will not compile with a C++20-capable compiler, it's looks like an unnecessary limitation, a minimum standard seems more future-proof than a hardcoded list of standards

The library does not officially support C++20 so, the cmake forbids it instead of having issues from contributors. Officially supporting a C++ version means to enhance the CI build system to validate all the supported C++ version on all officially supported platform / compilers. That could increase the challenge to contribute/maintain the library which imposes to be careful around what is officially supported.

On the other hand, the VFX Reference platform guides all the open source projects part of this ecosystem around various aspect of the builds. The goal is to ease the integration in DCCs. But there is currently no policies around the deprecation. Today's situation is then to support C++ 11, 14, and 17 for all supported platforms / compilers and C++20 is not currently in the requirements.

So I think the target_compile_features(OpenColorIO PUBLIC cxx_std_11) is the correct a sufficient code mostly. I wouldn't set the CMAKE_CXX_STANDARD bits in the CMake files at all, just in CI.

That's two different aspects, one sets the C++ version of the library (cap the OpenColorIO target to C++11 feature set only), and one sets the compilation (11, 14, or 17). Both provide a way to compile in C++XX but still impose limitations to the library itself.

Note 1: OSL expects CMAKE_CXX_STANDARD ([with a default to 14](set (CSTD_FLAGS "-std=c++${CMAKE_CXX_STANDARD}"))) and manually set the std version set (CSTD_FLAGS "-std=c++${CMAKE_CXX_STANDARD}")

Note 2: OpenColorIO does not compile for C++20 (macOS 10.15 and AppleClang 12).

Anyway having directly or indirectly -DCMAKE_CXX_STANDARD=11 and -DCMAKE_CXX_FLAGS=-std=c++17 is definitively ambiguous and will have to be addressed in some way.

A solution could be to consistently set CMAKE_CXX_STANDARD and CMAKE_CXX_FLAGS a little bit like OSL (i.e. trap them, and auto adjust them to be consistent) -- or -- fail if inconsistent.

The library does not officially support C++20 so, the cmake forbids it instead of having issues from contributors. Officially supporting a C++ version means to enhance the CI build system to validate all the supported C++ version on all officially supported platform / compilers. That could increase the challenge to contribute/maintain the library which imposes to be careful around what is officially supported.

Ok, it did not occur to me that the fatal error on C++20 was intended to reduce bug reports / not let people think it's officially supported. Still, I think if there was bug reports about C++20, they could be useful even if it's not officially supported.

What do you think of this alternative? Checking the effective standard computed by CMake and warn the user about potential issues:

target_compile_features(OpenColorIO PUBLIC cxx_std_11)

get_property(OCIO_EFFECTIVE_CXX_STANDARD
  TARGET OpenColorIO
  PROPERTY CXX_STANDARD
)
if (NOT OCIO_EFFECTIVE_CXX_STANDARD)
  set(OCIO_EFFECTIVE_CXX_STANDARD ${CMAKE_CXX_STANDARD_DEFAULT})
endif()

set(SUPPORTED_CXX_STANDARDS 11 14 17)
string(REPLACE ";" ", " SUPPORTED_CXX_STANDARDS_STR "${SUPPORTED_CXX_STANDARDS}")
if (NOT OCIO_EFFECTIVE_CXX_STANDARD IN_LIST SUPPORTED_CXX_STANDARDS)
  message(WARNING "C++${OCIO_EFFECTIVE_CXX_STANDARD} is unsupported. Supported standards are: ${SUPPORTED_CXX_STANDARDS_STR}.\n"
    "Hint: use cmake -DCMAKE_CXX_STANDARD=14 ...")
endif()

This produces the following warning when cmake is run.

A solution could be to consistently set CMAKE_CXX_STANDARD and CMAKE_CXX_FLAGS a little bit like OSL (i.e. trap them, and auto adjust them to be consistent) -- or -- fail if inconsistent.

I guess it goes against the point I was trying to make: that "less smart" CMake code, would improve compatibility. Here you suggest to add some more smartness, to try to support the uses cases CMake supports naturally.

CMAKE_CXX_FLAGS is one of these variables starting with CMAKE_ that are nice for packagers, CI and presets, but in the CMake project files there are often more suited alternatives such as target_compile_{features,definitions,options} or some target properties, ...

That's a good idea to authorize any C++ versions (with a warning when it's not officially supported) to have community feedback. But the cmake code must forbid any versions lower than C++11.

But the cmake code must forbid any versions lower than C++11.

That's one of target_compile_features(OpenColorIO PUBLIC cxx_std_17) purposes, you get something like this (tested with C++17, because it's not easy to get a hand on a pre-c++11 compiler):

-- The CXX compiler identification is GNU 4.8.5
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error at CMakeLists.txt:55 (target_compile_features):
  target_compile_features The compiler feature "cxx_std_17" is not known to
  CXX compiler

  "GNU"

  version 4.8.5.

-- Configuring incomplete, errors occurred!
See also "/root/app/build/CMakeFiles/CMakeOutput.log".
See also "/root/app/build/CMakeFiles/CMakeError.log".

Thanks for raising these points @Sarcasm, here is what I got from the discussion so far:

We should remove the strict set(CMAKE_CXX_STANDARD 11) when no CMAKE_CXX_STANDARD is specified as the target_compile_features will already make sure we have at least a C++11 ready compiler,
Replace the CMake FATAL_ERROR by a warning when the standard is not in the supported list to allow package maintainers or others advanced users to use newer language versions if desired,
We could add the latest standard (at least C++20) to the nightly build to make sure we discover new warning / errors early, this would probably be beneficial for the code quality as well, I can confirm what @hodoulp said OCIO, doesn't compile on mac OS 11.6 Clang 13 neither.

On the usage of the 3 CMAKE_CXX_ variants I think it's a good pragmatic approach given that we have lot of targets and also external dependencies built through ExternalProject_Add.

AcademySoftwareFoundation / OpenColorIO

CMake handling of the C++ standard suggestions #1525