[PATCH] D47073: Document and Enforce new Host Compiler Policy

[JF Bastien via Phabricator via llvm-commits]

jfb added inline comments.


================
Comment at: CMakeLists.txt:28
+  set(LLVM_IGNORE_HOST_VERSION_CHECK OFF)
+endif()
 
----------------
"HOST_COMPILER" instead of "HOST".


================
Comment at: cmake/modules/CheckCompilerVersion.cmake:5
+# previous version was branched. Additionally, a CMake warning is issued for
+# versions older than 1.5 years. Compiler version checking for GCC/Clang can be
+# ignored with cmake option LLVM_IGNORE_HOST_VERISON_CHECK, 
----------------
This is still pretty dogmatic:

> For GCC and Clang, LLVM's guideline is to support all
> major compiler versions released within 3 years from the date in which the
> previous version was branched. Additionally, a CMake warning is issued for
> versions older than 1.5 years.

I'd say:

> For GCC and Clang, LLVM's rough guideline is to support major compiler versions released about 3 years from the date in which the previous versions was branched. This time-based guideline isn't strict: we want to keep some discretion when supporting older compilers still makes sense, or when newer compilers inflict significant pain.
> Additionally, a CMake warning is issued for versions older than roughly 1.5 years.


================
Comment at: cmake/modules/CheckCompilerVersion.cmake:6
+# versions older than 1.5 years. Compiler version checking for GCC/Clang can be
+# ignored with cmake option LLVM_IGNORE_HOST_VERISON_CHECK, 
+#
----------------
I'd move this to the error message, not the comment.


================
Comment at: cmake/modules/CheckCompilerVersion.cmake:23
+      if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS GCC_MIN AND NOT LLVM_IGNORE_HOST_VERSION_CHECK)
+        message(FATAL_ERROR "GCC ${CMAKE_CXX_COMPILER_VERSION} is unsupported (minimum version is GCC ${GCC_MIN}).")
+      elseif(CMAKE_CXX_COMPILER_VERSION VERSION_LESS GCC_MIN)
----------------
Move the instructions on silencing this error here.


================
Comment at: cmake/modules/CheckCompilerVersion.cmake:31
+      if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS CLANG_MIN AND NOT LLVM_IGNORE_HOST_VERSION_CHECK)
+        message(FATAL_ERROR "Clang ${CMAKE_CXX_COMPILER_VERSION} is unsupported (minimum version is Clang ${CLANG_MIN}).")
+      elseif(CMAKE_CXX_COMPILER_VERSION VERSION_LESS CLANG_MIN)
----------------
Ditto.


================
Comment at: docs/CMake.rst:578
+  If enabled, the CMake checks for Clang and GCC minimum host compiler versions
+  will be ignored. This defaults to OFF.
+
----------------
Also say: this flag will be removed for LLVM 9, since we'll likely bump the required C++ version.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47073/new/

https://reviews.llvm.org/D47073