Merge pull request #27845 from nkcsgexi/comments-address-previous-patch

SerializeLoc: address Jordan's previous code review comments. NFC

Author: nkcsgexi

include/swift/Frontend/Frontend.h

@@ -60,6 +60,18 @@ namespace Lowering {
 class TypeConverter;
 }
 
+struct ModuleBuffers {
+  std::unique_ptr<llvm::MemoryBuffer> ModuleBuffer;
+  std::unique_ptr<llvm::MemoryBuffer> ModuleDocBuffer;
+  std::unique_ptr<llvm::MemoryBuffer> ModuleSourceInfoBuffer;
+  ModuleBuffers(std::unique_ptr<llvm::MemoryBuffer> ModuleBuffer,
+                std::unique_ptr<llvm::MemoryBuffer> ModuleDocBuffer = nullptr,
+                std::unique_ptr<llvm::MemoryBuffer> ModuleSourceInfoBuffer = nullptr):
+                  ModuleBuffer(std::move(ModuleBuffer)),
+                  ModuleDocBuffer(std::move(ModuleDocBuffer)),
+                  ModuleSourceInfoBuffer(std::move(ModuleSourceInfoBuffer)) {}
+};
+
 /// The abstract configuration of the compiler, including:
 ///   - options for all stages of translation,
 ///   - information about the build environment,
