[PATCH] D56799: [NFC] Factor out + document build requirements

JF Bastien via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 16 13:40:48 PST 2019


jfb marked 5 inline comments as done.
jfb added inline comments.


================
Comment at: cmake/modules/CheckCompilerVersion.cmake:11-12
+set(CLANG_WARN 3.1)
+set(APPLECLANG_MIN 3.1)
+set(APPLECLANG_WARN 3.1)
+set(MSVC_MIN 19.0)
----------------
erichkeane wrote:
> smeenai wrote:
> > Did you really mean these to be the same as CLANG_MIN and CLANG_WARN?
> According to this: https://gist.github.com/yamaya/2924292 those are the same versions.  AppleClang 3.1 IS Clang 3.1.
Right, this is on purpose. The divergence in version numbers came later.


================
Comment at: cmake/modules/CheckCompilerVersion.cmake:17
+function(check_compiler_version NAME NICE_NAME MINIMUM_VERSION WARN_VERSION)
+  if(CMAKE_CXX_COMPILER_ID STREQUAL NAME)
+    if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS MINIMUM_VERSION)
----------------
smeenai wrote:
> You could invert this condition and do an early return instead, which would reduce the remainder of the function by one indent level.
Much nicer, thanks!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56799





More information about the llvm-commits mailing list