[CMake] Check CMAKE_CXX_STANDARD and error if it's to old

[thieta]

Summary

Also nuke the old value out of the cache if it's there, otherwise
it could lead to problems when the project increases the standard
and the developer just runs the "make/ninja" program.

[royjacobson]

Wow, that was quick, thanks!

I played with it a bit locally and it seems to solve this problem. I have one comment about the fatal error, but otherwise LGTM.

llvm/CMakeLists.txt

71

I tried to give it an explicit -DCMAKE_CXX_STANDARD=14 and it will warn and reset it to 17, but not error. It's an improvement over silently using 14 which we do now, but even with the warning it might be slightly surprising? Personally I think you can just get rid of this check.

[barannikov88]

I myself stumbled upon this caching behavior, and although I knew that the minimum required standard has changed, it was surprising to notice that the project is still being built with -std=c++14.
Nevertheless, I don't find this change necessary. The minimum requirement does not change often (is this the second time?).
People will eventually clear the cache, as they once did. Active developers are already aware of the change and it will not take long for them to realize what is the cause of the issue.

I don't have strong objections, only that this slightly complicates already complex build system, and only needed once in a decade.

llvm/CMakeLists.txt

68

This resets the cache variable in the main project, but what about subprojects? Will they also be affected?

[thieta]

In D131367#3705320, @barannikov88 wrote:

I myself stumbled upon this caching behavior, and although I knew that the minimum required standard has changed, it was surprising to notice that the project is still being built with -std=c++14.
Nevertheless, I don't find this change necessary. The minimum requirement does not change often (is this the second time?).
People will eventually clear the cache, as they once did. Active developers are already aware of the change and it will not take long for them to realize what is the cause of the issue.

I don't have strong objections, only that this slightly complicates already complex build system, and only needed once in a decade.

I agree with a lot of this. Maybe the right thing is to just check if CMAKE_CXX_STANDARD is lower than the required and skip the whole cache check. Since this is a user-configurable setting it would be good to make sure we don't hand the user a foot-gun.

[thieta]

llvm/CMakeLists.txt

68

It should be reset for every other subproject that's included with ENABLE_PROJECTS - but with runtimes I am not sure what makes it re-gen the cache there which will be needed.

[thieta]

After a bit of thinking - I think we should merge this as it is now and we can always revert it later in a few weeks if we want - but for now it would self-heal a lot of local setups and bots.

[barannikov88]

In D131367#3706309, @thieta wrote:

After a bit of thinking - I think we should merge this as it is now and we can always revert it later in a few weeks if we want - but for now it would self-heal a lot of local setups and bots.

Fine with me

llvm/CMakeLists.txt

 
 project(LLVM
   VERSION ${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}
   LANGUAGES C CXX ASM)
 
 # Must go after project(..)
 include(GNUInstallDirs)
 
-set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
+# This C++ standard is required to build LLVM.
+set(LLVM_REQUIRED_CXX_STANDARD 17)
+
+# If we find that the cache contains CMAKE_CXX_STANDARD it means that it's a old CMakeCache.txt
+# and we can just inform the user and then reset it.
+if($CACHE{CMAKE_CXX_STANDARD} AND $CACHE{CMAKE_CXX_STANDARD} LESS ${LLVM_REQUIRED_CXX_STANDARD})
+  message(WARNING "Resetting cache value for CMAKE_CXX_STANDARD to ${LLVM_REQUIRED_CXX_STANDARD}")
+  unset(CMAKE_CXX_STANDARD CACHE)
+endif()
+
+# if CMAKE_CXX_STANDARD is still set after the cache unset above it means that the user requested it
+# and we allow it to be set to something newer than the required standard but otherwise we fail.
+if(DEFINED CMAKE_CXX_STANDARD AND CMAKE_CXX_STANDARD LESS ${LLVM_REQUIRED_CXX_STANDARD})
+  message(FATAL_ERROR "Requested CMAKE_CXX_STANDARD=${CMAKE_CXX_STANDARD} which is less than the required ${LLVM_REQUIRED_CXX_STANDARD}.")
+endif()
+
+set(CMAKE_CXX_STANDARD ${LLVM_REQUIRED_CXX_STANDARD} CACHE STRING "C++ standard to conform to")
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
+
 if (CYGWIN)
   # Cygwin is a bit stricter and lack things like 'strdup', 'stricmp', etc in
   # c++xx mode.
   set(CMAKE_CXX_EXTENSIONS YES)
 else()
   set(CMAKE_CXX_EXTENSIONS NO)
 endif()