shared_ptr-ize the llvm::Module*.

This is in prepare for the upcoming llvm upgrade. The future orc jit compile
layer needs a std::shared_ptr<llvm::Module>. The current design passes a
llvm::Module* around and any conversions to a shared_ptr cause the
destruction of the llvm::Module which is a long-lived object in cling.

include/cling/Interpreter/Transaction.h

@@ -149,7 +149,7 @@ namespace cling {
 
     ///\brief The llvm Module containing the information that we will revert
     ///
-    std::unique_ptr<llvm::Module> m_Module;
+    std::shared_ptr<llvm::Module> m_Module;
 
     ///\brief The JIT handle allowing a removal of the Transaction's symbols.
     ///
@@ -463,8 +463,8 @@ namespace cling {
         m_NestedTransactions->clear();
     }
 
-    llvm::Module* getModule() const { return m_Module.get(); }
+    std::shared_ptr<llvm::Module> getModule() const { return m_Module; }
-    void setModule(std::unique_ptr<llvm::Module> M) { m_Module.swap(M); }
+    void setModule(std::unique_ptr<llvm::Module> M) { m_Module = std::move(M); }
 
     ExeUnloadHandle getExeUnloadHandle() const { return m_ExeUnload; }
     IncrementalExecutor* getExecutor() const { return m_Exe; }

lib/Interpreter/DeclUnloader.cpp

@@ -867,7 +867,7 @@ bool DeclUnloader::VisitRedeclarable(clang::Redeclarable<T>* R, DeclContext* DC)
         }
       }
 
