From 916823d93c7df0544ec91f7de2c9f5fb7f88cfb2 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Wed, 20 Feb 2019 10:02:34 +0100
Subject: [PATCH 01/20] [cxxmodules] Implement global module indexing to
 improve performance.

The global module index represents an efficient on-disk hash table which stores
identifier->module mapping. Every time clang finds a unknown identifier we
are informed and we can load the corresponding module on demand.

This way we can provide minimal set of loaded modules. Currently, we see that
for hsimple.C only the half of the modules are loaded. This can be further
improved because we currently load all modules which have an identifier, that is
when looking for (for example TPad) we will load all modules which have the
identifier TPad, including modules which contain only a forward declaration of
it.

Kudos Arpitha Raghunandan (arpi-r)!
---
 core/metacling/src/TCling.cxx                 | 79 ++++++++++++++++++-
 core/metacling/src/TClingCallbacks.cxx        | 39 ++++++++-
 .../cling/lib/Interpreter/CIFactory.cpp       | 16 ++--
 .../cling/lib/Interpreter/Interpreter.cpp     | 26 +++++-
 .../clang/Serialization/GlobalModuleIndex.h   |  3 +
 .../src/tools/clang/lib/Sema/SemaDecl.cpp     |  2 +-
 .../lib/Serialization/GlobalModuleIndex.cpp   | 47 ++++++++---
 7 files changed, 184 insertions(+), 28 deletions(-)

diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx
index e683bd6269188..4d4b98be98ba5 100644
--- a/core/metacling/src/TCling.cxx
+++ b/core/metacling/src/TCling.cxx
@@ -95,9 +95,11 @@ clang/LLVM technology.
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Parse/Parser.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Sema.h"
-#include "clang/Parse/Parser.h"
+#include "clang/Serialization/ASTReader.h"
+#include "clang/Serialization/GlobalModuleIndex.h"
 
 #include "cling/Interpreter/ClangInternalState.h"
 #include "cling/Interpreter/DynamicLibraryManager.h"
@@ -1194,6 +1196,51 @@ static void RegisterPreIncludedHeaders(cling::Interpreter &clingInterp)
    clingInterp.declare(PreIncludes);
 }
 
+static bool HaveFullGlobalModuleIndex = false;
+static GlobalModuleIndex *loadGlobalModuleIndex(cling::Interpreter &interp, SourceLocation TriggerLoc)
+{
+   CompilerInstance &CI = *interp.getCI();
+   Preprocessor &PP = CI.getPreprocessor();
+   auto ModuleManager = CI.getModuleManager();
+   assert(ModuleManager);
+   // StringRef ModuleIndexPath = HSI.getModuleCachePath();
+   // HeaderSearch& HSI = PP.getHeaderSearchInfo();
+   // HSI.setModuleCachePath(TROOT::GetLibDir().Data());
+   std::string ModuleIndexPath = TROOT::GetLibDir().Data();
+   if (ModuleIndexPath.empty())
+      return nullptr;
+   // Get an existing global index. This loads it if not already loaded.
+   ModuleManager->resetForReload();
+   ModuleManager->loadGlobalIndex();
+   GlobalModuleIndex *GlobalIndex = ModuleManager->getGlobalIndex();
+   if (!GlobalIndex && CI.hasFileManager()) {
+   }
+
+   // For finding modules needing to be imported for fixit messages,
+   // we need to make the global index cover all modules, so we do that here.
+   if (!GlobalIndex && !HaveFullGlobalModuleIndex) {
+      ModuleMap &MMap = PP.getHeaderSearchInfo().getModuleMap();
+      bool RecreateIndex = false;
+      for (ModuleMap::module_iterator I = MMap.module_begin(), E = MMap.module_end(); I != E; ++I) {
+         Module *TheModule = I->second;
+         // We do want the index only of the prebuilt modules
+         std::string ModuleName = GetModuleNameAsString(TheModule, PP);
+         if (ModuleName.empty())
+            continue;
+         LoadModule(ModuleName, interp);
+         RecreateIndex = true;
+      }
+      if (RecreateIndex) {
+         GlobalModuleIndex::writeIndex(CI.getFileManager(), CI.getPCHContainerReader(), ModuleIndexPath);
+         ModuleManager->resetForReload();
+         ModuleManager->loadGlobalIndex();
+         GlobalIndex = ModuleManager->getGlobalIndex();
+      }
+      HaveFullGlobalModuleIndex = true;
+   }
+   return GlobalIndex;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 /// Initialize the cling interpreter interface.
 /// \param argv - array of arguments passed to the cling::Interpreter constructor
@@ -1325,6 +1372,7 @@ TCling::TCling(const char *name, const char *title, const char* const argv[])
 
          clingArgsStorage.push_back("-fmodule-map-file=" + ModuleMapLoc);
       }
+      clingArgsStorage.push_back("-fmodules-cache-path=" + std::string(TROOT::GetLibDir()));
    }
 
    std::vector<const char*> interpArgs;
@@ -1401,7 +1449,34 @@ TCling::TCling(const char *name, const char *title, const char* const argv[])
    static llvm::raw_fd_ostream fMPOuts (STDOUT_FILENO, /*ShouldClose*/false);
    fMetaProcessor = llvm::make_unique<cling::MetaProcessor>(*fInterpreter, fMPOuts);
 
-   RegisterCxxModules(*fInterpreter);
+   if (fInterpreter->getCI()->getLangOpts().Modules) {
+      // Setup core C++ modules if we have any to setup.
+
+      // Load libc and stl first.
+#ifdef R__MACOSX
+      LoadModules({"Darwin", "std"}, *fInterpreter);
+#else
+      LoadModules({"libc", "stl"}, *fInterpreter);
+#endif
+
+      if (!fromRootCling)
+         loadGlobalModuleIndex(*fInterpreter, SourceLocation());
+
+      // C99 decided that it's a very good idea to name a macro `I` (the letter I).
+      // This seems to screw up nearly all the template code out there as `I` is
+      // common template parameter name and iterator variable name.
+      // Let's follow the GCC recommendation and undefine `I` in case any of the
+      // core modules have defined it:
+      // https://www.gnu.org/software/libc/manual/html_node/Complex-Numbers.html
+      fInterpreter->declare("#ifdef I\n #undef I\n #endif\n");
+
+      // These macros are from loading R related modules, which conflict with
+      // user's code.
+      fInterpreter->declare("#ifdef PI\n #undef PI\n #endif\n");
+      fInterpreter->declare("#ifdef ERROR\n #undef ERROR\n #endif\n");
+   }
+
+   // RegisterCxxModules(*fInterpreter);
    RegisterPreIncludedHeaders(*fInterpreter);
 
    // We are now ready (enough is loaded) to init the list of opaque typedefs.
diff --git a/core/metacling/src/TClingCallbacks.cxx b/core/metacling/src/TClingCallbacks.cxx
index 31bdfb5f0c0e8..a757298f90360 100644
--- a/core/metacling/src/TClingCallbacks.cxx
+++ b/core/metacling/src/TClingCallbacks.cxx
@@ -29,6 +29,8 @@
 #include "clang/Parse/Parser.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Scope.h"
+#include "clang/Serialization/ASTReader.h"
+#include "clang/Serialization/GlobalModuleIndex.h"
 
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -285,6 +287,28 @@ bool TClingCallbacks::LookupObject(LookupResult &R, Scope *S) {
    return tryResolveAtRuntimeInternal(R, S);
 }
 
