[PATCH] D115043: [clang][deps] Use MemoryBuffer in minimizing FS

[Jan Svoboda via Phabricator via llvm-commits]

jansvoboda11 updated this revision to Diff 392745.
jansvoboda11 marked an inline comment as done.
jansvoboda11 added a comment.

Update comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115043/new/

https://reviews.llvm.org/D115043

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -9,6 +9,7 @@
 #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
 #include "clang/Lex/DependencyDirectivesSourceMinimizer.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/SmallVectorMemoryBuffer.h"
 #include "llvm/Support/Threading.h"
 
 using namespace clang;
@@ -43,11 +44,7 @@
     // FIXME: Propage the diagnostic if desired by the client.
     CachedFileSystemEntry Result;
     Result.MaybeStat = std::move(*Stat);
-    Result.Contents.reserve(Buffer->getBufferSize() + 1);
-    Result.Contents.append(Buffer->getBufferStart(), Buffer->getBufferEnd());
-    // Implicitly null terminate the contents for Clang's lexer.
-    Result.Contents.push_back('\0');
-    Result.Contents.pop_back();
+    Result.Contents = std::move(*MaybeBuffer);
     return Result;
   }
 
@@ -60,15 +57,8 @@
   // The contents produced by the minimizer must be null terminated.
   assert(MinimizedFileContents.data()[MinimizedFileContents.size()] == '\0' &&
          "not null terminated contents");
-  // Even though there's an implicit null terminator in the minimized contents,
-  // we want to temporarily make it explicit. This will ensure that the
-  // std::move will preserve it even if it needs to do a copy if the
-  // SmallString still has the small capacity.
-  MinimizedFileContents.push_back('\0');
-  Result.Contents = std::move(MinimizedFileContents);
-  // Now make the null terminator implicit again, so that Clang's lexer can find
-  // it right where the buffer ends.
-  Result.Contents.pop_back();
+  Result.Contents = std::make_unique<llvm::SmallVectorMemoryBuffer>(
+      std::move(MinimizedFileContents));
 
   // Compute the skipped PP ranges that speedup skipping over inactive
   // preprocessor blocks.
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===================================================================
--- clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -43,9 +43,7 @@
   ///
   /// The filesystem opens the file even for `stat` calls open to avoid the
   /// issues with stat + open of minimized files that might lead to a
-  /// mismatching size of the file. If file is not minimized, the full file is
-  /// read and copied into memory to ensure that it's not memory mapped to avoid
-  /// running out of file descriptors.
+  /// mismatching size of the file.
   static CachedFileSystemEntry createFileEntry(StringRef Filename,
                                                llvm::vfs::FileSystem &FS,
                                                bool Minimize = true);
@@ -65,7 +63,7 @@
       return MaybeStat.getError();
     assert(!MaybeStat->isDirectory() && "not a file");
     assert(isValid() && "not initialized");
-    return Contents.str();
+    return Contents->getBuffer();
   }
 
   /// \returns The error or the status of the entry.
@@ -94,11 +92,7 @@
 
 private:
   llvm::ErrorOr<llvm::vfs::Status> MaybeStat;
-  // Store the contents in a small string to allow a
-  // move from the small string for the minimized contents.
-  // Note: small size of 1 allows us to store an empty string with an implicit
-  // null terminator without any allocations.
-  llvm::SmallString<1> Contents;
+  std::unique_ptr<llvm::MemoryBuffer> Contents;
   PreprocessorSkippedRangeMapping PPSkippedRangeMapping;
 };
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115043.392745.patch
Type: text/x-patch
Size: 3743 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211208/75212690/attachment.bin>