-      llvm::Module* M = m_CurTransaction->getModule();
+      auto M = m_CurTransaction->getModule();
       GlobalValue* GV = M->getNamedValue(mangledName);
       if (GV) { // May be deferred decl and thus 0
         GlobalValueEraser GVEraser(m_CodeGen);

lib/Interpreter/IncrementalExecutor.cpp

@@ -125,7 +125,7 @@ void IncrementalExecutor::shuttingDown() {
 }
 
 void IncrementalExecutor::AddAtExitFunc(void (*func)(void*), void* arg,
-                                        llvm::Module* M) {
+                                       const std::shared_ptr<llvm::Module>& M) {
   // Register a CXAAtExit function
   cling::internal::SpinLockGuard slg(m_AtExitFuncsSpinLock);
   m_AtExitFuncs[M].emplace_back(func, arg);
@@ -203,8 +203,8 @@ freeCallersOfUnresolvedSymbols(llvm::SmallVectorImpl<llvm::Function*>&
 
 IncrementalExecutor::ExecutionResult
 IncrementalExecutor::runStaticInitializersOnce(const Transaction& T) {
-  llvm::Module* m = T.getModule();
+  auto m = T.getModule();
-  assert(m && "Module must not be null");
+  assert(m.get() && "Module must not be null");
 
   // We don't care whether something was unresolved before.
   m_unresolvedSymbols.clear();

lib/Interpreter/IncrementalExecutor.h

@@ -116,7 +116,9 @@ namespace cling {
     ///\brief Function registered via __cxa_atexit, atexit, or one of
     /// it's C++ overloads that should be run when a module is unloaded.
     ///
-    typedef utils::OrderedMap<llvm::Module*, std::vector<CXAAtExitElement>>
+    // FIXME: We should probably try using a weak_ptr instead of a shared_ptr.
+    typedef utils::OrderedMap<std::shared_ptr<llvm::Module>,
+                              std::vector<CXAAtExitElement>>
         AtExitFunctions;
     AtExitFunctions m_AtExitFuncs;
 
@@ -171,8 +173,10 @@ namespace cling {
     }
 
     ///\brief Unload a set of JIT symbols.
-    bool unloadFromJIT(llvm::Module* M, Transaction::ExeUnloadHandle H) {
+    bool unloadFromJIT(const std::shared_ptr<llvm::Module>& M,
-      auto iMod = std::find(m_ModulesToJIT.begin(), m_ModulesToJIT.end(), M);
+                       Transaction::ExeUnloadHandle H) {
+      auto iMod =
+          std::find(m_ModulesToJIT.begin(), m_ModulesToJIT.end(), M.get());
       if (iMod != m_ModulesToJIT.end())
         m_ModulesToJIT.erase(iMod);
       else
@@ -256,7 +260,8 @@ namespace cling {
 
     ///\brief Keep track of the entities whose dtor we need to call.
     ///
-    void AddAtExitFunc(void (*func) (void*), void* arg, llvm::Module* M);
+    void AddAtExitFunc(void (*func)(void*), void* arg,
+                       const std::shared_ptr<llvm::Module>& M);
 
     ///\brief Try to resolve a symbol through our LazyFunctionCreators;
     /// print an error message if that fails.

lib/Interpreter/Interpreter.cpp

@@ -125,11 +125,9 @@ namespace cling {
       // we need a transaction.
       PushTransactionRAII pushedT(i);
 
-      m_State.reset(new ClangInternalState(CI.getASTContext(),
+      m_State.reset(
-                                           CI.getPreprocessor(),
+          new ClangInternalState(CI.getASTContext(), CI.getPreprocessor(),
-                                           CG ? CG->GetModule() : 0,
+                                 CG ? CG->GetModule() : nullptr, CG, "aName"));
-                                           CG,
-                                           "aName"));
     }
   }
 
@@ -251,7 +249,7 @@ namespace cling {
     // and overrides.
     if (!isInSyntaxOnlyMode()) {
       if (const Transaction* T = getLastTransaction()) {
-        if (llvm::Module* M = T->getModule()) {
+        if (auto M = T->getModule()) {
           for (const llvm::StringRef& Sym : Syms) {
             const llvm::GlobalValue* GV = M->getNamedValue(Sym);
   #if defined(__GLIBCXX__) && !defined(__APPLE__)
@@ -532,11 +530,9 @@ namespace cling {
     // This may induce deserialization
     PushTransactionRAII RAII(this);
     CodeGenerator* CG = m_IncrParser->getCodeGenerator();
-    ClangInternalState* state
+    ClangInternalState* state = new ClangInternalState(
-      = new ClangInternalState(getCI()->getASTContext(),
+        getCI()->getASTContext(), getCI()->getPreprocessor(),
-                               getCI()->getPreprocessor(),
+        getLastTransaction()->getModule().get(), CG, name);
-                               getLastTransaction()->getModule(),
-                               CG, name);
     m_StoredStates.push_back(state);
   }
 
@@ -696,7 +692,7 @@ namespace cling {
 
     // Copied from PPDirectives.cpp
     SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> path;
-    for (Module *mod = suggestedModule.getModule(); mod; mod = mod->Parent) {
+    for (auto mod = suggestedModule.getModule(); mod; mod = mod->Parent) {
       IdentifierInfo* II
         = &getSema().getPreprocessor().getIdentifierTable().get(mod->Name);
       path.push_back(std::make_pair(II, fileNameLoc));
@@ -1282,10 +1278,10 @@ namespace cling {
   void Interpreter::unload(Transaction& T) {
     // Clear any stored states that reference the llvm::Module.
     // Do it first in case
-    llvm::Module* const Module = T.getModule();
+    auto Module = T.getModule();
     if (Module && !m_StoredStates.empty()) {
       const auto Predicate = [&Module](const ClangInternalState* S) {
-        return S->getModule() == Module;
+        return S->getModule() == Module.get();
       };
       auto Itr =
           std::find_if(m_StoredStates.begin(), m_StoredStates.end(), Predicate);
@@ -1482,13 +1478,13 @@ namespace cling {
     assert(!isInSyntaxOnlyMode() && "Running on what?");
     assert(T.getState() == Transaction::kCommitted && "Must be committed");
 
-    llvm::Module* const M = T.getModule();
+    std::shared_ptr<const llvm::Module> M = T.getModule();
     if (!M)
       return Interpreter::kExeNoModule;
 
     IncrementalExecutor::ExecutionResult ExeRes
        = IncrementalExecutor::kExeSuccess;
-    if (!isPracticallyEmptyModule(M)) {
+    if (!isPracticallyEmptyModule(M.get())) {
       T.setExeUnloadHandle(m_Executor.get(), m_Executor->emitToJIT());
 
       // Forward to IncrementalExecutor; should not be called by

lib/Interpreter/Transaction.cpp

@@ -51,6 +51,7 @@ namespace cling {
   }
 
   Transaction::~Transaction() {
+    assert(m_Module.use_count() <= 1 && "There is still a reference!");
     if (hasNestedTransactions())
       for (size_t i = 0; i < m_NestedTransactions->size(); ++i) {
         assert(((*m_NestedTransactions)[i]->getState() == kCommitted

lib/Interpreter/TransactionUnloader.cpp

@@ -147,7 +147,8 @@ namespace cling {
     return cling::UnloadDecl(m_Sema, m_CodeGen, D);
   }
 
-  bool TransactionUnloader::unloadModule(llvm::Module* M) {
+  bool
+  TransactionUnloader::unloadModule(const std::shared_ptr<llvm::Module>& M) {
     for (auto& Func: M->functions())
       m_CodeGen->forgetGlobal(&Func);
     for (auto& Glob: M->globals())

lib/Interpreter/TransactionUnloader.h

@@ -10,6 +10,8 @@
 #ifndef CLING_TRANSACTION_UNLOADER
 #define CLING_TRANSACTION_UNLOADER
 
+#include <memory>
+
 namespace llvm {
   class Module;
 }
@@ -40,7 +42,7 @@ namespace cling {
     bool unloadDeserializedDeclarations(Transaction* T,
                                         DeclUnloader& DeclU);
     bool unloadFromPreprocessor(Transaction* T, DeclUnloader& DeclU);
-    bool unloadModule(llvm::Module* M);
+    bool unloadModule(const std::shared_ptr<llvm::Module>& M);
 
   public:
     TransactionUnloader(cling::Interpreter* I, clang::Sema* Sema,