[CMake] Make -fmodules-local-submodule-visibility optional.

[aprantl]

Summary

On Darwin it is currently impossible to build LLVM with modules because the Darwin system module map is not compatible with -fmodules-local-submodule-visibility at this point in time.
This patch makes the flag optional and off by default on Darwin so it becomes possible to build LLVM with modules again.

[aprantl]

I forgot to mention that this patch also turns on the necessary "-fcxx-modules" on Darwin.

It's probably a good idea to ask more specific questions to the reviewers:

@beanz: Does my CMake copypasta make sense?
@bruno: Are you ok with this as an intermediate step on Darwin?
@rsmith: Let me know if there are any caveats from the Linux side — the intention is that this is NFC on Linux.

[beanz]

I have one minor suggestion that would make the code cleaner.

Right now, you construct the module flags both before and after the check. What if instead you put the following code under the if(LLVM_ENABLE_MODULES) line:

set(module_flags "-fmodules -Xclang -fmodules-cache-path=module.cache")
if (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
  # On Darwin -fmodules does not imply -fcxx-modules.
  set(module_flags "${module_flags} -fcxx-modules")
endif()
if (LLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY)
  set(module_flags "${module_flags} -fmodules-local-submodule-visibility")
endif()

Then you can replace the first place you construct the modules flags with:

set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} ${module_flags}")

And the second place you construct the flags with just:

append("${module_flags}" CMAKE_CXX_FLAGS)

-Chris

[aprantl]

Update patch according to beanz' suggestion. Much cleaner.

thanks!

[beanz]

CMake code LGTM.

[bruno]

Hi Adrian,

This LGTM as an intermediate step on Darwin! :-)

[aprantl]

Thanks! Closing now.

CMakeLists.txt

set(LLVM_TARGETS_TO_BUILD
    ${LLVM_EXPERIMENTAL_TARGETS_TO_BUILD})
 list(REMOVE_DUPLICATES LLVM_TARGETS_TO_BUILD)
 
 include(AddLLVMDefinitions)
 
 option(LLVM_ENABLE_PIC "Build Position-Independent Code" ON)
 option(LLVM_ENABLE_WARNINGS "Enable compiler warnings." ON)
 option(LLVM_ENABLE_MODULES "Compile with C++ modules enabled." OFF)
+if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
+  option(LLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY "Compile with -fmodules-local-submodule-visibility." OFF)
+else()
+  option(LLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY "Compile with -fmodules-local-submodule-visibility." ON)
+endif()
 option(LLVM_ENABLE_CXX1Y "Compile with C++1y enabled." OFF)
 option(LLVM_ENABLE_LIBCXX "Use libc++ if available." OFF)
 option(LLVM_ENABLE_LIBCXXABI "Use libc++abi when using libc++." OFF)
 option(LLVM_ENABLE_PEDANTIC "Compile with pedantic enabled." ON)
 option(LLVM_ENABLE_WERROR "Fail and stop if a warning is triggered." OFF)
 
 if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
   option(LLVM_ENABLE_ASSERTIONS "Enable assertions" OFF)

cmake/modules/HandleLLVMOptions.cmake

if (CXX_SUPPORTS_CXX11)
         append("-std=c++11" CMAKE_CXX_FLAGS)
       endif()
     else()
       message(FATAL_ERROR "LLVM requires C++11 support but the '-std=c++11' flag isn't supported.")
     endif()
   endif()
   if (LLVM_ENABLE_MODULES)
     set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
-    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fmodules -Xclang -fmodules-local-submodule-visibility -fmodules-cache-path=module.cache")
+    set(module_flags "-fmodules -Xclang -fmodules-cache-path=module.cache")
+    if (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
+      # On Darwin -fmodules does not imply -fcxx-modules.
+      set(module_flags "${module_flags} -fcxx-modules")
+    endif()
+    if (LLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY)
+      set(module_flags "${module_flags} -fmodules-local-submodule-visibility")
+    endif()
+    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} ${module_flags}")
+
     # Check that we can build code with modules enabled, and that repeatedly
     # including <cassert> still manages to respect NDEBUG properly.
     CHECK_CXX_SOURCE_COMPILES("#undef NDEBUG
                                #include <cassert>
                                #define NDEBUG
                                #include <cassert>
                                int main() { assert(this code is not compiled); }"
                                CXX_SUPPORTS_MODULES)
     set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
     if (CXX_SUPPORTS_MODULES)
-      append_if(CXX_SUPPORTS_MODULES "-fmodules -Xclang -fmodules-local-submodule-visibility -fmodules-cache-path=module.cache" CMAKE_CXX_FLAGS)
+      append("${module_flags}" CMAKE_CXX_FLAGS)
     else()
       message(FATAL_ERROR "LLVM_ENABLE_MODULES is not supported by this compiler")
     endif()
   endif(LLVM_ENABLE_MODULES)
 endif( MSVC )
 
 macro(append_common_sanitizer_flags)
   if (NOT MSVC)