r293400 - Modules: Return early in ModuleManager::addModule; NFC

Duncan P. N. Exon Smith via cfe-commits cfe-commits at lists.llvm.org
Sat Jan 28 15:22:40 PST 2017


Author: dexonsmith
Date: Sat Jan 28 17:22:40 2017
New Revision: 293400

URL: http://llvm.org/viewvc/llvm-project?rev=293400&view=rev
Log:
Modules: Return early in ModuleManager::addModule; NFC

Invert the main branch in ModuleManager::addModule to return early and
reduce indentation, and clean up a bunch of logic as a result.  I split
out a function called updateModuleImports to avoid triggering code
duplication.

Modified:
    cfe/trunk/lib/Serialization/ModuleManager.cpp

Modified: cfe/trunk/lib/Serialization/ModuleManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ModuleManager.cpp?rev=293400&r1=293399&r2=293400&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ModuleManager.cpp (original)
+++ cfe/trunk/lib/Serialization/ModuleManager.cpp Sat Jan 28 17:22:40 2017
@@ -63,6 +63,19 @@ static bool checkSignature(ASTFileSignat
   return true;
 }
 
+static void updateModuleImports(ModuleFile &MF, ModuleFile *ImportedBy,
+                                SourceLocation ImportLoc) {
+  if (ImportedBy) {
+    MF.ImportedBy.insert(ImportedBy);
+    ImportedBy->Imports.insert(&MF);
+  } else {
+    if (!MF.DirectlyImported)
+      MF.ImportLoc = ImportLoc;
+
+    MF.DirectlyImported = true;
+  }
+}
+
 ModuleManager::AddModuleResult
 ModuleManager::addModule(StringRef FileName, ModuleKind Type,
                          SourceLocation ImportLoc, ModuleFile *ImportedBy,
@@ -95,91 +108,79 @@ ModuleManager::addModule(StringRef FileN
   }
 
   // Check whether we already loaded this module, before
-  ModuleFile *ModuleEntry = Modules[Entry];
-  std::unique_ptr<ModuleFile> NewModule;
-  if (!ModuleEntry) {
-    // Allocate a new module.
-    NewModule = llvm::make_unique<ModuleFile>(Type, Generation);
-    NewModule->Index = Chain.size();
-    NewModule->FileName = FileName.str();
-    NewModule->File = Entry;
-    NewModule->ImportLoc = ImportLoc;
-    NewModule->InputFilesValidationTimestamp = 0;
-
-    if (NewModule->Kind == MK_ImplicitModule) {
-      std::string TimestampFilename = NewModule->getTimestampFilename();
-      vfs::Status Status;
-      // A cached stat value would be fine as well.
-      if (!FileMgr.getNoncachedStatValue(TimestampFilename, Status))
-        NewModule->InputFilesValidationTimestamp =
-            llvm::sys::toTimeT(Status.getLastModificationTime());
-    }
+  if (ModuleFile *ModuleEntry = Modules.lookup(Entry)) {
+    // Check the stored signature.
+    if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr))
+      return OutOfDate;
 
-    // Load the contents of the module
-    if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) {
-      // The buffer was already provided for us.
-      NewModule->Buffer = std::move(Buffer);
-    } else {
-      // Open the AST file.
-      llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buf(
-          (std::error_code()));
-      if (FileName == "-") {
-        Buf = llvm::MemoryBuffer::getSTDIN();
-      } else {
-        // Leave the FileEntry open so if it gets read again by another
-        // ModuleManager it must be the same underlying file.
-        // FIXME: Because FileManager::getFile() doesn't guarantee that it will
-        // give us an open file, this may not be 100% reliable.
-        Buf = FileMgr.getBufferForFile(NewModule->File,
-                                       /*IsVolatile=*/false,
-                                       /*ShouldClose=*/false);
-      }
-
-      if (!Buf) {
-        ErrorStr = Buf.getError().message();
-        return Missing;
-      }
+    Module = ModuleEntry;
+    updateModuleImports(*ModuleEntry, ImportedBy, ImportLoc);
+    return AlreadyLoaded;
+  }
 
