Break dependency from Core to ObjectFileJIT

[zturner]

Summary

This was responsible for the cycle Core > ObjectFile > Core.

The only reason this dependency was here was so that Module could have a function called CreateJITModule which created things in a special order. Instead of making this specific to creating a jit module, I turn this into a function that can create a module for any type of object file plugin and forwards the arguments through. Since it is not instantiated in Core, the linker (and header file) dependency moves to the point where it is instantiated, which only happens in Expression. Conceptually, this location also happens to make more sense for a dependency on ObjectFileJIT. After all, we JIT expressions so it's no surprise that Expression needs to make use of ObjectFileJIT.

This change reduces the distinct cycle count from 38 to 37

[labath]

The change sounds reasonable to me. I should just point out that this is the Core <-> ObjectFileJIT cycle you are breaking.

lldb/include/lldb/Core/Module.h

165 ↗

(On Diff #148100)

std::forward<Args>(args)...

lldb/source/Expression/IRExecutionUnit.cpp

35–36 ↗

(On Diff #148100)

#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h" should work here

lldb/trunk/include/lldb/Core/Module.h

public:
   Module(
       const FileSpec &file_spec, const ArchSpec &arch,
       const ConstString *object_name = nullptr,
       lldb::offset_t object_offset = 0,
       const llvm::sys::TimePoint<> &object_mod_time = llvm::sys::TimePoint<>());
 
   Module(const ModuleSpec &module_spec);
 
-  static lldb::ModuleSP
-  CreateJITModule(const lldb::ObjectFileJITDelegateSP &delegate_sp);
+  template <typename ObjFilePlugin, typename... Args>
+  static lldb::ModuleSP CreateModuleFromObjectFile(Args &&... args) {
+    // Must create a module and place it into a shared pointer before we can
+    // create an object file since it has a std::weak_ptr back to the module,
+    // so we need to control the creation carefully in this static function
+    lldb::ModuleSP module_sp(new Module());
+    module_sp->m_objfile_sp =
+        std::make_shared<ObjFilePlugin>(module_sp, std::forward<Args>(args)...);
+
+    // Once we get the object file, update our module with the object file's
+    // architecture since it might differ in vendor/os if some parts were
+    // unknown.
+    if (!module_sp->m_objfile_sp->GetArchitecture(module_sp->m_arch))
+      return nullptr;
+
+    return module_sp;
+  }
 
   //------------------------------------------------------------------
   /// Destructor.
   //------------------------------------------------------------------
   ~Module() override;
 
   bool MatchesModuleSpec(const ModuleSpec &module_ref);
 

lldb/trunk/source/Core/CMakeLists.txt

LINK_LIBS
     lldbHost
     lldbInterpreter
     lldbSymbol
     lldbTarget
     lldbUtility
     lldbPluginProcessUtility
     lldbPluginCPlusPlusLanguage
     lldbPluginObjCLanguage
-    lldbPluginObjectFileJIT
     ${LLDB_CURSES_LIBS}
 
   LINK_COMPONENTS
     BinaryFormat
     Support
     Demangle
   )
 
 # Needed to properly resolve references in a debug build.
 # TODO: Remove once we have better layering
 set_target_properties(lldbCore PROPERTIES LINK_INTERFACE_MULTIPLICITY 4)

lldb/trunk/source/Core/Module.cpp

 #include "lldb/Utility/Timer.h"
 
 #if defined(_WIN32)
 #include "lldb/Host/windows/PosixApi.h" // for PATH_MAX
 #endif
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 #include "Plugins/Language/ObjC/ObjCLanguage.h"
-#include "Plugins/ObjectFile/JIT/ObjectFileJIT.h"
 
 #include "llvm/ADT/STLExtras.h"    // for make_unique
 #include "llvm/Support/Compiler.h" // for LLVM_PRETT...
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h" // for raw_string...
 
 #include <assert.h>    // for assert
uint32_t Module::GetVersion(uint32_t *versions, uint32_t num_versions) {
 
   if (versions != nullptr && num_versions != 0) {
     for (uint32_t i = 0; i < num_versions; ++i)
       versions[i] = LLDB_INVALID_MODULE_VERSION;
   }
   return 0;
 }
 
-ModuleSP
-Module::CreateJITModule(const lldb::ObjectFileJITDelegateSP &delegate_sp) {
-  if (delegate_sp) {
-    // Must create a module and place it into a shared pointer before we can
-    // create an object file since it has a std::weak_ptr back to the module,
-    // so we need to control the creation carefully in this static function
-    ModuleSP module_sp(new Module());
-    module_sp->m_objfile_sp =
-        std::make_shared<ObjectFileJIT>(module_sp, delegate_sp);
-    if (module_sp->m_objfile_sp) {
-      // Once we get the object file, update our module with the object file's
-      // architecture since it might differ in vendor/os if some parts were
-      // unknown.
-      module_sp->m_objfile_sp->GetArchitecture(module_sp->m_arch);
-    }
-    return module_sp;
-  }
-  return ModuleSP();
-}
-
 bool Module::GetIsDynamicLinkEditor() {
   ObjectFile *obj_file = GetObjectFile();
 
   if (obj_file)
     return obj_file->GetIsDynamicLinkEditor();
 
   return false;
 }

lldb/trunk/source/Expression/CMakeLists.txt

add_lldb_library(lldbExpression
   LINK_LIBS
     lldbCore
     lldbHost
     lldbInterpreter
     lldbSymbol
     lldbTarget
     lldbUtility
     lldbPluginExpressionParserClang
+    lldbPluginObjectFileJIT
 
   LINK_COMPONENTS
     Core
     ExecutionEngine
     Support
   )

lldb/trunk/source/Expression/IRExecutionUnit.cpp

 #include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 
 #include "lldb/../../source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "lldb/../../source/Plugins/ObjectFile/JIT/ObjectFileJIT.h"
 
 using namespace lldb_private;
 
 IRExecutionUnit::IRExecutionUnit(std::unique_ptr<llvm::LLVMContext> &context_ap,
                                  std::unique_ptr<llvm::Module> &module_ap,
                                  ConstString &name,
                                  const lldb::TargetSP &target_sp,
                                  const SymbolContext &sym_ctx,
bool IRExecutionUnit::GetArchitecture(lldb_private::ArchSpec &arch) {
   else
     arch.Clear();
   return arch.IsValid();
 }
 
 lldb::ModuleSP IRExecutionUnit::GetJITModule() {
   ExecutionContext exe_ctx(GetBestExecutionContextScope());
   Target *target = exe_ctx.GetTargetPtr();
-  if (target) {
-    lldb::ModuleSP jit_module_sp = lldb_private::Module::CreateJITModule(
-        std::static_pointer_cast<lldb_private::ObjectFileJITDelegate>(
-            shared_from_this()));
-    if (jit_module_sp) {
-      bool changed = false;
-      jit_module_sp->SetLoadAddress(*target, 0, true, changed);
-    }
-    return jit_module_sp;
-  }
-  return lldb::ModuleSP();
-}
+  if (!target)
+    return nullptr;
+
+  auto Delegate = std::static_pointer_cast<lldb_private::ObjectFileJITDelegate>(
+      shared_from_this());
+
+  lldb::ModuleSP jit_module_sp =
+      lldb_private::Module::CreateModuleFromObjectFile<ObjectFileJIT>(Delegate);
+  if (!jit_module_sp)
+    return nullptr;
+