[clang] 12eafd9 - [clang][deps] NFC: Clean up wording (ignored vs minimized)

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-11-26T12:18:37+01:00
New Revision: 12eafd944e0ffccae93402ddbe2855beb7a939ff

URL: https://github.com/llvm/llvm-project/commit/12eafd944e0ffccae93402ddbe2855beb7a939ff
DIFF: https://github.com/llvm/llvm-project/commit/12eafd944e0ffccae93402ddbe2855beb7a939ff.diff

LOG: [clang][deps] NFC: Clean up wording (ignored vs minimized)

The filesystem used during dependency scanning does two things: it caches file entries and minimizes source file contents. We use the term "ignored file" in a couple of places, but it's not clear what exactly that means. This commit clears up the semantics, explicitly spelling out this relates to minimization.

Added: 
    

Modified: 
    clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
    clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
    clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
    clang/unittests/Tooling/DependencyScannerTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index c52da3305f7c1..b3a869af61078 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -195,11 +195,14 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem {
   llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
   openFileForRead(const Twine &Path) override;
 
-  void clearIgnoredFiles() { IgnoredFiles.clear(); }
-  void ignoreFile(StringRef Filename);
+  /// Disable minimization of the given file.
+  void disableMinimization(StringRef Filename);
+  /// Enable minimization of all files.
+  void enableMinimizationOfAllFiles() { NotToBeMinimized.clear(); }
 
 private:
-  bool shouldIgnoreFile(StringRef Filename);
+  /// Check whether the file should be minimized.
+  bool shouldMinimize(StringRef Filename);
 
   llvm::ErrorOr<const CachedFileSystemEntry *>
   getOrCreateFileSystemEntry(const StringRef Filename);
@@ -214,7 +217,7 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem {
   /// currently active preprocessor.
   ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings;
   /// The set of files that should not be minimized.
-  llvm::StringSet<> IgnoredFiles;
+  llvm::StringSet<> NotToBeMinimized;
 };
 
 } // end namespace dependencies

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 5f3e6480eba48..8973f2658ed69 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -129,7 +129,7 @@ DependencyScanningFilesystemSharedCache::get(StringRef Key, bool Minimized) {
 ///
 /// This is kinda hacky, it would be better if we knew what kind of file Clang
 /// was expecting instead.
-static bool shouldMinimize(StringRef Filename) {
+static bool shouldMinimizeBasedOnExtension(StringRef Filename) {
   StringRef Ext = llvm::sys::path::extension(Filename);
   if (Ext.empty())
     return true; // C++ standard library
@@ -147,26 +147,30 @@ static bool shouldCacheStatFailures(StringRef Filename) {
   StringRef Ext = llvm::sys::path::extension(Filename);
   if (Ext.empty())
     return false; // This may be the module cache directory.
-  return shouldMinimize(Filename); // Only cache stat failures on source files.
+  // Only cache stat failures on source files.
+  return shouldMinimizeBasedOnExtension(Filename);
 }
 
-void DependencyScanningWorkerFilesystem::ignoreFile(StringRef RawFilename) {
+void DependencyScanningWorkerFilesystem::disableMinimization(
+    StringRef RawFilename) {
   llvm::SmallString<256> Filename;
   llvm::sys::path::native(RawFilename, Filename);
-  IgnoredFiles.insert(Filename);
+  NotToBeMinimized.insert(Filename);
 }
 
-bool DependencyScanningWorkerFilesystem::shouldIgnoreFile(
-    StringRef RawFilename) {
+bool DependencyScanningWorkerFilesystem::shouldMinimize(StringRef RawFilename) {
+  if (!shouldMinimizeBasedOnExtension(RawFilename))
+    return false;
+
   llvm::SmallString<256> Filename;
   llvm::sys::path::native(RawFilename, Filename);
-  return IgnoredFiles.contains(Filename);
+  return !NotToBeMinimized.contains(Filename);
 }
 
 llvm::ErrorOr<const CachedFileSystemEntry *>
 DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
     const StringRef Filename) {
-  bool ShouldMinimize = !shouldIgnoreFile(Filename) && shouldMinimize(Filename);
+  bool ShouldMinimize = shouldMinimize(Filename);
 
   if (const auto *Entry = Cache.getCachedEntry(Filename, ShouldMinimize))
     return Entry;

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 7fdc49271791a..70bb6c5caf874 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -193,20 +193,19 @@ class DependencyScanningAction : public tooling::ToolAction {
 
     // Use the dependency scanning optimized file system if requested to do so.
     if (DepFS) {
-      DepFS->clearIgnoredFiles();
-      // Ignore any files that contributed to prebuilt modules. The implicit
-      // build validates the modules by comparing the reported sizes of their
-      // inputs to the current state of the filesystem. Minimization would throw
-      // this mechanism off.
+      DepFS->enableMinimizationOfAllFiles();
+      // Don't minimize any files that contributed to prebuilt modules. The
+      // implicit build validates the modules by comparing the reported sizes of
+      // their inputs to the current state of the filesystem. Minimization would
+      // throw this mechanism off.
       for (const auto &File : PrebuiltModulesInputFiles)
-        DepFS->ignoreFile(File.getKey());
-      // Add any filenames that were explicity passed in the build settings and
-      // that might be opened, as we want to ensure we don't run source
-      // minimization on them.
+        DepFS->disableMinimization(File.getKey());
+      // Don't minimize any files that were explicitly passed in the build
+      // settings and that might be opened.
       for (const auto &E : ScanInstance.getHeaderSearchOpts().UserEntries)
-        DepFS->ignoreFile(E.Path);
+        DepFS->disableMinimization(E.Path);
       for (const auto &F : ScanInstance.getHeaderSearchOpts().VFSOverlayFiles)
-        DepFS->ignoreFile(F);
+        DepFS->disableMinimization(F);
 
       // Support for virtual file system overlays on top of the caching
       // filesystem.

diff  --git a/clang/unittests/Tooling/DependencyScannerTest.cpp b/clang/unittests/Tooling/DependencyScannerTest.cpp
index a45223b0b04e4..0488b5cab04a9 100644
--- a/clang/unittests/Tooling/DependencyScannerTest.cpp
+++ b/clang/unittests/Tooling/DependencyScannerTest.cpp
@@ -214,12 +214,12 @@ TEST(DependencyScanningFilesystem, IgnoredFilesHaveSeparateCache) {
   DependencyScanningWorkerFilesystem DepFS(SharedCache, VFS, Mappings.get());
 
   auto StatusMinimized0 = DepFS.status("/mod.h");
-  DepFS.ignoreFile("/mod.h");
+  DepFS.disableMinimization("/mod.h");
   auto StatusFull1 = DepFS.status("/mod.h");
-  DepFS.clearIgnoredFiles();
+  DepFS.enableMinimizationOfAllFiles();
 
   auto StatusMinimized2 = DepFS.status("/mod.h");
-  DepFS.ignoreFile("/mod.h");
+  DepFS.disableMinimization("/mod.h");
   auto StatusFull3 = DepFS.status("/mod.h");
 
   EXPECT_TRUE(StatusMinimized0);