Mark the file entry invalid, until reread. Invalidate SLocEntry cache, readd it on reread. Do not use translateFile, because it pulls in parts of the pch.

[tapaswenipathak]

Summary

Mark the file entry invalid, until reread.

This patch marks the file entry invalid, until reread. Invalidates SLocEntry
cache, readd it on reread. Do not use translateFile, because it pulls
in parts of the pch.

This adds an invalidateCache success test in filemanager and
sourcemanager, using #define public private to access private member in
the test.

Some more related information is available here:
https://github.com/vgvassilev/clang/pull/2.

Co-authored-by: Vassil Vassilev <[e-mail]>

[v.g.vassilev]

Just to add that the invalidateCache is important for cling and clang-repl where we do something like:

clang-repl> #include "file_with_error.h"
// error is printed, we edit the file and include it again:
clang-repl> #include "file_with_error.h"

I am not sure if we can write such a test for clang-repl easily using the lit infrastructure...

[tapaswenipathak]

fix for build failure:

cmdline: git reset --hard
stderr: 'fatal: Unable to create '/var/lib/buildkite-agent/builds/llvm-project-fork/.git/index.lock': File exists.

https://buildkite.com/llvm-project/diff-checks/builds/110904#01818451-4c05-49bb-9f93-ea6d0480f9ec

could be temporary, or could be because i didn't run a git pull for a few days.

[tapaswenipathak]

omit an unrelated change.

[tapaswenipathak]

git clang-format HEAD~1

[v.g.vassilev]

After landing https://reviews.llvm.org/D126682 we might be able to write a test in clang-repl such as:

cpp
clang-repl> #include <iostream>
clang-repl> #include <fstream>
clang-repl>using namespace std;
clang-repl> int write_str (const char* str) {
  ofstream myfile;
  myfile.open ("a.h");
  myfile << str<<"\n";
  myfile.close();
  return 0;
}
clang-repl> auto r1 = write_str("int i = 42");
clang-repl> extern "C" int printf(const char*,...);
clang-repl> #include "a.h"
clang-repl> auto r2 = printf("i=%d\n", i);
clang-repl>%undo
clang-repl>%undo 
clang-repl> auto r3 = write_str("int i = 0");
clang-repl> #include "a.h"
clang-repl> auto r2 = printf("i=%d\n", i);
// Here we should print `i=0`.

[vsapsai]

Hmm, I'm concerned with the pieces added purely for testing. Specifically, FileEntriesToReread and TestHelper friend functions. Need to think how else we can test it.

Do you intend to support only the error cases like

clang-repl> #include "file_with_error.h"
// error is printed, we edit the file and include it again:
clang-repl> #include "file_with_error.h"

or do you want to handle re-including files? Something like

clang-repl> #include "experiments.h"
// edit the file and include it again:
clang-repl> #include "experiments.h"

Asking because maybe in the error case we commit to some state too eagerly and fixing that sticky eagerness is another option (just guessing, have no idea if it is an actual option and if it is "better").

[v.g.vassilev]

In D126266#3629145, @vsapsai wrote:

Hmm, I'm concerned with the pieces added purely for testing. Specifically, FileEntriesToReread and TestHelper friend functions. Need to think how else we can test it.

I am not thrilled about that either. We have discussed this approach here https://reviews.llvm.org/D126183 and we have past experience going that route. I would be happy to drop this test in favor of the mentioned clang-repl test. Should be enough to cover the usecase.

Do you intend to support only the error cases like

clang-repl> #include "file_with_error.h"
// error is printed, we edit the file and include it again:
clang-repl> #include "file_with_error.h"

or do you want to handle re-including files? Something like

clang-repl> #include "experiments.h"
// edit the file and include it again:
clang-repl> #include "experiments.h"

Asking because maybe in the error case we commit to some state too eagerly and fixing that sticky eagerness is another option (just guessing, have no idea if it is an actual option and if it is "better").

We want both. The error case is somewhat easier to deal with. However, in case we have error in the logic in #include "experiments.h" we want to execute and then re-execute the new code.

