Teach ModuleManager::addModule() to check whether a particular module

has already been loaded before allocating a new Module structure. If
the module has already been loaded (uniquing based on file name), then
just return the existing module rather than trying to load it again.

This allows us to load a DAG of modules. Introduce a simple test case
that forms a diamond-shaped module graph, and illustrates that a
source file importing the bottom of the diamond can see declarations
in all four of the modules that make up the diamond.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@137925 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h
index 514d36db919c45462d1b660d7fd3c42361afd669..f00e805863acf5bf4ab9ac2bba4574dfa72a8ce5 100644 (file)
--- a/include/clang/Serialization/ASTReader.h
+++ b/include/clang/Serialization/ASTReader.h
@@ -507,8 +507,11 @@ public:
   ///
   /// \param ImportedBy The module that is importing this module, or NULL if
   /// this module is imported directly by the user.
-  Module &addModule(StringRef FileName, ModuleKind Type,
-                    Module *ImportedBy);
+  ///
+  /// \return A pointer to the module that corresponds to this file name,
+  /// and a boolean indicating whether the module was newly added.
+  std::pair<Module *, bool> 
+  addModule(StringRef FileName, ModuleKind Type, Module *ImportedBy);
   
   /// \brief Add an in-memory buffer the list of known buffers
   void addInMemoryBuffer(StringRef FileName, llvm::MemoryBuffer *Buffer);

diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index d81e9d8a36a47a4198a8be78bc546f26bb97c981..b635b38c10ac5c35bdef2051ec6d2d5a29c995d5 100644 (file)
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -2831,7 +2831,23 @@ ASTReader::ASTReadResult ASTReader::ReadAST(const std::string &FileName,
 ASTReader::ASTReadResult ASTReader::ReadASTCore(StringRef FileName,
                                                 ModuleKind Type,
                                                 Module *ImportedBy) {
-  Module &F = ModuleMgr.addModule(FileName, Type, ImportedBy);
+  Module *M;
+  bool NewModule;
+  llvm::tie(M, NewModule) = ModuleMgr.addModule(FileName, Type, ImportedBy);
+
+  if (!M) {
+    // We couldn't load the module.
+    std::string Msg = "Unable to load module \"" + FileName.str() + "\"";
+    Error(Msg);
+    return Failure;
+  }
+
+  if (!NewModule) {
+    // We've already loaded this module.
+    return Success;
+  }
+
+  Module &F = *M;
 
   if (FileName != "-") {
     CurrentDir = llvm::sys::path::parent_path(FileName);
@@ -5709,25 +5725,34 @@ llvm::MemoryBuffer *ModuleManager::lookupBuffer(StringRef Name) {
   return InMemoryBuffers[Entry];
 }
 
-/// \brief Creates a new module and adds it to the list of known modules
-Module &ModuleManager::addModule(StringRef FileName, ModuleKind Type,
-                                 Module *ImportedBy) {
-  Module *Current = new Module(Type);
-  Current->FileName = FileName.str();
-  Chain.push_back(Current);
-
+std::pair<Module *, bool>
+ModuleManager::addModule(StringRef FileName, ModuleKind Type, 
+                         Module *ImportedBy) {
   const FileEntry *Entry = FileMgr.getFile(FileName);
-  // FIXME: Check whether we already loaded this module, before 
-  Modules[Entry] = Current;
+  if (!Entry)
+    return std::make_pair(static_cast<Module*>(0), false);
+
+  // Check whether we already loaded this module, before 
+  Module *&ModuleEntry = Modules[Entry];
+  bool NewModule = false;
+  if (!ModuleEntry) {
+    // Allocate a new module.
+    Module *New = new Module(Type);
+    New->FileName = FileName.str();
+    Chain.push_back(New);
+  
+    NewModule = true;
+    ModuleEntry = New;
+  }
 
   if (ImportedBy) {
-    Current->ImportedBy.insert(ImportedBy);
-    ImportedBy->Imports.insert(Current);
+    ModuleEntry->ImportedBy.insert(ImportedBy);
+    ImportedBy->Imports.insert(ModuleEntry);
   } else {
-    Current->DirectlyImported = true;
+    ModuleEntry->DirectlyImported = true;
   }
   
-  return *Current;
+  return std::make_pair(ModuleEntry, NewModule);
 }
 
 void ModuleManager::addInMemoryBuffer(StringRef FileName, 

diff --git a/test/Modules/Inputs/diamond_bottom.h b/test/Modules/Inputs/diamond_bottom.h
new file mode 100644 (file)
index 0000000..40afc9b
--- /dev/null
+++ b/test/Modules/Inputs/diamond_bottom.h
@@ -0,0 +1 @@
+char bottom(char *x);

diff --git a/test/Modules/Inputs/diamond_left.h b/test/Modules/Inputs/diamond_left.h
new file mode 100644 (file)
index 0000000..dfdd803
--- /dev/null
+++ b/test/Modules/Inputs/diamond_left.h
@@ -0,0 +1 @@
+float left(float *);

diff --git a/test/Modules/Inputs/diamond_right.h b/test/Modules/Inputs/diamond_right.h
new file mode 100644 (file)
index 0000000..bbed7ec
--- /dev/null
+++ b/test/Modules/Inputs/diamond_right.h
@@ -0,0 +1 @@
+double right(double *);

diff --git a/test/Modules/Inputs/diamond_top.h b/test/Modules/Inputs/diamond_top.h
new file mode 100644 (file)
index 0000000..189687a
--- /dev/null
+++ b/test/Modules/Inputs/diamond_top.h
@@ -0,0 +1 @@
+int top(int *);

diff --git a/test/Modules/diamond.c b/test/Modules/diamond.c
new file mode 100644 (file)
index 0000000..fdec7b3
--- /dev/null
+++ b/test/Modules/diamond.c
@@ -0,0 +1,14 @@
+// in diamond-bottom.h: expected-note{{passing argument to parameter 'x' here}}
+void test_diamond(int i, float f, double d, char c) {
+  top(&i);
+  left(&f);
+  right(&d);
+  bottom(&c);
+  bottom(&d); // expected-warning{{incompatible pointer types passing 'double *' to parameter of type 'char *'}}
+}
+
+// RUN: %clang_cc1 -emit-pch -o %t_top.h.pch %S/Inputs/diamond_top.h
+// RUN: %clang_cc1 -import-module %t_top.h.pch -emit-pch -o %t_left.h.pch %S/Inputs/diamond_left.h
+// RUN: %clang_cc1 -import-module %t_top.h.pch -emit-pch -o %t_right.h.pch %S/Inputs/diamond_right.h
+// RUN: %clang_cc1 -import-module %t_left.h.pch -import-module %t_right.h.pch -emit-pch -o %t_bottom.h.pch %S/Inputs/diamond_bottom.h
+// RUN: %clang_cc1 -import-module %t_bottom.h.pch -verify %s