Add modules support for lldb headers in include/

[teemperor]

Summary

This patch adds a modulemap which allows compiling the lldb headers into C++ modules
(for example in builds with LLVM_ENABLE_MODULES=On).

Even though most of the affected code has been cleaned up to work with the more strict
C++ module semantics, there are still some workarounds left in the current modulemap
(the most obvious one is the big lldb wrapper module).

It also moves the Obj-C++ files in lldb to their own subdirectories. This was necessary
because we need to filter out the modules flags for this code.

Note: With the latest clang and libstdc++ it seems necessary to have a STL C++ module
to get a working LLVM_ENABLE_MODULES build for lldb. Otherwise clang will falsely
detect ODR violations in the textually included STL code inside the lldb modules.

[teemperor]

I generated a report for the remaining cyclic dependencies in the lldb module here.

Note that I'll provide a modulemap for the source/ headers in another patch, so this is out of scope for this review. The reason is that this modulemap seems to barely affect compilation time and is quite verbose (as I can't use umbrellas with the mixed source/header files in the same directory).

[aprantl]

That's awesome! May I ask why you chose to use lldb_Component modules instead of doing submodules? Is this for build performance?

[bruno]

Very nice!

I generated a report for the remaining cyclic dependencies in the lldb module here.

Do you plan to fix headers at some point to break out the dependencies?

include/lldb/module.modulemap

67

Will this trick be enough for local submodules visibility mode as well?

[teemperor]

@aprantl It's good for build times and more importantly it's consistent with the way Clang/LLVM are naming/organizing their modules. But I actually don't have a strong opinion on how the modules are named/organized.

@bruno Breaking up the lldb module in the future is the plan. I'm just not sure when I get around to do that. But the modulemap in the current state should bring most of the module benefits to LLDB (build time, better code checks), so I just opened a review for it.

Someone else with more LLDB experience could also break the cyclic dependencies. The report I linked should point out quite well what exactly the problematic includes are, so that person wouldn't even need to enable modules for that.

include/lldb/module.modulemap

67

From my (very limited) experience with disabled local submodule visibility I would say yes, but I can't test it on my current system (compilation already fails at the first LLVM module without LSV).

[teemperor]

• Removed some inconsistent white space.

[bruno]

LGTM

[teemperor]

It seems the compilation fails on OS X with this modulemap (or any modulemap). The reason seems to be that on OS X we compile Obj-C++ as part of lldb which assumes that Clang/LLVM is valid Obj-C++. However, the Obj-C parsing code in Clang is *not* valid Obj-C++ as some parts of the code use variables named like IBAction for describing, well, IBAction in Obj-C.

So we either try to disable modules for the Obj-C++ code or we make Clang's source code Obj-C++ compatible.

[teemperor]

• All Obj-C files are now in their own subdirectory. This way we can filter out the modules flags for them.

[teemperor]

include/lldb/module.modulemap

67

It seems to work!

[aprantl]

source/Host/CMakeLists.txt

7

ObjC++ or Objective C++

107

This is really Objective C++, so ObjCXX or objcxx would be a more appropriate directory name.

109

macosx

