[cfe-commits] r138030 - in /cfe/trunk: include/clang/Serialization/ASTReader.h lib/Serialization/ASTReader.cpp test/Modules/ test/Modules/Inputs/ test/Modules/Inputs/diamond_bottom.h test/Modules/Inputs/diamond_left.h test/Modules/Inputs/diamond_right.h test/Modules/Inputs/diamond_top.h test/Modules/diamond.c

Douglas Gregor dgregor at apple.com
Thu Aug 18 19:29:29 PDT 2011


Author: dgregor
Date: Thu Aug 18 21:29:29 2011
New Revision: 138030

URL: http://llvm.org/viewvc/llvm-project?rev=138030&view=rev
Log:
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.

Note that this version moves the file-opening logic into the module
manager, rather than splitting it between the module manager and the
AST reader. More importantly, it properly handles the
weird-but-possibly-useful case of loading an AST file from "-".

Added:
    cfe/trunk/test/Modules/
      - copied from r137970, cfe/trunk/test/Modules/
    cfe/trunk/test/Modules/Inputs/
      - copied from r137970, cfe/trunk/test/Modules/Inputs/
    cfe/trunk/test/Modules/Inputs/diamond_bottom.h
      - copied unchanged from r137970, cfe/trunk/test/Modules/Inputs/diamond_bottom.h
    cfe/trunk/test/Modules/Inputs/diamond_left.h
      - copied unchanged from r137970, cfe/trunk/test/Modules/Inputs/diamond_left.h
    cfe/trunk/test/Modules/Inputs/diamond_right.h
      - copied unchanged from r137970, cfe/trunk/test/Modules/Inputs/diamond_right.h
    cfe/trunk/test/Modules/Inputs/diamond_top.h
      - copied unchanged from r137970, cfe/trunk/test/Modules/Inputs/diamond_top.h
    cfe/trunk/test/Modules/diamond.c
      - copied unchanged from r137970, cfe/trunk/test/Modules/diamond.c
Modified:
    cfe/trunk/include/clang/Serialization/ASTReader.h
    cfe/trunk/lib/Serialization/ASTReader.cpp

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=138030&r1=138029&r2=138030&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Thu Aug 18 21:29:29 2011
@@ -507,8 +507,15 @@
   ///
   /// \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);
+  ///
+  /// \param ErrorStr Will be set to a non-empty string if any errors occurred
+  /// while trying to load the module.
+  ///
+  /// \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,
+            std::string &ErrorStr);
   
   /// \brief Add an in-memory buffer the list of known buffers
   void addInMemoryBuffer(StringRef FileName, llvm::MemoryBuffer *Buffer);

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=138030&r1=138029&r2=138030&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Thu Aug 18 21:29:29 2011
@@ -2831,37 +2831,33 @@
 ASTReader::ASTReadResult ASTReader::ReadASTCore(StringRef FileName,
                                                 ModuleKind Type,
                                                 Module *ImportedBy) {
-  Module &F = ModuleMgr.addModule(FileName, Type, ImportedBy);
+  Module *M;
+  bool NewModule;
+  std::string ErrorStr;
+  llvm::tie(M, NewModule) = ModuleMgr.addModule(FileName, Type, ImportedBy,
+                                                ErrorStr);
+
+  if (!M) {
+    // We couldn't load the module.
+    std::string Msg = "Unable to load module \"" + FileName.str() + "\": "
+      + ErrorStr;
+    Error(Msg);
+    return Failure;
+  }
+
+  if (!NewModule) {
+    // We've already loaded this module.
+    return Success;
+  }
 
+  // FIXME: This seems rather a hack. Should CurrentDir be part of the
+  // module?
   if (FileName != "-") {
     CurrentDir = llvm::sys::path::parent_path(FileName);
     if (CurrentDir.empty()) CurrentDir = ".";
   }
 
-  if (llvm::MemoryBuffer *Buffer = ModuleMgr.lookupBuffer(FileName)) {
-    F.Buffer.reset(Buffer);
-    assert(F.Buffer && "Passed null buffer");
-  } else {
-    // Open the AST file.
-    //
-    // FIXME: This shouldn't be here, we should just take a raw_ostream.
-    std::string ErrStr;
-    llvm::error_code ec;
-    if (FileName == "-") {
-      ec = llvm::MemoryBuffer::getSTDIN(F.Buffer);
-      if (ec)
-        ErrStr = ec.message();
-    } else
-      F.Buffer.reset(FileMgr.getBufferForFile(FileName, &ErrStr));
-    if (!F.Buffer) {
-      Error(ErrStr.c_str());
-      return IgnorePCH;
-    }
-  }
-
-  // Initialize the stream
-  F.StreamFile.init((const unsigned char *)F.Buffer->getBufferStart(),
-                    (const unsigned char *)F.Buffer->getBufferEnd());
+  Module &F = *M;
   llvm::BitstreamCursor &Stream = F.Stream;
   Stream.init(F.StreamFile);
   F.SizeInBits = F.Buffer->getBufferSize() * 8;
@@ -5709,25 +5705,57 @@
   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, std::string &ErrorStr) {
   const FileEntry *Entry = FileMgr.getFile(FileName);
-  // FIXME: Check whether we already loaded this module, before 
-  Modules[Entry] = Current;
+  if (!Entry && FileName != "-") {
+    ErrorStr = "file not found";
+    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;
+
+    // Load the contents of the module
+    if (llvm::MemoryBuffer *Buffer = lookupBuffer(FileName)) {
+      // The buffer was already provided for us.
+      assert(Buffer && "Passed null buffer");
+      New->Buffer.reset(Buffer);
+    } else {
+      // Open the AST file.
+      llvm::error_code ec;
+      if (FileName == "-") {
+        ec = llvm::MemoryBuffer::getSTDIN(New->Buffer);
+        if (ec)
+          ErrorStr = ec.message();
+      } else
+        New->Buffer.reset(FileMgr.getBufferForFile(FileName, &ErrorStr));
+
+      if (!New->Buffer)
+        return std::make_pair(static_cast<Module*>(0), false);
+    }
+
+    // Initialize the stream
+    New->StreamFile.init((const unsigned char *)New->Buffer->getBufferStart(),
+                         (const unsigned char *)New->Buffer->getBufferEnd());     }
 
   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, 





More information about the cfe-commits mailing list