+static bool findInGlobalIndex(cling::Interpreter &Interp, DeclarationName Name, bool loadFirstMatchOnly = true)
+{
+   GlobalModuleIndex *Index = Interp.getCI()->getModuleManager()->getGlobalIndex();
+   if (!Index)
+      return false;
+
+   GlobalModuleIndex::FileNameHitSet FoundModules;
+
+   // Find the modules that reference the identifier.
+   // Note that this only finds top-level modules.
+   if (Index->lookupIdentifier(Name.getAsString(), FoundModules)) {
+      for (auto FileName : FoundModules) {
+         StringRef ModuleName = llvm::sys::path::stem(*FileName);
+         Interp.loadModule(ModuleName);
+         if (loadFirstMatchOnly)
+            break;
+      }
+      return true;
+   }
+   return false;
+}
+
 bool TClingCallbacks::LookupObject(const DeclContext* DC, DeclarationName Name) {
    if (!fROOTSpecialNamespace) {
       // init error or rootcling
@@ -293,9 +317,21 @@ bool TClingCallbacks::LookupObject(const DeclContext* DC, DeclarationName Name)
 
    if (!IsAutoLoadingEnabled() || fIsAutoLoadingRecursively) return false;
 
-   if (Name.getNameKind() != DeclarationName::Identifier) return false;
+   // We are currently building a module, we should not autoload.
+   Sema &SemaR = m_Interpreter->getSema();
+   const LangOptions &LangOpts = SemaR.getPreprocessor().getLangOpts();
+   if (LangOpts.Modules) {
+      if (LangOpts.isCompilingModule())
+         return false;
 
+      // FIXME: We should load only the first available and rely on other callbacks
+      // such as RequireCompleteType and LookupUnqualified to load all.
+      if (findInGlobalIndex(*m_Interpreter, Name, /*loadFirstMatchOnly*/ false))
+         return true;
+   }
 
+   if (Name.getNameKind() != DeclarationName::Identifier)
+      return false;
 
    // Get the 'lookup' decl context.
    // We need to cast away the constness because we will lookup items of this
@@ -311,7 +347,6 @@ bool TClingCallbacks::LookupObject(const DeclContext* DC, DeclarationName Name)
    if (primaryDC != DC)
       return false;
 
-   Sema &SemaR = m_Interpreter->getSema();
    LookupResult R(SemaR, Name, SourceLocation(), Sema::LookupOrdinaryName);
    R.suppressDiagnostics();
    // We need the qualified name for TCling to find the right library.
diff --git a/interpreter/cling/lib/Interpreter/CIFactory.cpp b/interpreter/cling/lib/Interpreter/CIFactory.cpp
index c1fb1c46c3563..b4869d55820f9 100644
--- a/interpreter/cling/lib/Interpreter/CIFactory.cpp
+++ b/interpreter/cling/lib/Interpreter/CIFactory.cpp
@@ -547,17 +547,15 @@ namespace {
     for (StringRef ModulePath : Paths) {
       // FIXME: If we have a prebuilt module path that is equal to our module
       // cache we fail to compile the clang builtin modules for some reason.
-      // This can't be reproduced in clang, so I assume we have some strange
-      // error in our interpreter setup where this is causing errors (or maybe
-      // clang is doing the same check in some hidden place).
-      // The error looks like this:
-      //   .../include/stddef.h error: unknown type name '__PTRDIFF_TYPE__'
-      //   typedef __PTRDIFF_TYPE__ ptrdiff_t;
-      //   <similar follow up errors>
+      // This makes clang to think it failed to build a dependency module, i.e.
+      // if we are building module C, clang goes off and builds B and A first.
+      // If the module cache points to the same location as the prebuilt module
+      // path, clang errors out on building module A, however, it builds it.
+      // Next time we run, it will build module B and issue diagnostics.
+      // If we run third time, it'd build successfully C and continue.
       // For now it is fixed by just checking those two paths are not identical.
-      if (normalizePath(ModulePath) != normalizePath(Opts.ModuleCachePath)) {
+      if (normalizePath(ModulePath) != normalizePath(Opts.ModuleCachePath))
         Opts.AddPrebuiltModulePath(ModulePath);
-      }
     }
   }
 
diff --git a/interpreter/cling/lib/Interpreter/Interpreter.cpp b/interpreter/cling/lib/Interpreter/Interpreter.cpp
index 843c77ca46e4c..a66cf86b52770 100644
--- a/interpreter/cling/lib/Interpreter/Interpreter.cpp
+++ b/interpreter/cling/lib/Interpreter/Interpreter.cpp
@@ -873,10 +873,32 @@ namespace cling {
     if (getSema().isModuleVisible(M))
       return true;
 
+    // We cannot use #pragma clang module import because the on-demand modules
+    // may load a module in the middle of a function body for example. In this
+    // case this triggers an error:
+    // fatal error: import of module '...' appears within function '...'
+    //
+    // if (declare("#pragma clang module import \"" + M->Name + "\"") ==
+    // kSuccess)
+    //   return true;
+
     // FIXME: What about importing submodules such as std.blah. This disables
     // this functionality.
-    if (declare("#pragma clang module import \"" + M->Name + "\"") == kSuccess)
-      return true;
+    Preprocessor& PP = getCI()->getPreprocessor();
+    IdentifierInfo* II = PP.getIdentifierInfo(M->Name);
+    SourceLocation ValidLoc = M->DefinitionLoc;
+    Interpreter::PushTransactionRAII RAII(this);
+    bool success = !getCI()
+                        ->getSema()
+                        .ActOnModuleImport(ValidLoc, ValidLoc,
+                                           std::make_pair(II, ValidLoc))
+                        .isInvalid();
+
+    if (success) {
+      // Also make the module visible in the preprocessor to export its macros.
+      PP.makeModuleVisible(M, ValidLoc);
+      return success;
+    }
 
     if (complain) {
       if (M->IsSystem)
diff --git a/interpreter/llvm/src/tools/clang/include/clang/Serialization/GlobalModuleIndex.h b/interpreter/llvm/src/tools/clang/include/clang/Serialization/GlobalModuleIndex.h
index 0f14eca0fd86a..299b80982b0c7 100644
--- a/interpreter/llvm/src/tools/clang/include/clang/Serialization/GlobalModuleIndex.h
+++ b/interpreter/llvm/src/tools/clang/include/clang/Serialization/GlobalModuleIndex.h
@@ -179,6 +179,9 @@ class GlobalModuleIndex {
   /// \returns true if the identifier is known to the index, false otherwise.
   bool lookupIdentifier(StringRef Name, HitSet &Hits);
 
+  typedef llvm::SmallPtrSet<std::string *, 4> FileNameHitSet;
+  bool lookupIdentifier(StringRef Name, FileNameHitSet &Hits);
+
   /// \brief Note that the given module file has been loaded.
   ///
   /// \returns false if the global module index has information about this
diff --git a/interpreter/llvm/src/tools/clang/lib/Sema/SemaDecl.cpp b/interpreter/llvm/src/tools/clang/lib/Sema/SemaDecl.cpp
index b1c7c61f2bc4f..261756436146c 100644
--- a/interpreter/llvm/src/tools/clang/lib/Sema/SemaDecl.cpp
+++ b/interpreter/llvm/src/tools/clang/lib/Sema/SemaDecl.cpp
@@ -16196,7 +16196,7 @@ DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
 
   VisibleModules.setVisible(Mod, ImportLoc);
 
-  checkModuleImportContext(*this, Mod, ImportLoc, CurContext);
+  // checkModuleImportContext(*this, Mod, ImportLoc, CurContext);
 
   // FIXME: we should support importing a submodule within a different submodule
   // of the same top-level module. Until we do, make it an error rather than
diff --git a/interpreter/llvm/src/tools/clang/lib/Serialization/GlobalModuleIndex.cpp b/interpreter/llvm/src/tools/clang/lib/Serialization/GlobalModuleIndex.cpp
index 6978e7e09774f..b98fa26977241 100644
--- a/interpreter/llvm/src/tools/clang/lib/Serialization/GlobalModuleIndex.cpp
+++ b/interpreter/llvm/src/tools/clang/lib/Serialization/GlobalModuleIndex.cpp
@@ -313,6 +313,32 @@ bool GlobalModuleIndex::lookupIdentifier(StringRef Name, HitSet &Hits) {
   return true;
 }
 
+bool GlobalModuleIndex::lookupIdentifier(StringRef Name, FileNameHitSet &Hits) {
+  Hits.clear();
+
+  // If there's no identifier index, there is nothing we can do.
+  if (!IdentifierIndex)
+    return false;
+
+  // Look into the identifier index.
+  ++NumIdentifierLookups;
+  IdentifierIndexTable &Table =
+      *static_cast<IdentifierIndexTable *>(IdentifierIndex);
+  IdentifierIndexTable::iterator Known = Table.find(Name);
+  if (Known == Table.end()) {
+    return true;
+  }
+
+  SmallVector<unsigned, 2> ModuleIDs = *Known;
+  for (unsigned I = 0, N = ModuleIDs.size(); I != N; ++I) {
+    assert(!Modules[ModuleIDs[I]].FileName.empty());
+    Hits.insert(&Modules[ModuleIDs[I]].FileName);
+  }
+
+  ++NumIdentifierLookupHits;
+  return true;
+}
+
 bool GlobalModuleIndex::loadedModuleFile(ModuleFile *File) {
   // Look for the module in the global module index based on the module name.
   StringRef Name = File->ModuleName;
@@ -660,10 +686,7 @@ bool GlobalModuleIndexBuilder::loadModuleFile(const FileEntry *File) {
                                                      DEnd = Table->data_end();
            D != DEnd; ++D) {
         std::pair<StringRef, bool> Ident = *D;
-        if (Ident.second)
-          InterestingIdentifiers[Ident.first].push_back(ID);
-        else
-          (void)InterestingIdentifiers[Ident.first];
+        InterestingIdentifiers[Ident.first].push_back(ID);
       }
     }
 
@@ -725,14 +748,14 @@ bool GlobalModuleIndexBuilder::writeIndex(llvm::BitstreamWriter &Stream) {
   for (auto MapEntry : ImportedModuleFiles) {
     auto *File = MapEntry.first;
     ImportedModuleFileInfo &Info = MapEntry.second;
-    if (getModuleFileInfo(File).Signature) {
-      if (getModuleFileInfo(File).Signature != Info.StoredSignature)
-        // Verify Signature.
-        return true;
-    } else if (Info.StoredSize != File->getSize() ||
-               Info.StoredModTime != File->getModificationTime())
-      // Verify Size and ModTime.
-      return true;
+    // if (getModuleFileInfo(File).Signature) {
+    //   if (getModuleFileInfo(File).Signature != Info.StoredSignature)
+    //     // Verify Signature.
+    //     return true;
+    // } else if (Info.StoredSize != File->getSize() ||
+    //            Info.StoredModTime != File->getModificationTime())
+    //   // Verify Size and ModTime.
+    //   return true;
   }
 
   using namespace llvm;

From e4bb1a84711ce8dbe2f01514d9d23d0711e424dd Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Sun, 30 Jun 2019 20:30:06 +0300
Subject: [PATCH 02/20] [cxxmodules] Do not cache the file lookup failure.

In cases where we have the module cache path and prebuilt module path pointing
to the same location, the FileManager should not cache the module file lookup
failure because it may currently be building the module.

This patch is necessary because the global module index is built from the
module cache path and it is loaded from the prebuilt module path.

In a full explicit or implicit module build infrastructure this is not a problem.
However, in a mixed scenario (where modules for third-party dependencies are
built implicitly) such as ours this is problematic. One of the reasons is that
we cannot configure the prebuilt modules paths or module cache paths externally.
This is because the interpreter (at construction time) #includes RuntimeUniverse
which may trigger module build.

This patch allows us to refactor some of the code working around this issue.
---
 .../cling/lib/Interpreter/CIFactory.cpp       | 26 ++-----------------
 .../src/tools/clang/lib/Lex/HeaderSearch.cpp  | 14 +++++++---
 2 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/interpreter/cling/lib/Interpreter/CIFactory.cpp b/interpreter/cling/lib/Interpreter/CIFactory.cpp
index b4869d55820f9..3e4df0d56996c 100644
--- a/interpreter/cling/lib/Interpreter/CIFactory.cpp
+++ b/interpreter/cling/lib/Interpreter/CIFactory.cpp
@@ -529,34 +529,12 @@ namespace {
     return Paths;
   }
 
-  /// \brief Prepares a file path for string comparison with another file path.
-  /// This easily be tricked by a malicious user with hardlinking directories
-  /// and so on, but for a comparison in good faith this should be enough.
-  static std::string normalizePath(StringRef path) {
-    SmallVector<char, 256> AbsolutePath, Result;
-    AbsolutePath.insert(AbsolutePath.begin(), path.begin(), path.end());
-    llvm::sys::fs::make_absolute(AbsolutePath);
-    llvm::sys::fs::real_path(AbsolutePath, Result, true);
-    return llvm::Twine(Result).str();
-  }
-
   /// \brief Adds all the paths to the prebuilt module paths of the given
   /// HeaderSearchOptions.
   static void addPrebuiltModulePaths(clang::HeaderSearchOptions& Opts,
                                      const SmallVectorImpl<StringRef>& Paths) {
-    for (StringRef ModulePath : Paths) {
-      // FIXME: If we have a prebuilt module path that is equal to our module
-      // cache we fail to compile the clang builtin modules for some reason.
-      // This makes clang to think it failed to build a dependency module, i.e.
-      // if we are building module C, clang goes off and builds B and A first.
-      // If the module cache points to the same location as the prebuilt module
-      // path, clang errors out on building module A, however, it builds it.
-      // Next time we run, it will build module B and issue diagnostics.
-      // If we run third time, it'd build successfully C and continue.
-      // For now it is fixed by just checking those two paths are not identical.
-      if (normalizePath(ModulePath) != normalizePath(Opts.ModuleCachePath))
-        Opts.AddPrebuiltModulePath(ModulePath);
-    }
+    for (StringRef ModulePath : Paths)
+      Opts.AddPrebuiltModulePath(ModulePath);
   }
 
   static std::string getIncludePathForHeader(const clang::HeaderSearch& HS,
diff --git a/interpreter/llvm/src/tools/clang/lib/Lex/HeaderSearch.cpp b/interpreter/llvm/src/tools/clang/lib/Lex/HeaderSearch.cpp
index e06f8fcd77d63..2f85c48572934 100644
--- a/interpreter/llvm/src/tools/clang/lib/Lex/HeaderSearch.cpp
+++ b/interpreter/llvm/src/tools/clang/lib/Lex/HeaderSearch.cpp
@@ -138,6 +138,7 @@ std::string HeaderSearch::getModuleFileName(Module *Module) {
 std::string HeaderSearch::getModuleFileName(StringRef ModuleName,
                                             StringRef ModuleMapPath,
                                             bool UsePrebuiltPath) {
+  std::string ModuleCachePath = getModuleCachePath();
   if (UsePrebuiltPath) {
     if (HSOpts->PrebuiltModulePaths.empty())
       return std::string();
@@ -146,9 +147,14 @@ std::string HeaderSearch::getModuleFileName(StringRef ModuleName,
     for (const std::string &Dir : HSOpts->PrebuiltModulePaths) {
       SmallString<256> Result(Dir);
       llvm::sys::fs::make_absolute(Result);
-
+      bool CacheFailure = true;
+      // If we have the same ModuleCachePath and PrebuiltModulePath pointing
+      // to the same folder we should not cache the file lookup failure as it
+      // may be currently building an implicit module.
+      if (!ModuleCachePath.empty() && ModuleCachePath == Result)
+        CacheFailure = false;
       llvm::sys::path::append(Result, ModuleName + ".pcm");
-      if (getFileMgr().getFile(Result.str()))
+      if (getFileMgr().getFile(Result.str(), /*Open=*/false, CacheFailure))
         return Result.str().str();
     }
     return std::string();
@@ -156,10 +162,10 @@ std::string HeaderSearch::getModuleFileName(StringRef ModuleName,
 
   // If we don't have a module cache path or aren't supposed to use one, we
   // can't do anything.
-  if (getModuleCachePath().empty())
+  if (ModuleCachePath.empty())
     return std::string();
 
-  SmallString<256> Result(getModuleCachePath());
+  SmallString<256> Result(ModuleCachePath);
   llvm::sys::fs::make_absolute(Result);
 
   if (HSOpts->DisableModuleHash) {

From 7c14fcf30b3d82a0763537259afe39af8c52badb Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Sun, 30 Jun 2019 22:29:58 +0300
Subject: [PATCH 03/20] [cxxmodules] Preload only common modules.

---
 core/metacling/src/TCling.cxx | 156 +++++++++++++---------------------
 1 file changed, 57 insertions(+), 99 deletions(-)

diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx
index 4d4b98be98ba5..8ee9d08f83a9e 100644
--- a/core/metacling/src/TCling.cxx
+++ b/core/metacling/src/TCling.cxx
@@ -1090,7 +1090,50 @@ static std::string GetModuleNameAsString(clang::Module *M, const clang::Preproce
    return std::string(llvm::sys::path::stem(ModuleName));
 }
 
-static void RegisterCxxModules(cling::Interpreter &clingInterp)
+static bool HaveFullGlobalModuleIndex = false;
+static GlobalModuleIndex *loadGlobalModuleIndex(SourceLocation TriggerLoc, cling::Interpreter &interp)
+{
+   CompilerInstance &CI = *interp.getCI();
+   Preprocessor &PP = CI.getPreprocessor();
+   auto ModuleManager = CI.getModuleManager();
+   assert(ModuleManager);
+   // StringRef ModuleIndexPath = HSI.getModuleCachePath();
+   // HeaderSearch& HSI = PP.getHeaderSearchInfo();
+   // HSI.setModuleCachePath(TROOT::GetLibDir().Data());
+   std::string ModuleIndexPath = TROOT::GetLibDir().Data();
+   if (ModuleIndexPath.empty())
+      return nullptr;
+   // Get an existing global index. This loads it if not already loaded.
+   ModuleManager->resetForReload();
+   ModuleManager->loadGlobalIndex();
+   GlobalModuleIndex *GlobalIndex = ModuleManager->getGlobalIndex();
+
+   // For finding modules needing to be imported for fixit messages,
+   // we need to make the global index cover all modules, so we do that here.
+   if (!GlobalIndex && !HaveFullGlobalModuleIndex) {
+      ModuleMap &MMap = PP.getHeaderSearchInfo().getModuleMap();
+      bool RecreateIndex = false;
+      for (ModuleMap::module_iterator I = MMap.module_begin(), E = MMap.module_end(); I != E; ++I) {
+         Module *TheModule = I->second;
+         // We do want the index only of the prebuilt modules
+         std::string ModuleName = GetModuleNameAsString(TheModule, PP);
+         if (ModuleName.empty())
+            continue;
+         LoadModule(ModuleName, interp);
+         RecreateIndex = true;
+      }
+      if (RecreateIndex) {
+         GlobalModuleIndex::writeIndex(CI.getFileManager(), CI.getPCHContainerReader(), ModuleIndexPath);
+         ModuleManager->resetForReload();
+         ModuleManager->loadGlobalIndex();
+         GlobalIndex = ModuleManager->getGlobalIndex();
+      }
+      HaveFullGlobalModuleIndex = true;
+   }
+   return GlobalIndex;
+}
+
+static void RegisterCommonCxxModules(cling::Interpreter &clingInterp)
 {
    if (!clingInterp.getCI()->getLangOpts().Modules)
       return;
@@ -1115,35 +1158,22 @@ static void RegisterCxxModules(cling::Interpreter &clingInterp)
                                            "Core",
                                            "RIO"};
 
-   // FIXME: Reducing those will let us be less dependent on rootmap files
-   static constexpr std::array<const char *, 3> ExcludeModules = {
-      {"Rtools", "RSQLite", "RInterface"}};
-
    LoadModules(CoreModules, clingInterp);
 
+   // FIXME: Reducing those will let us be less dependent on rootmap files
+   // static constexpr std::array<const char *, 3> ExcludeModules = {
+   //    {"Rtools", "RSQLite", "RInterface"}};
+
    // Take this branch only from ROOT because we don't need to preload modules in rootcling
    if (!IsFromRootCling()) {
-      // Dynamically get all the modules and load them if they are not in core modules
-      clang::CompilerInstance &CI = *clingInterp.getCI();
-      clang::ModuleMap &moduleMap = CI.getPreprocessor().getHeaderSearchInfo().getModuleMap();
-      clang::Preprocessor &PP = CI.getPreprocessor();
-      std::vector<std::string> ModulesPreloaded;
-      for (auto I = moduleMap.module_begin(), E = moduleMap.module_end(); I != E; ++I) {
-         clang::Module *M = I->second;
-         assert(M);
-
-         std::string ModuleName = GetModuleNameAsString(M, PP);
-         if (!ModuleName.empty() &&
-             std::find(CoreModules.begin(), CoreModules.end(), ModuleName) == CoreModules.end() &&
-             std::find(ExcludeModules.begin(), ExcludeModules.end(), ModuleName) ==
-                ExcludeModules.end()) {
-            if (M->IsSystem && !M->IsMissingRequirement)
-               LoadModule(ModuleName, clingInterp);
-            else if (!M->IsSystem && !M->IsMissingRequirement)
-               ModulesPreloaded.push_back(ModuleName);
-         }
-      }
-      LoadModules(ModulesPreloaded, clingInterp);
+      std::vector<std::string> CommonModules = {"MathCore"};
+      LoadModules(CommonModules, clingInterp);
+
+      // These modules should not be preloaded but they fix issues.
+      std::vector<std::string> FIXMEModules = {"Gpad"};
+      LoadModules(FIXMEModules, clingInterp);
+
+      loadGlobalModuleIndex(SourceLocation(), clingInterp);
    }
 
    // Check that the gROOT macro was exported by any core module.
@@ -1196,51 +1226,6 @@ static void RegisterPreIncludedHeaders(cling::Interpreter &clingInterp)
    clingInterp.declare(PreIncludes);
 }
 
-static bool HaveFullGlobalModuleIndex = false;
-static GlobalModuleIndex *loadGlobalModuleIndex(cling::Interpreter &interp, SourceLocation TriggerLoc)
-{
-   CompilerInstance &CI = *interp.getCI();
-   Preprocessor &PP = CI.getPreprocessor();
-   auto ModuleManager = CI.getModuleManager();
-   assert(ModuleManager);
-   // StringRef ModuleIndexPath = HSI.getModuleCachePath();
-   // HeaderSearch& HSI = PP.getHeaderSearchInfo();
-   // HSI.setModuleCachePath(TROOT::GetLibDir().Data());
-   std::string ModuleIndexPath = TROOT::GetLibDir().Data();
-   if (ModuleIndexPath.empty())
-      return nullptr;
-   // Get an existing global index. This loads it if not already loaded.
-   ModuleManager->resetForReload();
-   ModuleManager->loadGlobalIndex();
-   GlobalModuleIndex *GlobalIndex = ModuleManager->getGlobalIndex();
-   if (!GlobalIndex && CI.hasFileManager()) {
-   }
-
-   // For finding modules needing to be imported for fixit messages,
-   // we need to make the global index cover all modules, so we do that here.
-   if (!GlobalIndex && !HaveFullGlobalModuleIndex) {
-      ModuleMap &MMap = PP.getHeaderSearchInfo().getModuleMap();
-      bool RecreateIndex = false;
-      for (ModuleMap::module_iterator I = MMap.module_begin(), E = MMap.module_end(); I != E; ++I) {
-         Module *TheModule = I->second;
-         // We do want the index only of the prebuilt modules
-         std::string ModuleName = GetModuleNameAsString(TheModule, PP);
-         if (ModuleName.empty())
-            continue;
-         LoadModule(ModuleName, interp);
-         RecreateIndex = true;
-      }
-      if (RecreateIndex) {
-         GlobalModuleIndex::writeIndex(CI.getFileManager(), CI.getPCHContainerReader(), ModuleIndexPath);
-         ModuleManager->resetForReload();
-         ModuleManager->loadGlobalIndex();
-         GlobalIndex = ModuleManager->getGlobalIndex();
-      }
-      HaveFullGlobalModuleIndex = true;
-   }
-   return GlobalIndex;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 /// Initialize the cling interpreter interface.
 /// \param argv - array of arguments passed to the cling::Interpreter constructor
@@ -1449,34 +1434,7 @@ TCling::TCling(const char *name, const char *title, const char* const argv[])
    static llvm::raw_fd_ostream fMPOuts (STDOUT_FILENO, /*ShouldClose*/false);
    fMetaProcessor = llvm::make_unique<cling::MetaProcessor>(*fInterpreter, fMPOuts);
 
-   if (fInterpreter->getCI()->getLangOpts().Modules) {
-      // Setup core C++ modules if we have any to setup.
-
-      // Load libc and stl first.
-#ifdef R__MACOSX
-      LoadModules({"Darwin", "std"}, *fInterpreter);
-#else
-      LoadModules({"libc", "stl"}, *fInterpreter);
-#endif
-
-      if (!fromRootCling)
-         loadGlobalModuleIndex(*fInterpreter, SourceLocation());
-
-      // C99 decided that it's a very good idea to name a macro `I` (the letter I).
-      // This seems to screw up nearly all the template code out there as `I` is
-      // common template parameter name and iterator variable name.
-      // Let's follow the GCC recommendation and undefine `I` in case any of the
-      // core modules have defined it:
-      // https://www.gnu.org/software/libc/manual/html_node/Complex-Numbers.html
-      fInterpreter->declare("#ifdef I\n #undef I\n #endif\n");
-
-      // These macros are from loading R related modules, which conflict with
-      // user's code.
-      fInterpreter->declare("#ifdef PI\n #undef PI\n #endif\n");
-      fInterpreter->declare("#ifdef ERROR\n #undef ERROR\n #endif\n");
-   }
-
-   // RegisterCxxModules(*fInterpreter);
+   RegisterCommonCxxModules(*fInterpreter);
    RegisterPreIncludedHeaders(*fInterpreter);
 
    // We are now ready (enough is loaded) to init the list of opaque typedefs.

From 3bda1e6d006aacf8169fe62542e182bf3fc15b42 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Thu, 4 Jul 2019 23:24:49 +0300
Subject: [PATCH 04/20] [cxxmodules][cling] Add a callback for start/finish
 code generation.

When we are generating code, CodeGen automatically tries to complete decl's
redeclaration chain. This ends up a call to the external sources, one of which
is our global module index (GMI).

The semantics of the GMI is to aid the frontend, that is to automatically
import missing modules based on the typed *by the user* identifiers. It does
not intend to aid the code generation by any means. Currently this happens
when completing chains for existing identifiers such as 'volume' which happens
to be an indentifier part of TMVA.pcm, too. Thus, CodeGen unintentionally loads
the module of TMVA. This is an overkill, but worse -- it brings recursiveness
to the identifier resolution system.

This patch disables looking in the GMI at codegen time.
---
 core/metacling/src/TClingCallbacks.cxx         | 12 +++++++-----
 core/metacling/src/TClingCallbacks.h           | 18 ++++++++++++++++++
 .../cling/Interpreter/InterpreterCallbacks.h   | 14 ++++++++++++++
 .../lib/Interpreter/IncrementalParser.cpp      |  6 ++++++
 .../MultiplexInterpreterCallbacks.h            | 12 ++++++++++++
 5 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/core/metacling/src/TClingCallbacks.cxx b/core/metacling/src/TClingCallbacks.cxx
index a757298f90360..4e1a2a2cfeb3c 100644
--- a/core/metacling/src/TClingCallbacks.cxx
+++ b/core/metacling/src/TClingCallbacks.cxx
@@ -76,11 +76,11 @@ extern "C" {
                                     cling::Interpreter &interpreter, bool searchSystem);
 }
 
-TClingCallbacks::TClingCallbacks(cling::Interpreter* interp, bool hasCodeGen)
-   : InterpreterCallbacks(interp),
-     fLastLookupCtx(0), fROOTSpecialNamespace(0),
-     fFirstRun(true), fIsAutoLoading(false), fIsAutoLoadingRecursively(false),
-     fIsAutoParsingSuspended(false), fPPOldFlag(false), fPPChanged(false) {
+TClingCallbacks::TClingCallbacks(cling::Interpreter *interp, bool hasCodeGen)
+   : InterpreterCallbacks(interp), fLastLookupCtx(0), fROOTSpecialNamespace(0), fFirstRun(true), fIsAutoloading(false),
+     fIsAutoloadingRecursively(false), fIsAutoParsingSuspended(false), fPPOldFlag(false), fPPChanged(false),
+     fIsCodeGening(false)
+{
    if (hasCodeGen) {
       Transaction* T = 0;
       m_Interpreter->declare("namespace __ROOT_SpecialObjects{}", &T);
@@ -323,6 +323,8 @@ bool TClingCallbacks::LookupObject(const DeclContext* DC, DeclarationName Name)
    if (LangOpts.Modules) {
       if (LangOpts.isCompilingModule())
          return false;
+      if (fIsCodeGening)
+         return false;
 
       // FIXME: We should load only the first available and rely on other callbacks
       // such as RequireCompleteType and LookupUnqualified to load all.
diff --git a/core/metacling/src/TClingCallbacks.h b/core/metacling/src/TClingCallbacks.h
index 77680a3ded503..92f2ea76f5c4a 100644
--- a/core/metacling/src/TClingCallbacks.h
+++ b/core/metacling/src/TClingCallbacks.h
@@ -45,6 +45,8 @@ class TClingCallbacks : public cling::InterpreterCallbacks {
    bool fIsAutoParsingSuspended;
    bool fPPOldFlag;
    bool fPPChanged;
+   bool fIsCodeGening;
+
 public:
    TClingCallbacks(cling::Interpreter* interp, bool hasCodeGen);
 
@@ -85,6 +87,22 @@ class TClingCallbacks : public cling::InterpreterCallbacks {
    //
    virtual void TransactionCommitted(const cling::Transaction &T);
 
+   // The callback is used to inform ROOT when cling started code generation.
+   //
+   virtual void TransactionCodeGenStarted(const cling::Transaction &T)
+   {
+      assert(!fIsCodeGening);
+      fIsCodeGening = true;
+   }
+
+   // The callback is used to inform ROOT when cling finished code generation.
+   //
+   virtual void TransactionCodeGenFinished(const cling::Transaction &T)
+   {
+      assert(fIsCodeGening);
+      fIsCodeGening = false;
+   }
+
    // The callback is used to update the list of globals in ROOT.
    //
    virtual void TransactionUnloaded(const cling::Transaction &T);
diff --git a/interpreter/cling/include/cling/Interpreter/InterpreterCallbacks.h b/interpreter/cling/include/cling/Interpreter/InterpreterCallbacks.h
index 67bf9ec6c79c8..6781c36ec8245 100644
--- a/interpreter/cling/include/cling/Interpreter/InterpreterCallbacks.h
+++ b/interpreter/cling/include/cling/Interpreter/InterpreterCallbacks.h
@@ -141,6 +141,20 @@ namespace cling {
     ///
     virtual void TransactionCommitted(const Transaction&) {}
 
+    /// This callback is invoked whenever interpreter has started code
+    /// generation for the transaction.
+    ///
+    ///\param[in] - The transaction that is being codegen-ed.
+    ///
+    virtual void TransactionCodeGenStarted(const Transaction&) {}
+
+    /// This callback is invoked whenever interpreter has finished code
+    /// generation for the transaction.
+    ///
+    ///\param[in] - The transaction that is being codegen-ed.
+    ///
+    virtual void TransactionCodeGenFinished(const Transaction&) {}
+
     ///\brief This callback is invoked whenever interpreter has reverted a
     /// transaction that has been fully committed.
     ///
diff --git a/interpreter/cling/lib/Interpreter/IncrementalParser.cpp b/interpreter/cling/lib/Interpreter/IncrementalParser.cpp
index 5900fab11bf86..d9c4339679bb9 100644
--- a/interpreter/cling/lib/Interpreter/IncrementalParser.cpp
+++ b/interpreter/cling/lib/Interpreter/IncrementalParser.cpp
@@ -657,6 +657,9 @@ namespace cling {
     // This llvm::Module is done; finalize it and pass it to the execution
     // engine.
     if (!T->isNestedTransaction() && hasCodeGenerator()) {
+      if (InterpreterCallbacks* callbacks = m_Interpreter->getCallbacks())
+        callbacks->TransactionCodeGenStarted(*T);
+
       // The initializers are emitted to the symbol "_GLOBAL__sub_I_" + filename.
       // Make that unique!
       deserT = beginTransaction(CompilationOptions());
@@ -678,6 +681,9 @@ namespace cling {
         Diags.getClient()->clear();
       }
 
+      if (InterpreterCallbacks* callbacks = m_Interpreter->getCallbacks())
+        callbacks->TransactionCodeGenFinished(*T);
+
       // Create a new module.
       StartModule();
     }
diff --git a/interpreter/cling/lib/Interpreter/MultiplexInterpreterCallbacks.h b/interpreter/cling/lib/Interpreter/MultiplexInterpreterCallbacks.h
index 6a8b121c091be..714cc99a7b749 100644
--- a/interpreter/cling/lib/Interpreter/MultiplexInterpreterCallbacks.h
+++ b/interpreter/cling/lib/Interpreter/MultiplexInterpreterCallbacks.h
@@ -83,6 +83,18 @@ namespace cling {
        }
      }
 
+     void TransactionCodeGenStarted(const Transaction& T) override {
+       for (auto&& cb : m_Callbacks) {
+         cb->TransactionCodeGenStarted(T);
+       }
+     }
+
+     void TransactionCodeGenFinished(const Transaction& T) override {
+       for (auto&& cb : m_Callbacks) {
+         cb->TransactionCodeGenFinished(T);
+       }
+     }
+
      bool LibraryLoadingFailed(const std::string& errmessage, const std::string& libStem, bool permanent,
          bool resolved) override {
        for (auto&& cb : m_Callbacks) {

From 3db4b285469e379816caa8388dc67304037078e8 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Thu, 4 Jul 2019 23:36:03 +0300
Subject: [PATCH 05/20] [tcling] Modernize header file virtual -> override.

---
 core/metacling/src/TClingCallbacks.h | 57 ++++++++++++----------------
 1 file changed, 24 insertions(+), 33 deletions(-)

diff --git a/core/metacling/src/TClingCallbacks.h b/core/metacling/src/TClingCallbacks.h
index 92f2ea76f5c4a..f37f4d2e1ffee 100644
--- a/core/metacling/src/TClingCallbacks.h
+++ b/core/metacling/src/TClingCallbacks.h
@@ -60,36 +60,29 @@ class TClingCallbacks : public cling::InterpreterCallbacks {
    void SetAutoParsingSuspended(bool val = true) { fIsAutoParsingSuspended = val; }
    bool IsAutoParsingSuspended() { return fIsAutoParsingSuspended; }
 
-   virtual bool LibraryLoadingFailed(const std::string&, const std::string&, bool, bool);
-
-   virtual void InclusionDirective(clang::SourceLocation /*HashLoc*/,
-                                   const clang::Token &/*IncludeTok*/,
-                                   llvm::StringRef FileName,
-                                   bool /*IsAngled*/,
-                                   clang::CharSourceRange /*FilenameRange*/,
-                                   const clang::FileEntry * /*File*/,
-                                   llvm::StringRef /*SearchPath*/,
-                                   llvm::StringRef /*RelativePath*/,
-                                   const clang::Module * /*Imported*/);
+   bool LibraryLoadingFailed(const std::string &, const std::string &, bool, bool) override;
+
+   void InclusionDirective(clang::SourceLocation /*HashLoc*/, const clang::Token & /*IncludeTok*/,
+                           llvm::StringRef FileName, bool /*IsAngled*/, clang::CharSourceRange /*FilenameRange*/,
+                           const clang::FileEntry * /*File*/, llvm::StringRef /*SearchPath*/,
+                           llvm::StringRef /*RelativePath*/, const clang::Module * /*Imported*/) override;
 
    // Preprocessor callbacks used to handle special cases like for example:
    // #include "myMacro.C+"
    //
-   virtual bool FileNotFound(llvm::StringRef FileName,
-                             llvm::SmallVectorImpl<char>& RecoveryPath);
+   bool FileNotFound(llvm::StringRef FileName, llvm::SmallVectorImpl<char> &RecoveryPath) override;
 
-   virtual bool LookupObject(clang::LookupResult &R, clang::Scope *S);
-   virtual bool LookupObject(const clang::DeclContext* DC,
-                             clang::DeclarationName Name);
-   virtual bool LookupObject(clang::TagDecl* Tag);
+   bool LookupObject(clang::LookupResult &R, clang::Scope *S) override;
+   bool LookupObject(const clang::DeclContext *DC, clang::DeclarationName Name) override;
+   bool LookupObject(clang::TagDecl *Tag) override;
 
    // The callback is used to update the list of globals in ROOT.
    //
-   virtual void TransactionCommitted(const cling::Transaction &T);
+   void TransactionCommitted(const cling::Transaction &T) override;
 
    // The callback is used to inform ROOT when cling started code generation.
    //
-   virtual void TransactionCodeGenStarted(const cling::Transaction &T)
+   void TransactionCodeGenStarted(const cling::Transaction &T) override
    {
       assert(!fIsCodeGening);
       fIsCodeGening = true;
@@ -97,7 +90,7 @@ class TClingCallbacks : public cling::InterpreterCallbacks {
 
    // The callback is used to inform ROOT when cling finished code generation.
    //
-   virtual void TransactionCodeGenFinished(const cling::Transaction &T)
+   void TransactionCodeGenFinished(const cling::Transaction &T) override
    {
       assert(fIsCodeGening);
       fIsCodeGening = false;
@@ -105,31 +98,29 @@ class TClingCallbacks : public cling::InterpreterCallbacks {
 
    // The callback is used to update the list of globals in ROOT.
    //
-   virtual void TransactionUnloaded(const cling::Transaction &T);
+   void TransactionUnloaded(const cling::Transaction &T) override;
 
    // The callback is used to clear the autoparsing caches.
    //
-   virtual void TransactionRollback(const cling::Transaction &T);
+   void TransactionRollback(const cling::Transaction &T) override;
 
    ///\brief A previous definition has been shadowed; invalidate TCling' stored
    /// data about the old (global) decl.
-   virtual void DefinitionShadowed(const clang::NamedDecl *D);
+   void DefinitionShadowed(const clang::NamedDecl *D) override;
 
    // Used to inform client about a new decl read by the ASTReader.
    //
-   virtual void DeclDeserialized(const clang::Decl* D);
+   void DeclDeserialized(const clang::Decl *D) override;
 
-   virtual void LibraryLoaded(const void* dyLibHandle,
-                              llvm::StringRef canonicalName);
-   virtual void LibraryUnloaded(const void* dyLibHandle,
-                                llvm::StringRef canonicalName);
+   void LibraryLoaded(const void *dyLibHandle, llvm::StringRef canonicalName) override;
+   void LibraryUnloaded(const void *dyLibHandle, llvm::StringRef canonicalName) override;
 
-   virtual void PrintStackTrace();
+   void PrintStackTrace() override;
 
-   virtual void *EnteringUserCode();
-   virtual void ReturnedFromUserCode(void *stateInfo);
-   virtual void *LockCompilationDuringUserCodeExecution();
-   virtual void UnlockCompilationDuringUserCodeExecution(void *StateInfo);
+   void *EnteringUserCode() override;
+   void ReturnedFromUserCode(void *stateInfo) override;
+   void *LockCompilationDuringUserCodeExecution() override;
+   void UnlockCompilationDuringUserCodeExecution(void *StateInfo) override;
 
 private:
    bool tryAutoParseInternal(llvm::StringRef Name, clang::LookupResult &R,

From e96522ae8febccadc0a0b3210c8e44a985a2f832 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Thu, 4 Jul 2019 23:39:25 +0300
Subject: [PATCH 06/20] [tcling] Modernize header file: use inline
 initialization.

---
 core/metacling/src/TClingCallbacks.cxx |  5 +----
 core/metacling/src/TClingCallbacks.h   | 18 +++++++++---------
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/core/metacling/src/TClingCallbacks.cxx b/core/metacling/src/TClingCallbacks.cxx
index 4e1a2a2cfeb3c..434fe15518333 100644
--- a/core/metacling/src/TClingCallbacks.cxx
+++ b/core/metacling/src/TClingCallbacks.cxx
@@ -76,10 +76,7 @@ extern "C" {
                                     cling::Interpreter &interpreter, bool searchSystem);
 }
 
-TClingCallbacks::TClingCallbacks(cling::Interpreter *interp, bool hasCodeGen)
-   : InterpreterCallbacks(interp), fLastLookupCtx(0), fROOTSpecialNamespace(0), fFirstRun(true), fIsAutoloading(false),
-     fIsAutoloadingRecursively(false), fIsAutoParsingSuspended(false), fPPOldFlag(false), fPPChanged(false),
-     fIsCodeGening(false)
+TClingCallbacks::TClingCallbacks(cling::Interpreter *interp, bool hasCodeGen) : InterpreterCallbacks(interp)
 {
    if (hasCodeGen) {
       Transaction* T = 0;
diff --git a/core/metacling/src/TClingCallbacks.h b/core/metacling/src/TClingCallbacks.h
index f37f4d2e1ffee..091cdcc37d3c5 100644
--- a/core/metacling/src/TClingCallbacks.h
+++ b/core/metacling/src/TClingCallbacks.h
@@ -37,15 +37,15 @@ namespace llvm {
 //
 class TClingCallbacks : public cling::InterpreterCallbacks {
 private:
-   void *fLastLookupCtx;
-   clang::NamespaceDecl *fROOTSpecialNamespace;
-   bool fFirstRun;
-   bool fIsAutoLoading;
-   bool fIsAutoLoadingRecursively;
-   bool fIsAutoParsingSuspended;
-   bool fPPOldFlag;
-   bool fPPChanged;
-   bool fIsCodeGening;
+   void *fLastLookupCtx = nullptr;
+   clang::NamespaceDecl *fROOTSpecialNamespace = nullptr;
+   bool fFirstRun = true;
+   bool fIsAutoLoading = false;
+   bool fIsAutoLoadingRecursively = false;
+   bool fIsAutoParsingSuspended = false;
+   bool fPPOldFlag = false;
+   bool fPPChanged = false;
+   bool fIsCodeGening = false;
 
 public:
    TClingCallbacks(cling::Interpreter* interp, bool hasCodeGen);

From 66d6e47afa05ad6127e40d74abd9f0bfc98f091c Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Fri, 5 Jul 2019 11:27:10 +0300
Subject: [PATCH 07/20] [cxxmodules] Tighten the findInGlobalModuleIndex
 routine.

All reasibility checks are done in the routine itself and we also call it,
when we need to complete type through LookupObject(TagDecl*).
---
 core/metacling/src/TClingCallbacks.cxx | 41 +++++++++++++++-----------
 core/metacling/src/TClingCallbacks.h   |  2 ++
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/core/metacling/src/TClingCallbacks.cxx b/core/metacling/src/TClingCallbacks.cxx
index 434fe15518333..2d176da42c788 100644
--- a/core/metacling/src/TClingCallbacks.cxx
+++ b/core/metacling/src/TClingCallbacks.cxx
@@ -284,12 +284,27 @@ bool TClingCallbacks::LookupObject(LookupResult &R, Scope *S) {
    return tryResolveAtRuntimeInternal(R, S);
 }
 
-static bool findInGlobalIndex(cling::Interpreter &Interp, DeclarationName Name, bool loadFirstMatchOnly = true)
+bool TClingCallbacks::findInGlobalModuleIndex(DeclarationName Name, bool loadFirstMatchOnly /*=true*/) const
 {
-   GlobalModuleIndex *Index = Interp.getCI()->getModuleManager()->getGlobalIndex();
+   const CompilerInstance *CI = m_Interpreter->getCI();
+   const LangOptions &LangOpts = CI->getPreprocessor().getLangOpts();
+
+   if (!LangOpts.Modules)
+      return false;
+
+   // We are currently building a module, we should not import .
+   if (LangOpts.isCompilingModule())
+      return false;
+
+   if (fIsCodeGening)
+      return false;
+
+   GlobalModuleIndex *Index = CI->getModuleManager()->getGlobalIndex();
    if (!Index)
       return false;
 
+   // FIXME: We should load only the first available and rely on other callbacks
+   // such as RequireCompleteType and LookupUnqualified to load all.
    GlobalModuleIndex::FileNameHitSet FoundModules;
 
    // Find the modules that reference the identifier.
@@ -297,7 +312,7 @@ static bool findInGlobalIndex(cling::Interpreter &Interp, DeclarationName Name,
    if (Index->lookupIdentifier(Name.getAsString(), FoundModules)) {
       for (auto FileName : FoundModules) {
          StringRef ModuleName = llvm::sys::path::stem(*FileName);
-         Interp.loadModule(ModuleName);
+         m_Interpreter->loadModule(ModuleName);
          if (loadFirstMatchOnly)
             break;
       }
@@ -314,20 +329,8 @@ bool TClingCallbacks::LookupObject(const DeclContext* DC, DeclarationName Name)
 
    if (!IsAutoLoadingEnabled() || fIsAutoLoadingRecursively) return false;
 
-   // We are currently building a module, we should not autoload.
-   Sema &SemaR = m_Interpreter->getSema();
-   const LangOptions &LangOpts = SemaR.getPreprocessor().getLangOpts();
-   if (LangOpts.Modules) {
-      if (LangOpts.isCompilingModule())
-         return false;
-      if (fIsCodeGening)
-         return false;
-
-      // FIXME: We should load only the first available and rely on other callbacks
-      // such as RequireCompleteType and LookupUnqualified to load all.
-      if (findInGlobalIndex(*m_Interpreter, Name, /*loadFirstMatchOnly*/ false))
-         return true;
-   }
+   if (findInGlobalModuleIndex(Name, /*loadFirstMatchOnly*/ false))
+      return true;
 
    if (Name.getNameKind() != DeclarationName::Identifier)
       return false;
@@ -346,6 +349,7 @@ bool TClingCallbacks::LookupObject(const DeclContext* DC, DeclarationName Name)
    if (primaryDC != DC)
       return false;
 
+   Sema &SemaR = m_Interpreter->getSema();
    LookupResult R(SemaR, Name, SourceLocation(), Sema::LookupOrdinaryName);
    R.suppressDiagnostics();
    // We need the qualified name for TCling to find the right library.
@@ -380,6 +384,9 @@ bool TClingCallbacks::LookupObject(clang::TagDecl* Tag) {
    // Clang needs Tag's complete definition. Can we parse it?
    if (fIsAutoLoadingRecursively || fIsAutoParsingSuspended) return false;
 
+   if (findInGlobalModuleIndex(Tag->getDeclName(), /*loadFirstMatchOnly*/ false))
+      return true;
+
    Sema &SemaR = m_Interpreter->getSema();
 
    SourceLocation Loc = Tag->getLocation();
diff --git a/core/metacling/src/TClingCallbacks.h b/core/metacling/src/TClingCallbacks.h
index 091cdcc37d3c5..13ec82fba567a 100644
--- a/core/metacling/src/TClingCallbacks.h
+++ b/core/metacling/src/TClingCallbacks.h
@@ -15,6 +15,7 @@
 
 namespace clang {
    class Decl;
+   class DeclarationName;
    class LookupResult;
    class NamespaceDecl;
    class Scope;
@@ -129,4 +130,5 @@ class TClingCallbacks : public cling::InterpreterCallbacks {
    bool tryResolveAtRuntimeInternal(clang::LookupResult &R, clang::Scope *S);
    bool shouldResolveAtRuntime(clang::LookupResult &R, clang::Scope *S);
    bool tryInjectImplicitAutoKeyword(clang::LookupResult &R, clang::Scope *S);
+   bool findInGlobalModuleIndex(clang::DeclarationName Name, bool loadFirstMatchOnly = true) const;
 };

From 9822d02df70caa42aedd94542d4022fc7a85eb4b Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Fri, 5 Jul 2019 11:29:17 +0300
Subject: [PATCH 08/20] [cxxmodules] Preload Hist because it has the same issue
 as Gpad.

The issue is that, it does not contain the identifier for TH1.
---
 core/metacling/src/TCling.cxx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx
index 8ee9d08f83a9e..7ac164af79b55 100644
--- a/core/metacling/src/TCling.cxx
+++ b/core/metacling/src/TCling.cxx
@@ -1170,7 +1170,7 @@ static void RegisterCommonCxxModules(cling::Interpreter &clingInterp)
       LoadModules(CommonModules, clingInterp);
 
       // These modules should not be preloaded but they fix issues.
-      std::vector<std::string> FIXMEModules = {"Gpad"};
+      std::vector<std::string> FIXMEModules = {"Hist", "Gpad"};
       LoadModules(FIXMEModules, clingInterp);
 
       loadGlobalModuleIndex(SourceLocation(), clingInterp);

From 07662602d305d11c84936e05a2c0319459ee991a Mon Sep 17 00:00:00 2001
From: Arpitha <arpitharaghunandan@gmail.com>
Date: Fri, 12 Jul 2019 22:31:08 +0530
Subject: [PATCH 09/20] Preload Graf for the same reason as Hist and Gpad

It does not contain identifier TFrame
---
 core/metacling/src/TCling.cxx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx
index 7ac164af79b55..270cc4d276e6c 100644
--- a/core/metacling/src/TCling.cxx
+++ b/core/metacling/src/TCling.cxx
@@ -1170,7 +1170,7 @@ static void RegisterCommonCxxModules(cling::Interpreter &clingInterp)
       LoadModules(CommonModules, clingInterp);
 
       // These modules should not be preloaded but they fix issues.
-      std::vector<std::string> FIXMEModules = {"Hist", "Gpad"};
+      std::vector<std::string> FIXMEModules = {"Hist", "Gpad", "Graf"};
       LoadModules(FIXMEModules, clingInterp);
 
       loadGlobalModuleIndex(SourceLocation(), clingInterp);

From a77d5edbb716dacfcb78821967a17c5f676fb767 Mon Sep 17 00:00:00 2001
From: Arpitha <arpitharaghunandan@gmail.com>
Date: Thu, 25 Jul 2019 19:31:20 +0530
Subject: [PATCH 10/20] Add GenVector FIXMEModules

Fixes some failing tests
---
 core/metacling/src/TCling.cxx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx
index 270cc4d276e6c..37aff57b1d724 100644
--- a/core/metacling/src/TCling.cxx
+++ b/core/metacling/src/TCling.cxx
@@ -1170,7 +1170,7 @@ static void RegisterCommonCxxModules(cling::Interpreter &clingInterp)
       LoadModules(CommonModules, clingInterp);
 
       // These modules should not be preloaded but they fix issues.
-      std::vector<std::string> FIXMEModules = {"Hist", "Gpad", "Graf"};
+      std::vector<std::string> FIXMEModules = {"Hist", "Gpad", "Graf", "GenVector"};
       LoadModules(FIXMEModules, clingInterp);
 
       loadGlobalModuleIndex(SourceLocation(), clingInterp);

From 661f9ddb68e60298c9af3cde91a6667406e2ca5a Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Sat, 25 Jan 2020 17:52:00 +0200
Subject: [PATCH 11/20] Do not resolve from require complete type.

---
 core/metacling/src/TClingCallbacks.cxx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/core/metacling/src/TClingCallbacks.cxx b/core/metacling/src/TClingCallbacks.cxx
index 2d176da42c788..98f3403dc2e4b 100644
--- a/core/metacling/src/TClingCallbacks.cxx
+++ b/core/metacling/src/TClingCallbacks.cxx
@@ -384,8 +384,8 @@ bool TClingCallbacks::LookupObject(clang::TagDecl* Tag) {
    // Clang needs Tag's complete definition. Can we parse it?
    if (fIsAutoLoadingRecursively || fIsAutoParsingSuspended) return false;
 
-   if (findInGlobalModuleIndex(Tag->getDeclName(), /*loadFirstMatchOnly*/ false))
-      return true;
+   // if (findInGlobalModuleIndex(Tag->getDeclName(), /*loadFirstMatchOnly*/false))
+   //    return true;
 
    Sema &SemaR = m_Interpreter->getSema();
 

From 1d59f46fa7409bb26b1f0a078cf1e58be95130fa Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Tue, 28 Jan 2020 11:22:55 +0200
Subject: [PATCH 12/20] If the identifier is unknown, return false.

---
 .../src/tools/clang/lib/Serialization/GlobalModuleIndex.cpp   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/interpreter/llvm/src/tools/clang/lib/Serialization/GlobalModuleIndex.cpp b/interpreter/llvm/src/tools/clang/lib/Serialization/GlobalModuleIndex.cpp
index b98fa26977241..9d5bc108bf54b 100644
--- a/interpreter/llvm/src/tools/clang/lib/Serialization/GlobalModuleIndex.cpp
+++ b/interpreter/llvm/src/tools/clang/lib/Serialization/GlobalModuleIndex.cpp
@@ -300,7 +300,7 @@ bool GlobalModuleIndex::lookupIdentifier(StringRef Name, HitSet &Hits) {
     = *static_cast<IdentifierIndexTable *>(IdentifierIndex);
   IdentifierIndexTable::iterator Known = Table.find(Name);
   if (Known == Table.end()) {
-    return true;
+    return false;
   }
 
   SmallVector<unsigned, 2> ModuleIDs = *Known;
@@ -326,7 +326,7 @@ bool GlobalModuleIndex::lookupIdentifier(StringRef Name, FileNameHitSet &Hits) {
       *static_cast<IdentifierIndexTable *>(IdentifierIndex);
   IdentifierIndexTable::iterator Known = Table.find(Name);
   if (Known == Table.end()) {
-    return true;
+    return false;
   }
 
   SmallVector<unsigned, 2> ModuleIDs = *Known;

From 4b3814306ed8c766d6f8d57ec9906d885543a679 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Tue, 28 Jan 2020 11:17:37 +0200
Subject: [PATCH 13/20] Rename the interface, add documentation, make it bool.

---
 core/metacling/src/TCling.cxx | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx
index 37aff57b1d724..9f5d55f27a177 100644
--- a/core/metacling/src/TCling.cxx
+++ b/core/metacling/src/TCling.cxx
@@ -1075,7 +1075,8 @@ static bool IsFromRootCling() {
   return foundSymbol;
 }
 
-static std::string GetModuleNameAsString(clang::Module *M, const clang::Preprocessor &PP)
+/// Checks if there is an ASTFile on disk for the given module \c M.
+static bool HasASTFileOnDisk(clang::Module *M, const clang::Preprocessor &PP)
 {
    const HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts();
 
@@ -1083,11 +1084,7 @@ static std::string GetModuleNameAsString(clang::Module *M, const clang::Preproce
    if (!HSOpts.PrebuiltModulePaths.empty())
       // Load the module from *only* in the prebuilt module path.
       ModuleFileName = PP.getHeaderSearchInfo().getModuleFileName(M->Name, /*ModuleMapPath*/"", /*UsePrebuiltPath*/ true);
-   if (ModuleFileName.empty()) return "";
-
-   std::string ModuleName = llvm::sys::path::filename(ModuleFileName);
-   // Return stem of the filename
-   return std::string(llvm::sys::path::stem(ModuleName));
+   return !ModuleFileName.empty();
 }
 
 static bool HaveFullGlobalModuleIndex = false;
@@ -1115,11 +1112,10 @@ static GlobalModuleIndex *loadGlobalModuleIndex(SourceLocation TriggerLoc, cling
       bool RecreateIndex = false;
       for (ModuleMap::module_iterator I = MMap.module_begin(), E = MMap.module_end(); I != E; ++I) {
          Module *TheModule = I->second;
-         // We do want the index only of the prebuilt modules
-         std::string ModuleName = GetModuleNameAsString(TheModule, PP);
-         if (ModuleName.empty())
+         // We want the index only of the prebuilt modules.
+         if (!HasASTFileOnDisk(TheModule, PP))
             continue;
-         LoadModule(ModuleName, interp);
+         LoadModule(TheModule->Name, interp);
          RecreateIndex = true;
       }
       if (RecreateIndex) {

From 9c06b0d222d7763d793f25fef63343ebed333714 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Tue, 28 Jan 2020 11:19:13 +0200
Subject: [PATCH 14/20] Preload modules which are not in the index.

---
 core/metacling/src/TCling.cxx                 | 33 ++++++++++++++++++-
 .../clang/Serialization/GlobalModuleIndex.h   |  3 ++
 .../lib/Serialization/GlobalModuleIndex.cpp   |  7 ++++
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx
index 9f5d55f27a177..36b0aa0543453 100644
--- a/core/metacling/src/TCling.cxx
+++ b/core/metacling/src/TCling.cxx
@@ -1076,7 +1076,7 @@ static bool IsFromRootCling() {
 }
 
 /// Checks if there is an ASTFile on disk for the given module \c M.
-static bool HasASTFileOnDisk(clang::Module *M, const clang::Preprocessor &PP)
+static bool HasASTFileOnDisk(clang::Module *M, const clang::Preprocessor &PP, std::string *FullFileName = nullptr)
 {
    const HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts();
 
@@ -1084,6 +1084,9 @@ static bool HasASTFileOnDisk(clang::Module *M, const clang::Preprocessor &PP)
    if (!HSOpts.PrebuiltModulePaths.empty())
       // Load the module from *only* in the prebuilt module path.
       ModuleFileName = PP.getHeaderSearchInfo().getModuleFileName(M->Name, /*ModuleMapPath*/"", /*UsePrebuiltPath*/ true);
+   if (FullFileName)
+      *FullFileName = ModuleFileName;
+
    return !ModuleFileName.empty();
 }
 
@@ -1170,6 +1173,34 @@ static void RegisterCommonCxxModules(cling::Interpreter &clingInterp)
       LoadModules(FIXMEModules, clingInterp);
 
       loadGlobalModuleIndex(SourceLocation(), clingInterp);
+      clang::CompilerInstance &CI = *clingInterp.getCI();
+      if (GlobalModuleIndex *GlobalIndex = CI.getModuleManager()->getGlobalIndex()) {
+         llvm::StringSet<> KnownModuleFileNames;
+         GlobalIndex->getKnownModuleFileNames(KnownModuleFileNames);
+
+         clang::Preprocessor &PP = CI.getPreprocessor();
+         ModuleMap &MMap = PP.getHeaderSearchInfo().getModuleMap();
+         for (auto I = MMap.module_begin(), E = MMap.module_end(); I != E; ++I) {
+            clang::Module *M = I->second;
+            assert(M);
+
+            // We want to load only already created modules.
+            std::string FullASTFilePath;
+            if (!HasASTFileOnDisk(M, PP, &FullASTFilePath))
+               continue;
+
+            if (KnownModuleFileNames.count(FullASTFilePath))
+               continue;
+
+            if (!M->IsMissingRequirement) {
+               if (gDebug > 2)
+                  ::Info("TCling::__RegisterCommonCxxModules", "Preloading %s because it is not in GMI. \n",
+                         M->Name.data());
+
+               LoadModule(M->Name, clingInterp);
+            }
+         }
+      }
    }
 
    // Check that the gROOT macro was exported by any core module.
diff --git a/interpreter/llvm/src/tools/clang/include/clang/Serialization/GlobalModuleIndex.h b/interpreter/llvm/src/tools/clang/include/clang/Serialization/GlobalModuleIndex.h
index 299b80982b0c7..7503a296ea0c2 100644
--- a/interpreter/llvm/src/tools/clang/include/clang/Serialization/GlobalModuleIndex.h
+++ b/interpreter/llvm/src/tools/clang/include/clang/Serialization/GlobalModuleIndex.h
@@ -45,6 +45,7 @@ namespace serialization {
 using llvm::SmallVector;
 using llvm::SmallVectorImpl;
 using llvm::StringRef;
+using llvm::StringSet;
 using serialization::ModuleFile;
 
 /// \brief A global index for a set of module files, providing information about
@@ -160,6 +161,8 @@ class GlobalModuleIndex {
   /// have been indexed.
   void getKnownModules(SmallVectorImpl<ModuleFile *> &ModuleFiles);
 
+  void getKnownModuleFileNames(StringSet<> &ModuleFiles);
+
   /// \brief Retrieve the set of module files on which the given module file
   /// directly depends.
   void getModuleDependencies(ModuleFile *File,
diff --git a/interpreter/llvm/src/tools/clang/lib/Serialization/GlobalModuleIndex.cpp b/interpreter/llvm/src/tools/clang/lib/Serialization/GlobalModuleIndex.cpp
index 9d5bc108bf54b..d6e07a83f08ac 100644
--- a/interpreter/llvm/src/tools/clang/lib/Serialization/GlobalModuleIndex.cpp
+++ b/interpreter/llvm/src/tools/clang/lib/Serialization/GlobalModuleIndex.cpp
@@ -269,6 +269,13 @@ GlobalModuleIndex::getKnownModules(SmallVectorImpl<ModuleFile *> &ModuleFiles) {
   }
 }
 
+void GlobalModuleIndex::getKnownModuleFileNames(StringSet<> &ModuleFiles) {
+  ModuleFiles.clear();
+  for (unsigned I = 0, N = Modules.size(); I != N; ++I) {
+    ModuleFiles[Modules[I].FileName];
+  }
+}
+
 void GlobalModuleIndex::getModuleDependencies(
        ModuleFile *File,
        SmallVectorImpl<ModuleFile *> &Dependencies) {

From d84e4a0c50a315ef38a0f09a9b74490ad47a90a4 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Tue, 28 Jan 2020 15:44:59 +0200
Subject: [PATCH 15/20] Add help how to automatically incorporate clang-format
 changes.

This does not require resolving conflicts or bloating the history.
---
 .ci/format_script.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.ci/format_script.sh b/.ci/format_script.sh
index 5f884be45e513..08ba5007d641f 100755
--- a/.ci/format_script.sh
+++ b/.ci/format_script.sh
@@ -22,7 +22,9 @@ else
   echo -e "\n\nPlease apply the code formatting changes without bloating the history."
   echo -e "\tConsider running:"
   echo -e "\t\tgit checkout $TRAVIS_BRANCH"
-  echo -e "\t\tgit rebase -i --autosquash -x 'git-clang-format-7 master && git commit -a --amend --no-edit' master"
+  echo -e "\t\tgit rebase -i -x \"git-clang-format-7 master && git commit -a --allow-empty --fixup=HEAD\" --strategy-option=theirs origin/master"
+  echo -e "\t Then inspect the results with git log --oneline"
+  echo -e "\t Then squash without poluting the history with: git rebase --autosquash -i master"
 
   exit 1
 fi

From da1ec924d182a9b43ceb633c5083deab95612fd6 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Wed, 29 Jan 2020 20:37:13 +0200
Subject: [PATCH 16/20] Preload Tree to fix the messy
 roottest-root-io-newstl-make

---
 core/metacling/src/TCling.cxx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx
index 36b0aa0543453..c373dad56b3bf 100644
--- a/core/metacling/src/TCling.cxx
+++ b/core/metacling/src/TCling.cxx
@@ -1169,7 +1169,7 @@ static void RegisterCommonCxxModules(cling::Interpreter &clingInterp)
       LoadModules(CommonModules, clingInterp);
 
       // These modules should not be preloaded but they fix issues.
-      std::vector<std::string> FIXMEModules = {"Hist", "Gpad", "Graf", "GenVector"};
+      std::vector<std::string> FIXMEModules = {"Hist", "Gpad", "Graf", "GenVector", "Tree"};
       LoadModules(FIXMEModules, clingInterp);
 
       loadGlobalModuleIndex(SourceLocation(), clingInterp);

From c0014fde4307193e08e34003910539219085e6f5 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Mon, 3 Feb 2020 11:44:15 +0200
Subject: [PATCH 17/20] Do not load recursively modules.

---
 core/metacling/src/TClingCallbacks.cxx | 10 +++++++++-
 core/metacling/src/TClingCallbacks.h   |  3 ++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/core/metacling/src/TClingCallbacks.cxx b/core/metacling/src/TClingCallbacks.cxx
index 98f3403dc2e4b..0c423742d25a7 100644
--- a/core/metacling/src/TClingCallbacks.cxx
+++ b/core/metacling/src/TClingCallbacks.cxx
@@ -284,7 +284,7 @@ bool TClingCallbacks::LookupObject(LookupResult &R, Scope *S) {
    return tryResolveAtRuntimeInternal(R, S);
 }
 
-bool TClingCallbacks::findInGlobalModuleIndex(DeclarationName Name, bool loadFirstMatchOnly /*=true*/) const
+bool TClingCallbacks::findInGlobalModuleIndex(DeclarationName Name, bool loadFirstMatchOnly /*=true*/)
 {
    const CompilerInstance *CI = m_Interpreter->getCI();
    const LangOptions &LangOpts = CI->getPreprocessor().getLangOpts();
@@ -312,7 +312,9 @@ bool TClingCallbacks::findInGlobalModuleIndex(DeclarationName Name, bool loadFir
    if (Index->lookupIdentifier(Name.getAsString(), FoundModules)) {
       for (auto FileName : FoundModules) {
          StringRef ModuleName = llvm::sys::path::stem(*FileName);
+         fIsLoadingModule = true;
          m_Interpreter->loadModule(ModuleName);
+         fIsLoadingModule = false;
          if (loadFirstMatchOnly)
             break;
       }
@@ -327,6 +329,9 @@ bool TClingCallbacks::LookupObject(const DeclContext* DC, DeclarationName Name)
       return false;
    }
 
+   if (fIsLoadingModule)
+      return false;
+
    if (!IsAutoLoadingEnabled() || fIsAutoLoadingRecursively) return false;
 
    if (findInGlobalModuleIndex(Name, /*loadFirstMatchOnly*/ false))
@@ -381,6 +386,9 @@ bool TClingCallbacks::LookupObject(clang::TagDecl* Tag) {
       return false;
    }
 
+   if (fIsLoadingModule)
+      return false;
+
    // Clang needs Tag's complete definition. Can we parse it?
    if (fIsAutoLoadingRecursively || fIsAutoParsingSuspended) return false;
 
diff --git a/core/metacling/src/TClingCallbacks.h b/core/metacling/src/TClingCallbacks.h
index 13ec82fba567a..5249aa1bfe1d1 100644
--- a/core/metacling/src/TClingCallbacks.h
+++ b/core/metacling/src/TClingCallbacks.h
@@ -47,6 +47,7 @@ class TClingCallbacks : public cling::InterpreterCallbacks {
    bool fPPOldFlag = false;
    bool fPPChanged = false;
    bool fIsCodeGening = false;
+   bool fIsLoadingModule = false;
 
 public:
    TClingCallbacks(cling::Interpreter* interp, bool hasCodeGen);
@@ -130,5 +131,5 @@ class TClingCallbacks : public cling::InterpreterCallbacks {
    bool tryResolveAtRuntimeInternal(clang::LookupResult &R, clang::Scope *S);
    bool shouldResolveAtRuntime(clang::LookupResult &R, clang::Scope *S);
    bool tryInjectImplicitAutoKeyword(clang::LookupResult &R, clang::Scope *S);
-   bool findInGlobalModuleIndex(clang::DeclarationName Name, bool loadFirstMatchOnly = true) const;
+   bool findInGlobalModuleIndex(clang::DeclarationName Name, bool loadFirstMatchOnly = true);
 };

From 7d936aeb07c3526cb44a42c480700c102212532e Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Mon, 3 Feb 2020 21:36:34 +0200
Subject: [PATCH 18/20] Preload module Physics.

We do that to fix the:
+Warning in <TClass::Init>: no dictionary for class TLorentzVector is available

errors. This is due to the fact that TLorentzVector and such identifiers do not
get to the module.idx. They are read in ::readIndex and visited in ::writeIndex
however it seems that the on-disk writer trait is filtering somehow them out.
---
 core/metacling/src/TCling.cxx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx
index c373dad56b3bf..cf0fe92773646 100644
--- a/core/metacling/src/TCling.cxx
+++ b/core/metacling/src/TCling.cxx
@@ -1169,7 +1169,7 @@ static void RegisterCommonCxxModules(cling::Interpreter &clingInterp)
       LoadModules(CommonModules, clingInterp);
 
       // These modules should not be preloaded but they fix issues.
-      std::vector<std::string> FIXMEModules = {"Hist", "Gpad", "Graf", "GenVector", "Tree"};
+      std::vector<std::string> FIXMEModules = {"Hist", "Gpad", "Graf", "GenVector", "Tree", "Physics"};
       LoadModules(FIXMEModules, clingInterp);
 
       loadGlobalModuleIndex(SourceLocation(), clingInterp);

From 0eea44a5018cceac4bd305fdb6985d6ab68d5d3c Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Tue, 11 Feb 2020 13:40:45 +0200
Subject: [PATCH 19/20] Preload Smatrix, TreePlayer, Proof and Geom.

This is needed due to see this commit: fc24e98392a9ddf92473079a6a9b077f7ac69cde
---
 core/metacling/src/TCling.cxx | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx
index cf0fe92773646..0b8bc4ab925d1 100644
--- a/core/metacling/src/TCling.cxx
+++ b/core/metacling/src/TCling.cxx
@@ -1169,7 +1169,10 @@ static void RegisterCommonCxxModules(cling::Interpreter &clingInterp)
       LoadModules(CommonModules, clingInterp);
 
       // These modules should not be preloaded but they fix issues.
-      std::vector<std::string> FIXMEModules = {"Hist", "Gpad", "Graf", "GenVector", "Tree", "Physics"};
+      std::vector<std::string> FIXMEModules = {"Hist", "Gpad", "Graf",
+                                               "GenVector", "Smatrix", "Tree",
+                                               "TreePlayer", "Physics",
+                                               "Proof", "Geom"};
       LoadModules(FIXMEModules, clingInterp);
 
       loadGlobalModuleIndex(SourceLocation(), clingInterp);

From 774c08f2592cff364617dec141364128262119d3 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Sun, 23 Feb 2020 13:33:30 +0200
Subject: [PATCH 20/20] [cxxmodules] Make the global module index opt-in.

By setting the ROOT_EXPERIMENTAL_GMI env variable. This is useful for comparing
performance results from both implementations.

ROOT_EXPERIMENTAL_GMI is temporary and will go away as soon as we switch to the
GMI by default.
---
 core/metacling/src/TCling.cxx | 70 +++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 23 deletions(-)

diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx
index 0b8bc4ab925d1..50d171ffa26d6 100644
--- a/core/metacling/src/TCling.cxx
+++ b/core/metacling/src/TCling.cxx
@@ -1057,6 +1057,10 @@ static bool LoadModule(const std::string &ModuleName, cling::Interpreter &interp
    std::string currentDir = gSystem->WorkingDirectory();
    assert(!currentDir.empty());
    gCling->RegisterPrebuiltModulePath(currentDir);
+   if (gDebug > 2)
+      ::Info("TCling::__LoadModule", "Preloading module %s. \n",
+             ModuleName.c_str());
+
    return interp.loadModule(ModuleName, /*Complain=*/true);
 }
 
@@ -1132,7 +1136,7 @@ static GlobalModuleIndex *loadGlobalModuleIndex(SourceLocation TriggerLoc, cling
    return GlobalIndex;
 }
 
-static void RegisterCommonCxxModules(cling::Interpreter &clingInterp)
+static void RegisterCxxModules(cling::Interpreter &clingInterp)
 {
    if (!clingInterp.getCI()->getLangOpts().Modules)
       return;
@@ -1160,8 +1164,8 @@ static void RegisterCommonCxxModules(cling::Interpreter &clingInterp)
    LoadModules(CoreModules, clingInterp);
 
    // FIXME: Reducing those will let us be less dependent on rootmap files
-   // static constexpr std::array<const char *, 3> ExcludeModules = {
-   //    {"Rtools", "RSQLite", "RInterface"}};
+   static constexpr std::array<const char *, 3> ExcludeModules = {
+      {"Rtools", "RSQLite", "RInterface"}};
 
    // Take this branch only from ROOT because we don't need to preload modules in rootcling
    if (!IsFromRootCling()) {
@@ -1175,35 +1179,55 @@ static void RegisterCommonCxxModules(cling::Interpreter &clingInterp)
                                                "Proof", "Geom"};
       LoadModules(FIXMEModules, clingInterp);
 
-      loadGlobalModuleIndex(SourceLocation(), clingInterp);
       clang::CompilerInstance &CI = *clingInterp.getCI();
-      if (GlobalModuleIndex *GlobalIndex = CI.getModuleManager()->getGlobalIndex()) {
-         llvm::StringSet<> KnownModuleFileNames;
+      GlobalModuleIndex *GlobalIndex = nullptr;
+      if (gSystem->Getenv("ROOT_EXPERIMENTAL_GMI")) {
+         loadGlobalModuleIndex(SourceLocation(), clingInterp);
+         GlobalIndex = CI.getModuleManager()->getGlobalIndex();
+      }
+      llvm::StringSet<> KnownModuleFileNames;
+      if (GlobalIndex)
          GlobalIndex->getKnownModuleFileNames(KnownModuleFileNames);
 
-         clang::Preprocessor &PP = CI.getPreprocessor();
-         ModuleMap &MMap = PP.getHeaderSearchInfo().getModuleMap();
-         for (auto I = MMap.module_begin(), E = MMap.module_end(); I != E; ++I) {
-            clang::Module *M = I->second;
-            assert(M);
+      clang::Preprocessor &PP = CI.getPreprocessor();
+      std::vector<std::string> PendingModules;
+      PendingModules.reserve(256);
+      ModuleMap &MMap = PP.getHeaderSearchInfo().getModuleMap();
+      for (auto I = MMap.module_begin(), E = MMap.module_end(); I != E; ++I) {
+         clang::Module *M = I->second;
+         assert(M);
 
-            // We want to load only already created modules.
-            std::string FullASTFilePath;
-            if (!HasASTFileOnDisk(M, PP, &FullASTFilePath))
-               continue;
+         // We want to load only already created modules.
+         std::string FullASTFilePath;
+         if (!HasASTFileOnDisk(M, PP, &FullASTFilePath))
+            continue;
 
-            if (KnownModuleFileNames.count(FullASTFilePath))
-               continue;
+         if (GlobalIndex && KnownModuleFileNames.count(FullASTFilePath))
+            continue;
 
-            if (!M->IsMissingRequirement) {
-               if (gDebug > 2)
-                  ::Info("TCling::__RegisterCommonCxxModules", "Preloading %s because it is not in GMI. \n",
-                         M->Name.data());
+         if (M->IsMissingRequirement)
+            continue;
+
+         if (GlobalIndex)
+            LoadModule(M->Name, clingInterp);
+         else {
+            // FIXME: We may be able to remove those checks as cling::loadModule
+            // checks if a module was alredy loaded.
+            if (std::find(CoreModules.begin(), CoreModules.end(), M->Name) != CoreModules.end())
+               continue; // This is a core module which was already loaded.
 
+            if (std::find(ExcludeModules.begin(), ExcludeModules.end(), M->Name) != ExcludeModules.end())
+               continue;
+
+            // Load system modules now and delay the other modules after we have
+            // loaded all system ones.
+            if (M->IsSystem)
                LoadModule(M->Name, clingInterp);
-            }
+            else
+               PendingModules.push_back(M->Name);
          }
       }
+      LoadModules(PendingModules, clingInterp);
    }
 
    // Check that the gROOT macro was exported by any core module.
@@ -1464,7 +1488,7 @@ TCling::TCling(const char *name, const char *title, const char* const argv[])
    static llvm::raw_fd_ostream fMPOuts (STDOUT_FILENO, /*ShouldClose*/false);
    fMetaProcessor = llvm::make_unique<cling::MetaProcessor>(*fInterpreter, fMPOuts);
 
-   RegisterCommonCxxModules(*fInterpreter);
+   RegisterCxxModules(*fInterpreter);
    RegisterPreIncludedHeaders(*fInterpreter);
 
    // We are now ready (enough is loaded) to init the list of opaque typedefs.