[PATCH] D146328: [clang][deps] Only cache files with specific extension

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

This doesn't appear to behave as expected for `PP_CacheFailure` or `PP_ScanFile` without `PP_CacheSuccess`. Looks like that will still cache. Should probably just assert that's not the computed policy.

Looks good with those changes.



================
Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:161
 
-/// Whitelist file extensions that should be minimized, treating no extension as
-/// a source file that should be minimized.
+/// Whitelist file extensions that should be cached/scanned.
 ///
----------------
ributzka wrote:
> s/Whitelist/Allowlist
I think we can just change this comment to something like:

> Determine caching and scanning behavior based on file extension.


================
Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:168
   if (Ext.empty())
-    return true; // C++ standard library
-  return llvm::StringSwitch<bool>(Ext)
-      .CasesLower(".c", ".cc", ".cpp", ".c++", ".cxx", true)
-      .CasesLower(".h", ".hh", ".hpp", ".h++", ".hxx", true)
-      .CasesLower(".m", ".mm", true)
-      .CasesLower(".i", ".ii", ".mi", ".mmi", true)
-      .CasesLower(".def", ".inc", true)
-      .Default(false);
-}
-
-static bool shouldCacheStatFailures(StringRef Filename) {
-  StringRef Ext = llvm::sys::path::extension(Filename);
-  if (Ext.empty())
-    return false; // This may be the module cache directory.
-  // Only cache stat failures on files that are not expected to change during
-  // the build.
-  StringRef FName = llvm::sys::path::filename(Filename);
-  if (FName == "module.modulemap" || FName == "module.map")
-    return true;
-  return shouldScanForDirectivesBasedOnExtension(Filename);
-}
-
-bool DependencyScanningWorkerFilesystem::shouldScanForDirectives(
-    StringRef Filename) {
-  return shouldScanForDirectivesBasedOnExtension(Filename);
+    return (PathPolicy)(PP_CacheSuccess | PP_ScanFile);
+  return (PathPolicy)llvm::StringSwitch<int>(Ext)
----------------
If you use `LLVM_MARK_AS_BITMASK_ENUM` I think you can remove these casts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146328