address comments

Author: abhina-sree

clang/include/clang/Basic/FileManager.h

@@ -290,14 +290,19 @@ class FileManager : public RefCountedBase<FileManager> {
 
   /// Open the specified file as a MemoryBuffer, returning a new
   /// MemoryBuffer if successful, otherwise returning null.
+  /// The IsText parameter controls whether the file should be opened as a text
+  /// or binary file, and should be set to false if the file contents should be
+  /// treated as binary.
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
   getBufferForFile(FileEntryRef Entry, bool isVolatile = false,
-                   bool RequiresNullTerminator = true, bool IsText = true,
-                   std::optional<int64_t> MaybeLimit = std::nullopt);
+                   bool RequiresNullTerminator = true,
+                   std::optional<int64_t> MaybeLimit = std::nullopt,
+                   bool IsText = true);
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
   getBufferForFile(StringRef Filename, bool isVolatile = false,
-                   bool RequiresNullTerminator = true, bool IsText = true,
-                   std::optional<int64_t> MaybeLimit = std::nullopt) const {
+                   bool RequiresNullTerminator = true,
+                   std::optional<int64_t> MaybeLimit = std::nullopt,
+                   bool IsText = true) const {
     return getBufferForFileImpl(Filename,
                                 /*FileSize=*/MaybeLimit.value_or(-1),
                                 isVolatile, RequiresNullTerminator, IsText);

clang/lib/Basic/FileManager.cpp

@@ -530,8 +530,8 @@ void FileManager::fillRealPathName(FileEntry *UFE, llvm::StringRef FileName) {
 
 llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
 FileManager::getBufferForFile(FileEntryRef FE, bool isVolatile,
-                              bool RequiresNullTerminator, bool IsText,
-                              std::optional<int64_t> MaybeLimit) {
+                              bool RequiresNullTerminator,
+                              std::optional<int64_t> MaybeLimit, bool IsText) {
   const FileEntry *Entry = &FE.getFileEntry();
   // If the content is living on the file entry, return a reference to it.
   if (Entry->Content)

clang/lib/Lex/HeaderMap.cpp

@@ -56,7 +56,8 @@ std::unique_ptr<HeaderMap> HeaderMap::Create(FileEntryRef FE, FileManager &FM) {
 
   auto FileBuffer =
       FM.getBufferForFile(FE, /*IsVolatile=*/false,
-                          /*RequiresNullTerminator=*/true, /*IsText=*/false);
+                          /*RequiresNullTerminator=*/true,
+                          /*MaybeList=*/std::nullopt, /*IsText=*/false);
   if (!FileBuffer || !*FileBuffer)
     return nullptr;
   bool NeedsByteSwap;

clang/lib/Serialization/ASTReader.cpp

@@ -5334,6 +5334,7 @@ std::string ASTReader::getOriginalSourceFile(
   // Open the AST file.
   auto Buffer = FileMgr.getBufferForFile(ASTFileName, /*IsVolatile=*/false,
                                          /*RequiresNullTerminator=*/false,
+                                         /*MaybeLimit=*/std::nullopt,
                                          /*IsText=*/false);
   if (!Buffer) {
     Diags.Report(diag::err_fe_unable_to_read_pch_file)

llvm/include/llvm/Support/VirtualFileSystem.h

@@ -275,16 +275,20 @@ class FileSystem : public llvm::ThreadSafeRefCountedBase<FileSystem>,
   virtual llvm::ErrorOr<std::unique_ptr<File>>
   openFileForRead(const Twine &Path) = 0;
 
-  /// Get a \p File objct for the binary file at \p Path, if one exists.
-  /// This function should be called instead of openFileForRead if the file
-  /// should be opened as a binary file.
+  /// Get a \p File object for the binary file at \p Path, if one exists.
+  /// Some non-ascii based file systems perform encoding conversions
+  /// when reading as a text file, and this function should be used if
+  /// a file's bytes should be read as-is. On most filesystems, this
+  /// is the same behaviour as openFileForRead.
   virtual llvm::ErrorOr<std::unique_ptr<File>>
   openFileForReadBinary(const Twine &Path) {
     return openFileForRead(Path);
   }
 
   /// This is a convenience method that opens a file, gets its content and then
   /// closes the file.
+  /// The IsText parameter is used to distinguish whether the file should be
+  /// opened as a binary or text file.
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
   getBufferForFile(const Twine &Name, int64_t FileSize = -1,
                    bool RequiresNullTerminator = true, bool IsVolatile = false,

llvm/lib/Support/VirtualFileSystem.cpp

@@ -305,8 +305,8 @@ class RealFileSystem : public FileSystem {
     return Storage;
   }
 
-  ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Name,
-                                                 sys::fs::OpenFlags Flags) {
+  ErrorOr<std::unique_ptr<File>>
+  openFileForReadWithFlags(const Twine &Name, sys::fs::OpenFlags Flags) {
     SmallString<256> RealName, Storage;
     Expected<file_t> FDOrErr = sys::fs::openNativeFileForRead(
         adjustPath(Name, Storage), Flags, &RealName);
@@ -338,12 +338,12 @@ ErrorOr<Status> RealFileSystem::status(const Twine &Path) {
 
 ErrorOr<std::unique_ptr<File>>
 RealFileSystem::openFileForRead(const Twine &Name) {
-  return openFileForRead(Name, sys::fs::OF_Text);
+  return openFileForReadWithFlags(Name, sys::fs::OF_Text);
 }
 
 ErrorOr<std::unique_ptr<File>>
 RealFileSystem::openFileForReadBinary(const Twine &Name) {
-  return openFileForRead(Name, sys::fs::OF_None);
+  return openFileForReadWithFlags(Name, sys::fs::OF_None);
 }
 
 llvm::ErrorOr<std::string> RealFileSystem::getCurrentWorkingDirectory() const {