@@ -392,15 +404,9 @@ class CompilerInstance {
   /// Contains buffer IDs for input source code files.
   std::vector<unsigned> InputSourceCodeBufferIDs;
 
-  struct PartialModuleInputs {
-    std::unique_ptr<llvm::MemoryBuffer> ModuleBuffer;
-    std::unique_ptr<llvm::MemoryBuffer> ModuleDocBuffer;
-    std::unique_ptr<llvm::MemoryBuffer> ModuleSourceInfoBuffer;
-  };
-
   /// Contains \c MemoryBuffers for partial serialized module files and
   /// corresponding partial serialized module documentation files.
-  std::vector<PartialModuleInputs> PartialModules;
+  std::vector<ModuleBuffers> PartialModules;
 
   enum : unsigned { NO_SUCH_BUFFER = ~0U };
   unsigned MainBufferID = NO_SUCH_BUFFER;
@@ -556,18 +562,6 @@ class CompilerInstance {
 
   Optional<unsigned> getRecordedBufferID(const InputFile &input, bool &failed);
 
-  struct ModuleBuffers {
-    std::unique_ptr<llvm::MemoryBuffer> ModuleBuffer;
-    std::unique_ptr<llvm::MemoryBuffer> ModuleDocBuffer;
-    std::unique_ptr<llvm::MemoryBuffer> ModuleSourceInfoBuffer;
-    ModuleBuffers(std::unique_ptr<llvm::MemoryBuffer> ModuleBuffer,
-                  std::unique_ptr<llvm::MemoryBuffer> ModuleDocBuffer = nullptr,
-                  std::unique_ptr<llvm::MemoryBuffer> ModuleSourceInfoBuffer = nullptr):
-                    ModuleBuffer(std::move(ModuleBuffer)),
-                    ModuleDocBuffer(std::move(ModuleDocBuffer)),
-                    ModuleSourceInfoBuffer(std::move(ModuleSourceInfoBuffer)) {}
-  };
-
   /// Given an input file, return a buffer to use for its contents,
   /// and a buffer for the corresponding module doc file if one exists.
   /// On failure, return a null pointer for the first element of the returned

include/swift/Serialization/SerializedModuleLoader.h

@@ -97,7 +97,7 @@ class SerializedModuleLoaderBase : public ModuleLoader {
   openModuleSourceInfoFileIfPresent(AccessPathElem ModuleID,
                                     StringRef ModulePath,
                                     StringRef ModuleSourceInfoFileName,
-                  std::unique_ptr<llvm::MemoryBuffer> *ModuleSourceInfoBuffer);
+      std::unique_ptr<llvm::MemoryBuffer> *ModuleSourceInfoBuffer);
 
   /// If the module loader subclass knows that all options have been tried for
   /// loading an architecture-specific file out of a swiftmodule bundle, try

lib/AST/USRGeneration.cpp

@@ -342,8 +342,6 @@ bool ide::printExtensionUSR(const ExtensionDecl *ED, raw_ostream &OS) {
 }
 
 bool ide::printDeclUSR(const Decl *D, raw_ostream &OS) {
-  if (D->isImplicit())
-    return true;
   if (auto *VD = dyn_cast<ValueDecl>(D)) {
     if (ide::printValueDeclUSR(VD, OS)) {
       return true;

lib/Frontend/Frontend.cpp

@@ -502,9 +502,7 @@ Optional<unsigned> CompilerInstance::getRecordedBufferID(const InputFile &input,
   // FIXME: The fact that this test happens twice, for some cases,
   // suggests that setupInputs could use another round of refactoring.
   if (serialization::isSerializedAST(buffers->ModuleBuffer->getBuffer())) {
-    PartialModules.push_back(
-        {std::move(buffers->ModuleBuffer), std::move(buffers->ModuleDocBuffer),
-         std::move(buffers->ModuleSourceInfoBuffer)});
+    PartialModules.push_back(std::move(*buffers));
     return None;
   }
   assert(buffers->ModuleDocBuffer.get() == nullptr);
@@ -516,7 +514,7 @@ Optional<unsigned> CompilerInstance::getRecordedBufferID(const InputFile &input,
   return bufferID;
 }
 
-Optional<CompilerInstance::ModuleBuffers> CompilerInstance::getInputBuffersIfPresent(
+Optional<ModuleBuffers> CompilerInstance::getInputBuffersIfPresent(
     const InputFile &input) {
   if (auto b = input.buffer()) {
     return ModuleBuffers(llvm::MemoryBuffer::getMemBufferCopy(b->getBuffer(),

lib/Serialization/ModuleFile.cpp

@@ -1261,7 +1261,7 @@ class ModuleFile::DeclUSRTableInfo {
 std::unique_ptr<ModuleFile::SerializedDeclUSRTable>
 ModuleFile::readDeclUSRsTable(ArrayRef<uint64_t> fields, StringRef blobData) {
   if (fields.empty() || blobData.empty())
-     return nullptr;
+    return nullptr;
   uint32_t tableOffset = static_cast<uint32_t>(fields.front());
   auto base = reinterpret_cast<const uint8_t *>(blobData.data());
   return std::unique_ptr<SerializedDeclUSRTable>(
@@ -2357,6 +2357,8 @@ Optional<CommentInfo> ModuleFile::getCommentForDecl(const Decl *D) const {
 
   if (!DeclCommentTable)
     return None;
+  if (D->isImplicit())
+    return None;
   // Compute the USR.
   llvm::SmallString<128> USRBuffer;
   llvm::raw_svector_ostream OS(USRBuffer);
@@ -2381,6 +2383,8 @@ ModuleFile::getBasicDeclLocsForDecl(const Decl *D) const {
   // Future compilers may not provide BasicDeclLocsData anymore.
   if (BasicDeclLocsData.empty())
     return None;
+  if (D->isImplicit())
+    return None;
   // Compute the USR.
   llvm::SmallString<128> USRBuffer;
   llvm::raw_svector_ostream OS(USRBuffer);

lib/Serialization/SerializeDoc.cpp

@@ -591,12 +591,12 @@ class StringWriter {
   llvm::SmallString<1024> Buffer;
 public:
   uint32_t getTextOffset(StringRef Text) {
-    if (IndexMap.find(Text) == IndexMap.end()) {
-      IndexMap.insert({Text, Buffer.size()});
+    auto IterAndIsNew = IndexMap.insert({Text, Buffer.size()});
+    if (IterAndIsNew.second) {
       Buffer.append(Text);
       Buffer.push_back('\0');
     }
-    return IndexMap[Text];
+    return (*IterAndIsNew.first).getValue();
   }
 
   void emitSourceFilesRecord(llvm::BitstreamWriter &Out) {

lib/Serialization/SerializedModuleLoader.cpp

@@ -267,10 +267,10 @@ std::error_code SerializedModuleLoaderBase::openModuleDocFile(
 
 void
 SerializedModuleLoaderBase::openModuleSourceInfoFileIfPresent(
-                                                      AccessPathElem ModuleID,
-                                                      StringRef ModulePath,
-                                            StringRef ModuleSourceInfoFilename,
-                              std::unique_ptr<llvm::MemoryBuffer> *ModuleSourceInfoBuffer) {
+    AccessPathElem ModuleID,
+    StringRef ModulePath,
+    StringRef ModuleSourceInfoFilename,
+    std::unique_ptr<llvm::MemoryBuffer> *ModuleSourceInfoBuffer) {
   if (!ModuleSourceInfoBuffer)
     return;
   llvm::vfs::FileSystem &FS = *Ctx.SourceMgr.getFileSystem();