[v.g.vassilev]

@tapaswenipathak, ping.

[v.g.vassilev]

@tapaswenipathak, ping...

clang/include/clang/Basic/FileEntry.h

   llvm::sys::fs::UniqueID UniqueID;
   unsigned UID = 0; // A unique (small) ID for the file.
   bool IsNamedPipe = false;
+  bool IsValid = false;
 
   /// The open file, if it is owned by the \p FileEntry.
   mutable std::unique_ptr<llvm::vfs::File> File;

   FileEntryRef getLastRef() const { return *LastRef; }
 
   StringRef tryGetRealPathName() const { return RealPathName; }
+  bool isValid() const { return IsValid; }
   off_t getSize() const { return Size; }
   unsigned getUID() const { return UID; }
   const llvm::sys::fs::UniqueID &getUniqueID() const { return UniqueID; }

clang/include/clang/Basic/FileManager.h

 #include "clang/Basic/FileEntry.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/PointerUnion.h"

 #include <ctime>
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
 
 namespace llvm {

   llvm::StringMap<llvm::ErrorOr<FileEntryRef::MapValue>, llvm::BumpPtrAllocator>
       SeenFileEntries;
 
+  std::set<const FileEntry *> FileEntriesToReread;
+
+  friend void FileManagerTestHelper(FileManager &);
+  friend void SourceManagerTestHelper(SourceManager &, FileManager &);
+
   /// A mirror of SeenFileEntries to give fake answers for getBypassFile().
   ///
   /// Don't bother hooking up a BumpPtrAllocator. This should be rarely used,

   std::error_code getNoncachedStatValue(StringRef Path,
                                         llvm::vfs::Status &Result);
 
+  /// Remove the real file \p Entry from the cache.
+  void invalidateCache(FileEntry *Entry);
+
   /// If path is not absolute and FileSystemOptions set the working
   /// directory, the path is modified to be relative to the given
   /// working directory.

clang/include/clang/Basic/SourceManager.h

   // If BufStr has an invalid BOM, returns the BOM name; otherwise, returns
   // nullptr
   static const char *getInvalidBOM(StringRef BufStr);
+  friend void SourceManagerTestHelper(SourceManager &, FileManager &);
 };
 
 // Assert that the \c ContentCache objects will always be 8-byte aligned so

   ~SourceManager();
 
   void clearIDTables();
+  friend void SourceManagerTestHelper(SourceManager &, FileManager &);
+  void invalidateCache(FileID FID);
 
   /// Initialize this source manager suitably to replay the compilation
   /// described by \p Old. Requires that \p Old outlive \p *this.

clang/lib/Basic/FileManager.cpp

   return std::error_code();
 }
 
+void FileManager::invalidateCache(FileEntry *Entry) {
+  assert(Entry && "Cannot invalidate a NULL FileEntry");
+  FileEntriesToReread.insert(Entry->getLastRef());
+  Entry->IsValid = false;
+}
+
 void FileManager::GetUniqueIDMapping(
     SmallVectorImpl<const FileEntry *> &UIDToFiles) const {
   UIDToFiles.clear();

clang/lib/Basic/SourceManager.cpp

   return false;
 }
 