-      NewModule->Buffer = std::move(*Buf);
-    }
+  // Allocate a new module.
+  auto NewModule = llvm::make_unique<ModuleFile>(Type, Generation);
+  NewModule->Index = Chain.size();
+  NewModule->FileName = FileName.str();
+  NewModule->File = Entry;
+  NewModule->ImportLoc = ImportLoc;
+  NewModule->InputFilesValidationTimestamp = 0;
+
+  if (NewModule->Kind == MK_ImplicitModule) {
+    std::string TimestampFilename = NewModule->getTimestampFilename();
+    vfs::Status Status;
+    // A cached stat value would be fine as well.
+    if (!FileMgr.getNoncachedStatValue(TimestampFilename, Status))
+      NewModule->InputFilesValidationTimestamp =
+          llvm::sys::toTimeT(Status.getLastModificationTime());
+  }
 
-    // Initialize the stream.
-    NewModule->Data = PCHContainerRdr.ExtractPCH(*NewModule->Buffer);
+  // Load the contents of the module
+  if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) {
+    // The buffer was already provided for us.
+    NewModule->Buffer = std::move(Buffer);
+  } else {
+    // Open the AST file.
+    llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buf((std::error_code()));
+    if (FileName == "-") {
+      Buf = llvm::MemoryBuffer::getSTDIN();
+    } else {
+      // Leave the FileEntry open so if it gets read again by another
+      // ModuleManager it must be the same underlying file.
+      // FIXME: Because FileManager::getFile() doesn't guarantee that it will
+      // give us an open file, this may not be 100% reliable.
+      Buf = FileMgr.getBufferForFile(NewModule->File,
+                                     /*IsVolatile=*/false,
+                                     /*ShouldClose=*/false);
+    }
 
-    // Read the signature eagerly now so that we can check it.
-    if (checkSignature(ReadSignature(NewModule->Data), ExpectedSignature, ErrorStr))
-      return OutOfDate;
+    if (!Buf) {
+      ErrorStr = Buf.getError().message();
+      return Missing;
+    }
 
-    // We're keeping this module.  Update the map entry.
-    ModuleEntry = NewModule.get();
-  } else if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr)) {
-    return OutOfDate;
+    NewModule->Buffer = std::move(*Buf);
   }
 
-  if (ImportedBy) {
-    ModuleEntry->ImportedBy.insert(ImportedBy);
-    ImportedBy->Imports.insert(ModuleEntry);
-  } else {
-    if (!ModuleEntry->DirectlyImported)
-      ModuleEntry->ImportLoc = ImportLoc;
-    
-    ModuleEntry->DirectlyImported = true;
-  }
+  // Initialize the stream.
+  NewModule->Data = PCHContainerRdr.ExtractPCH(*NewModule->Buffer);
 
-  Module = ModuleEntry;
+  // Read the signature eagerly now so that we can check it.
+  if (checkSignature(ReadSignature(NewModule->Data), ExpectedSignature,
+                     ErrorStr))
+    return OutOfDate;
 
-  if (!NewModule)
-    return AlreadyLoaded;
+  // We're keeping this module.  Store it everywhere.
+  Module = Modules[Entry] = NewModule.get();
 
-  assert(!Modules[Entry] && "module loaded twice");
-  Modules[Entry] = ModuleEntry;
+  updateModuleImports(*NewModule, ImportedBy, ImportLoc);
 
-  Chain.push_back(std::move(NewModule));
-  if (!ModuleEntry->isModule())
-    PCHChain.push_back(ModuleEntry);
+  if (!NewModule->isModule())
+    PCHChain.push_back(NewModule.get());
   if (!ImportedBy)
-    Roots.push_back(ModuleEntry);
+    Roots.push_back(NewModule.get());
 
+  Chain.push_back(std::move(NewModule));
   return NewlyLoaded;
 }
 




More information about the cfe-commits mailing list