(technically MacOS X is now macOS, but you don't need to change that)

[teemperor]

• Obj-C -> Obj-C++

[teemperor]

• Fixed a typo.

[aprantl]

source/Host/CMakeLists.txt

7

s/Obj-C++/Objective C++/

8

s/Obj-C++/Objective C++/

108

either ...objcxx or ...ObjCXX

[teemperor]

• The regex that removes the modules cache now actually removes the whole flag + path.
• Fixed more typos.

[teemperor]

• Finally found the one and only way to spell "Objective-C++"

[aprantl]

I'm getting a new warning now, can you also reproduce this?

In file included from <module-includes>:21:
In file included from ../tools/lldb/include/lldb/Host/MainLoop.h:13:
tools/lldb/include/lldb/Host/Config.h:33:9: warning: 'HAVE_LIBCOMPRESSION' macro redefined [-Wmacro-redefined]
#define HAVE_LIBCOMPRESSION
        ^
<command line>:1:9: note: previous definition is here
#define HAVE_LIBCOMPRESSION 1
        ^
1 warning generated.

include/lldb/module.modulemap

+
+module lldb_API {
+  requires cplusplus
+
+  umbrella "API"
+  module * { export * }
+}
+
+module lldb_Host {
+  requires cplusplus
+
+  // Because we have OS-specific headers in Host, we just list
+  // all OS-independent headers here that will include the correct
+  // OS-specific header for us.
+  module ConnectionFileDescriptor { header "Host/ConnectionFileDescriptor.h" export * }
+  module Debug { header "Host/Debug.h" export * }
+  module Editline { header "Host/Editline.h" export * }
+  module FileCache { header "Host/FileCache.h" export * }
+  module File { header "Host/File.h" export * }
+  module FileSystem { header "Host/FileSystem.h" export * }
+  module HostGetOpt { header "Host/HostGetOpt.h" export * }
+  module Host { header "Host/Host.h" export * }
+  module HostInfoBase { header "Host/HostInfoBase.h" export * }
+  module HostInfo { header "Host/HostInfo.h" export * }
+  module HostNativeProcessBase { header "Host/HostNativeProcessBase.h" export * }
+  module HostNativeProcess { header "Host/HostNativeProcess.h" export * }
+  module HostNativeThreadBase { header "Host/HostNativeThreadBase.h" export * }
+  module HostNativeThreadForward { header "Host/HostNativeThreadForward.h" export * }
+  module HostNativeThread { header "Host/HostNativeThread.h" export * }
+  module HostProcess { header "Host/HostProcess.h" export * }
+  module HostThread { header "Host/HostThread.h" export * }
+  module LockFileBase { header "Host/LockFileBase.h" export * }
+  module LockFile { header "Host/LockFile.h" export * }
+  module MainLoopBase { header "Host/MainLoopBase.h" export * }
+  module MainLoop { header "Host/MainLoop.h" export * }
+  module MonitoringProcessLauncher { header "Host/MonitoringProcessLauncher.h" export * }
+  module OptionParser { header "Host/OptionParser.h" export * }
+  module PipeBase { header "Host/PipeBase.h" export * }
+  module Pipe { header "Host/Pipe.h" export * }
+  module PosixApi { header "Host/PosixApi.h" export * }
+  module Predicate { header "Host/Predicate.h" export * }
+  module ProcessLauncher { header "Host/ProcessLauncher.h" export * }
+  module ProcessRunLock { header "Host/ProcessRunLock.h" export * }
+  module PseudoTerminal { header "Host/PseudoTerminal.h" export * }
+  module SocketAddress { header "Host/SocketAddress.h" export * }
+  module Socket { header "Host/Socket.h" export * }
+  module StringConvert { header "Host/StringConvert.h" export * }
+  module Symbols { header "Host/Symbols.h" export * }
+  module TaskPool { header "Host/TaskPool.h" export * }
+  module Terminal { header "Host/Terminal.h" export * }
+  module ThreadLauncher { header "Host/ThreadLauncher.h" export * }
+  module Time { header "Host/Time.h" export * }
+  module XML { header "Host/XML.h" export * }
+
+  export *
+}
+
+module lldb_Initialization {
+  requires cplusplus
+
+  umbrella "Initialization"
+  module * { export * }
+}
+
+
+module lldb_Wrapper {
+
+  module lldb_Breakpoint {
+    requires cplusplus
+
+    umbrella "Breakpoint"
+    module * { export * }
+  }
+
+  module lldb_Core {
+    requires cplusplus
+
+    umbrella "Core"
+    module * { export * }
+  }
+
+  module lldb_DataFormatters {
+    requires cplusplus
+
+    umbrella "DataFormatters"
+    module * { export * }
+  }
+
+  module lldb_Expression {
+    requires cplusplus
+
+    umbrella "Expression"
+    module * { export * }
+    // TODO: This file includes a non-public header.
+    exclude header "Expression/REPL.h"
+  }
+
+  module lldb_Interpreter {
+    requires cplusplus
+
+    umbrella "Interpreter"
+    module * { export * }
+  }
+
+  module lldb_Symbol {
+    requires cplusplus
+
+    umbrella "Symbol"
+    module * { export * }
+  }
+  module lldb_Target {
+    requires cplusplus
+
+    umbrella "Target"
+    module * { export * }
+  }
+}
+
+
+module lldb_Utility {
+  requires cplusplus
+
+  umbrella "Utility"
+  module * { export * }
+
+  module lldb_defines { header "lldb-defines.h" export * }
+  module lldb_enumerations { header "lldb-enumerations.h" export * }
+  module lldb_forward { header "lldb-forward.h" export * }
+  module lldb_private_enumerations { header "lldb-private-enumerations.h" export * }
+  module lldb_private_defines { header "lldb-private-defines.h" export * }
+  module lldb_private_forward { header "lldb-private-forward.h" export * }
+  module lldb_private { header "lldb-private.h" export * }
+  module lldb_private_interfaces { header "lldb-private-interfaces.h" export * }
+  module lldb_private_types { header "lldb-private-types.h" export * }
+  module lldb_public { header "lldb-public.h" export * }
+  module lldb_types { header "lldb-types.h" export * }
+  module lldb_versioning { header "lldb-versioning.h" export * }
+
+}

source/Host/CMakeLists.txt

 macro(add_host_subdirectory group)
   list(APPEND HOST_SOURCES ${ARGN})
   source_group(${group} FILES ${ARGN})
 endmacro()
 
+# Removes all module flags from the current CMAKE_CXX_FLAGS. Used for
+# the Obj-C++ code in lldb which we don't want to build with modules.
+# Reasons for this are that modules with Obj-C++ would require that all
+# LLVM/Clang modules are Obj-C++ compatible (which they are most likely
+# not) and we would have rebuild a second set of modules just for the few
+# Obj-C++ files in lldb (which slows down the build process).
+macro(remove_module_flags)
+  string(REGEX REPLACE "-fmodules-cache-path=[\\S]*" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+  string(REGEX REPLACE "-fmodules" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+  string(REGEX REPLACE "-fcxx-modules" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+endmacro()
+
 add_host_subdirectory(common
   common/File.cpp
   common/FileCache.cpp
   common/FileSystem.cpp
   common/GetOptInc.cpp
   common/Host.cpp
   common/HostInfoBase.cpp
   common/HostNativeThreadBase.cpp
add_host_subdirectory(posix
     posix/HostThreadPosix.cpp
     posix/LockFilePosix.cpp
     posix/PipePosix.cpp
     posix/ProcessLauncherPosixFork.cpp
     )
 
   if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
     include_directories(SYSTEM ${LIBXML2_INCLUDE_DIR})
-    add_host_subdirectory(macosx
-      macosx/Host.mm
-      macosx/HostInfoMacOSX.mm
-      macosx/HostThreadMacOSX.mm
+    add_subdirectory(macosx/objc)
+    set(LLDBObjCLibs lldbHostMacOSXObjC)
+    add_host_subdirectory(maqcosx
       macosx/Symbols.cpp
       macosx/cfcpp/CFCBundle.cpp
       macosx/cfcpp/CFCData.cpp
       macosx/cfcpp/CFCMutableArray.cpp
       macosx/cfcpp/CFCMutableDictionary.cpp
       macosx/cfcpp/CFCMutableSet.cpp
       macosx/cfcpp/CFCString.cpp
       )
if (HAVE_LIBDL)
   list(APPEND EXTRA_LIBS ${CMAKE_DL_LIBS})
 endif()
 if (NOT LLDB_DISABLE_LIBEDIT)
   list(APPEND EXTRA_LIBS edit)
 endif()
 
 add_lldb_library(lldbHost
   ${HOST_SOURCES}
-  
   LINK_LIBS
     lldbCore
     lldbSymbol
     lldbTarget
     lldbUtility
     ${LLDB_PLUGINS}
     ${EXTRA_LIBS}
-  
+    ${LLDBObjCLibs}
   LINK_COMPONENTS
     Object
     Support
   )

source/Host/common/Terminal.cpp

 //===-- Terminal.cpp --------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/Terminal.h"
 
+#include "lldb/Host/Config.h"
 #include "lldb/Host/PosixApi.h"
 #include "llvm/ADT/STLExtras.h"
 
 #include <fcntl.h>
 #include <signal.h>
 
 #ifdef LLDB_CONFIG_TERMIOS_SUPPORTED
 #include <termios.h>

source/Host/macosx/objc/CMakeLists.txt

+
+remove_module_flags()
+include_directories(..)
+
+add_lldb_library(lldbHostMacOSXObjC
+  Host.mm
+  HostInfoMacOSX.mm
+  HostThreadMacOSX.mm
+
+  LINK_LIBS
+    lldbCore
+    lldbSymbol
+    lldbTarget
+    lldbUtility
+    ${LLDB_PLUGINS}
+    ${EXTRA_LIBS}
+
+  LINK_COMPONENTS
+    Object
+    Support
+  )

source/Plugins/Platform/MacOSX/CMakeLists.txt

 list(APPEND PLUGIN_PLATFORM_MACOSX_SOURCES
   PlatformDarwin.cpp
   PlatformDarwinKernel.cpp
   PlatformMacOSX.cpp
   PlatformRemoteiOS.cpp
   PlatformRemoteAppleTV.cpp
   PlatformRemoteAppleWatch.cpp
   PlatformRemoteDarwinDevice.cpp
   )
 
 list(APPEND PLUGIN_PLATFORM_MACOSX_DARWIN_ONLY_SOURCES
   PlatformAppleSimulator.cpp
   PlatformiOSSimulator.cpp
-  PlatformiOSSimulatorCoreSimulatorSupport.mm
   PlatformAppleTVSimulator.cpp
   PlatformAppleWatchSimulator.cpp
   )
 
 if(CMAKE_SYSTEM_NAME MATCHES "Darwin")
   include_directories(${LIBXML2_INCLUDE_DIR})
+  add_subdirectory(objc)
+  set(OBJC_LIBS "lldbPluginPlatformMacOSXObjC")
   list(APPEND PLUGIN_PLATFORM_MACOSX_SOURCES
     ${PLUGIN_PLATFORM_MACOSX_DARWIN_ONLY_SOURCES})
 else()
   list(APPEND LLVM_OPTIONAL_SOURCES
     ${PLUGIN_PLATFORM_MACOSX_DARWIN_ONLY_SOURCES})
 endif()
 
 add_lldb_library(lldbPluginPlatformMacOSX PLUGIN
   ${PLUGIN_PLATFORM_MACOSX_SOURCES}
   
   LINK_LIBS
     clangBasic
     lldbBreakpoint
     lldbCore
     lldbHost
     lldbInterpreter
     lldbSymbol
     lldbTarget
     lldbUtility
     lldbPluginPlatformPOSIX
+    ${OBJC_LIBS}
   LINK_COMPONENTS
     Support
     )

source/Plugins/Platform/MacOSX/objc/CMakeLists.txt

+remove_module_flags()
+include_directories(..)
+
+add_lldb_library(lldbPluginPlatformMacOSXObjC
+  PlatformiOSSimulatorCoreSimulatorSupport.mm
+
+  LINK_LIBS
+    lldbCore
+    lldbSymbol
+    lldbTarget
+    lldbUtility
+    ${LLDB_PLUGINS}
+    ${EXTRA_LIBS}
+
+  LINK_COMPONENTS
+    Object
+    Support
+  )