+void SourceManager::invalidateCache(FileID FID) {
+  const FileEntry *Entry = getFileEntryForID(FID);
+  if (!Entry)
+    return;
+  if (ContentCache *&E = FileInfos[Entry]) {
+    E->setBuffer(nullptr);
+    E = 0;
+  }
+  if (!FID.isInvalid()) {
+    const SrcMgr::SLocEntry &SLocE = getSLocEntry(FID);
+    if (SLocE.isFile()) {
+      SrcMgr::ContentCache &CC =
+          const_cast<SrcMgr::ContentCache &>(SLocE.getFile().getContentCache());
+      CC.setBuffer(nullptr);
+    }
+  }
+  getFileManager().invalidateCache(const_cast<FileEntry *>(Entry));
+}
+
 void SourceManager::initializeForReplay(const SourceManager &Old) {
   assert(MainFileID.isInvalid() && "expected uninitialized SourceManager");
 

clang/unittests/Basic/FileManagerTest.cpp

 using namespace llvm;
 using namespace clang;
 
+void clang::FileManagerTestHelper(FileManager &manager) {
+  ASSERT_TRUE(!manager.FileEntriesToReread.empty());
+}
+
 namespace {
 
 // Used to create a fake file system for running the tests with such

   FileManager manager;
 };
 
+// If file entry valid, mark the file entry invalid, until reread.
+TEST_F(FileManagerTest, invalidateCacheSuccess) {
+  auto statCache = std::make_unique<FakeStatCache>();
+  statCache->InjectFile("file.cpp", 42);
+
+  manager.setStatCache(std::move(statCache));
+  manager.getVirtualFile("file.cpp", 100, 0);
+  auto file = manager.getFile("file.cpp");
+
+  // Check for file null assertion success
+  // manager.invalidateCache(NULL);
+
+  auto FileRef = manager.getFileRef("file.cpp");
+
+  ASSERT_FALSE(!FileRef);
+
+  FileEntry *FileEntryObj = const_cast<FileEntry *>(&FileRef->getFileEntry());
+  manager.invalidateCache(FileEntryObj);
+  FileManagerTestHelper(manager);
+
+  ASSERT_FALSE(FileEntryObj->isValid());
+}
+
 // When a virtual file is added, its getDir() field has correct name.
 TEST_F(FileManagerTest, getVirtualFileSetsTheDirFieldCorrectly) {
   FileEntryRef file = manager.getVirtualFileRef("foo.cpp", 42, 0);

clang/unittests/Basic/SourceManagerTest.cpp

 
 using namespace clang;
 
+void clang::SourceManagerTestHelper(SourceManager &SourceMgr,
+                                    FileManager &FileMgr) {
+  FileID mainFileID = SourceMgr.getMainFileID();
+  const SrcMgr::ContentCache *Cache =
+      &(SourceMgr.getSLocEntry(mainFileID).getFile().getContentCache());
+  // EXPECT_FALSE(Cache->Buffer);
+  ASSERT_TRUE(FileMgr.FileEntriesToReread.empty());
+}
+
 namespace {
 
 // The test fixture.

   IntrusiveRefCntPtr<TargetInfo> Target;
 };
 
+// Test for invalidate cache success, making the file entry invalid, until
+// reread
+TEST_F(SourceManagerTest, invalidateCacheSuccess) {
+  const char *Source = "int x;";
+
+  std::unique_ptr<llvm::MemoryBuffer> Buf =
+      llvm::MemoryBuffer::getMemBuffer(Source);
+  const FileEntry *SourceFile =
+      FileMgr.getVirtualFile("mainFile.cpp", Buf->getBufferSize(), 0);
+
+  FileID mainFileID = SourceMgr.createFileID(std::move(Buf));
+  SourceMgr.overrideFileContents(SourceFile, std::move(Buf));
+  SourceMgr.setMainFileID(mainFileID);
+
+  SourceMgr.invalidateCache(mainFileID);
+
+  EXPECT_FALSE(SourceFile->isValid());
+  EXPECT_FALSE(mainFileID.isInvalid());
+
+  const SrcMgr::ContentCache *Cache =
+      &(SourceMgr.getSLocEntry(mainFileID).getFile().getContentCache());
+
+  EXPECT_EQ(SourceMgr.getNonBuiltinFilenameForID(mainFileID), None);
+  // EXPECT_EQ(SourceMgr.getBufferDataIfLoaded(mainFileID), None);
+
+  EXPECT_FALSE(Cache->IsBufferInvalid);
+
+  SourceManagerTestHelper(SourceMgr, FileMgr);
+}
+
 TEST_F(SourceManagerTest, isBeforeInTranslationUnit) {
   const char *source =
     "#define M(x) [x]\n"