[Tooling/DependencyScanning] Make skipping excluded PP ranges during dependency scanning the default

This is to improve maintenance a bit and remove need to maintain the additional option and related code-paths.

Differential Revision: https://reviews.llvm.org/D124558

Author: akyrtzi

clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h

@@ -293,7 +293,7 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem {
   DependencyScanningWorkerFilesystem(
       DependencyScanningFilesystemSharedCache &SharedCache,
       IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
-      ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings)
+      ExcludedPreprocessorDirectiveSkipMapping &PPSkipMappings)
       : ProxyFileSystem(std::move(FS)), SharedCache(SharedCache),
         PPSkipMappings(PPSkipMappings) {}
 
@@ -398,10 +398,10 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem {
   /// The local cache is used by the worker thread to cache file system queries
   /// locally instead of querying the global cache every time.
   DependencyScanningFilesystemLocalCache LocalCache;
-  /// The optional mapping structure which records information about the
+  /// The mapping structure which records information about the
   /// excluded conditional directive skip mappings that are used by the
   /// currently active preprocessor.
-  ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings;
+  ExcludedPreprocessorDirectiveSkipMapping &PPSkipMappings;
   /// The set of files that should not be minimized.
   llvm::DenseSet<llvm::sys::fs::UniqueID> NotToBeMinimized;
 };

clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h

@@ -48,7 +48,6 @@ class DependencyScanningService {
 public:
   DependencyScanningService(ScanningMode Mode, ScanningOutputFormat Format,
                             bool ReuseFileManager = true,
-                            bool SkipExcludedPPRanges = true,
                             bool OptimizeArgs = false);
 
   ScanningMode getMode() const { return Mode; }
@@ -57,8 +56,6 @@ class DependencyScanningService {
 
   bool canReuseFileManager() const { return ReuseFileManager; }
 
-  bool canSkipExcludedPPRanges() const { return SkipExcludedPPRanges; }
-
   bool canOptimizeArgs() const { return OptimizeArgs; }
 
   DependencyScanningFilesystemSharedCache &getSharedCache() {
@@ -69,10 +66,6 @@ class DependencyScanningService {
   const ScanningMode Mode;
   const ScanningOutputFormat Format;
   const bool ReuseFileManager;
-  /// Set to true to use the preprocessor optimization that skips excluded PP
-  /// ranges by bumping the buffer pointer in the lexer instead of lexing the
-  /// tokens in the range until reaching the corresponding directive.
-  const bool SkipExcludedPPRanges;
   /// Whether to optimize the modules' command-line arguments.
   const bool OptimizeArgs;
   /// The global file system cache.

clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h

@@ -69,7 +69,7 @@ class DependencyScanningWorker {
 
 private:
   std::shared_ptr<PCHContainerOperations> PCHContainerOps;
-  std::unique_ptr<ExcludedPreprocessorDirectiveSkipMapping> PPSkipMappings;
+  ExcludedPreprocessorDirectiveSkipMapping PPSkipMappings;
 
   /// The physical filesystem overlaid by `InMemoryFS`.
   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> RealFS;

clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

@@ -309,7 +309,7 @@ class MinimizedVFSFile final : public llvm::vfs::File {
 
   static llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
   create(EntryRef Entry,
-         ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings);
+         ExcludedPreprocessorDirectiveSkipMapping &PPSkipMappings);
 
   llvm::ErrorOr<llvm::vfs::Status> status() override { return Stat; }
 
@@ -329,7 +329,7 @@ class MinimizedVFSFile final : public llvm::vfs::File {
 } // end anonymous namespace
 
 llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>> MinimizedVFSFile::create(
-    EntryRef Entry, ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings) {
+    EntryRef Entry, ExcludedPreprocessorDirectiveSkipMapping &PPSkipMappings) {
   assert(!Entry.isError() && "error");
 
   if (Entry.isDirectory())
@@ -342,8 +342,8 @@ llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>> MinimizedVFSFile::create(
       Entry.getStatus());
 
   const auto *EntrySkipMappings = Entry.getPPSkippedRangeMapping();
-  if (EntrySkipMappings && !EntrySkipMappings->empty() && PPSkipMappings)
-    (*PPSkipMappings)[Result->Buffer->getBufferStart()] = EntrySkipMappings;
+  if (EntrySkipMappings && !EntrySkipMappings->empty())
+    PPSkipMappings[Result->Buffer->getBufferStart()] = EntrySkipMappings;
 
   return llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>(
       std::unique_ptr<llvm::vfs::File>(std::move(Result)));

clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp

@@ -15,9 +15,9 @@ using namespace dependencies;
 
 DependencyScanningService::DependencyScanningService(
     ScanningMode Mode, ScanningOutputFormat Format, bool ReuseFileManager,
-    bool SkipExcludedPPRanges, bool OptimizeArgs)
+    bool OptimizeArgs)
     : Mode(Mode), Format(Format), ReuseFileManager(ReuseFileManager),
-      SkipExcludedPPRanges(SkipExcludedPPRanges), OptimizeArgs(OptimizeArgs) {
+      OptimizeArgs(OptimizeArgs) {
   // Initialize targets for object file support.
   llvm::InitializeAllTargets();
   llvm::InitializeAllTargetMCs();

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

@@ -137,7 +137,7 @@ class DependencyScanningAction : public tooling::ToolAction {
   DependencyScanningAction(
       StringRef WorkingDirectory, DependencyConsumer &Consumer,
       llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS,
-      ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings,
+      ExcludedPreprocessorDirectiveSkipMapping &PPSkipMappings,
       ScanningOutputFormat Format, bool OptimizeArgs,
       llvm::Optional<StringRef> ModuleName = None)
       : WorkingDirectory(WorkingDirectory), Consumer(Consumer),
@@ -204,9 +204,8 @@ class DependencyScanningAction : public tooling::ToolAction {
 
       // Pass the skip mappings which should speed up excluded conditional block
       // skipping in the preprocessor.
-      if (PPSkipMappings)
-        ScanInstance.getPreprocessorOpts()
-            .ExcludedConditionalDirectiveSkipMappings = PPSkipMappings;
+      ScanInstance.getPreprocessorOpts()
+          .ExcludedConditionalDirectiveSkipMappings = &PPSkipMappings;
     }
 
     // Create the dependency collector that will collect the produced
@@ -263,7 +262,7 @@ class DependencyScanningAction : public tooling::ToolAction {
   StringRef WorkingDirectory;
   DependencyConsumer &Consumer;
   llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
-  ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings;
+  ExcludedPreprocessorDirectiveSkipMapping &PPSkipMappings;
   ScanningOutputFormat Format;
   bool OptimizeArgs;
   llvm::Optional<StringRef> ModuleName;
@@ -288,12 +287,9 @@ DependencyScanningWorker::DependencyScanningWorker(
   OverlayFS->pushOverlay(InMemoryFS);
   RealFS = OverlayFS;
 
-  if (Service.canSkipExcludedPPRanges())
-    PPSkipMappings =
-        std::make_unique<ExcludedPreprocessorDirectiveSkipMapping>();
   if (Service.getMode() == ScanningMode::MinimizedSourcePreprocessing)
-    DepFS = new DependencyScanningWorkerFilesystem(
-        Service.getSharedCache(), RealFS, PPSkipMappings.get());
+    DepFS = new DependencyScanningWorkerFilesystem(Service.getSharedCache(),
+                                                   RealFS, PPSkipMappings);
   if (Service.canReuseFileManager())
     Files = new FileManager(FileSystemOptions(), RealFS);
 }
@@ -344,9 +340,8 @@ llvm::Error DependencyScanningWorker::computeDependencies(
   return runWithDiags(CreateAndPopulateDiagOpts(FinalCCommandLine).release(),
                       [&](DiagnosticConsumer &DC, DiagnosticOptions &DiagOpts) {
                         DependencyScanningAction Action(
-                            WorkingDirectory, Consumer, DepFS,
-                            PPSkipMappings.get(), Format, OptimizeArgs,
-                            ModuleName);
+                            WorkingDirectory, Consumer, DepFS, PPSkipMappings,
+                            Format, OptimizeArgs, ModuleName);
                         // Create an invocation that uses the underlying file
                         // system to ensure that any file system requests that
                         // are made by the driver do not go through the

clang/test/ClangScanDeps/regular_cdb.cpp

@@ -20,11 +20,6 @@
 // RUN: clang-scan-deps -compilation-database %t_clangcl.cdb -j 1 -mode preprocess | \
 // RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO,CHECK3 %s
 
-// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources \
-// RUN:   -skip-excluded-pp-ranges=0 | FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO,CHECK3 %s
-// RUN: clang-scan-deps -compilation-database %t_clangcl.cdb -j 1 -mode preprocess-minimized-sources \
-// RUN:   -skip-excluded-pp-ranges=0 | FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO,CHECK3 %s
-//
 // Make sure we didn't produce any dependency files!
 // RUN: not cat %t.dir/regular_cdb.d
 // RUN: not cat %t.dir/regular_cdb_clangcl.d

clang/tools/clang-scan-deps/ClangScanDeps.cpp

@@ -191,14 +191,6 @@ llvm::cl::opt<bool> ReuseFileManager(
     llvm::cl::desc("Reuse the file manager and its cache between invocations."),
     llvm::cl::init(true), llvm::cl::cat(DependencyScannerCategory));
 
-llvm::cl::opt<bool> SkipExcludedPPRanges(
-    "skip-excluded-pp-ranges",
-    llvm::cl::desc(
-        "Use the preprocessor optimization that skips excluded conditionals by "
-        "bumping the buffer pointer in the lexer instead of lexing the tokens  "
-        "until reaching the end directive."),
-    llvm::cl::init(true), llvm::cl::cat(DependencyScannerCategory));
-
 llvm::cl::opt<std::string> ModuleName(
     "module-name", llvm::cl::Optional,
     llvm::cl::desc("the module of which the dependencies are to be computed"),
@@ -522,7 +514,7 @@ int main(int argc, const char **argv) {
   SharedStream DependencyOS(llvm::outs());
 
   DependencyScanningService Service(ScanMode, Format, ReuseFileManager,
-                                    SkipExcludedPPRanges, OptimizeArgs);
+                                    OptimizeArgs);
   llvm::ThreadPool Pool(llvm::hardware_concurrency(NumThreads));
   std::vector<std::unique_ptr<DependencyScanningTool>> WorkerTools;
   for (unsigned I = 0; I < Pool.getThreadCount(); ++I)

clang/unittests/Tooling/DependencyScannerTest.cpp

@@ -212,8 +212,8 @@ TEST(DependencyScanningFilesystem, IgnoredFilesAreCachedSeparately1) {
                                                 "// hi there!\n"));
 
   DependencyScanningFilesystemSharedCache SharedCache;
-  auto Mappings = std::make_unique<ExcludedPreprocessorDirectiveSkipMapping>();
-  DependencyScanningWorkerFilesystem DepFS(SharedCache, VFS, Mappings.get());
+  ExcludedPreprocessorDirectiveSkipMapping Mappings;
+  DependencyScanningWorkerFilesystem DepFS(SharedCache, VFS, Mappings);
 
   DepFS.enableMinimizationOfAllFiles(); // Let's be explicit for clarity.
   auto StatusMinimized0 = DepFS.status("/mod.h");
@@ -235,8 +235,8 @@ TEST(DependencyScanningFilesystem, IgnoredFilesAreCachedSeparately2) {
                                                 "// hi there!\n"));
 
   DependencyScanningFilesystemSharedCache SharedCache;
-  auto Mappings = std::make_unique<ExcludedPreprocessorDirectiveSkipMapping>();
-  DependencyScanningWorkerFilesystem DepFS(SharedCache, VFS, Mappings.get());
+  ExcludedPreprocessorDirectiveSkipMapping Mappings;
+  DependencyScanningWorkerFilesystem DepFS(SharedCache, VFS, Mappings);
 
   DepFS.disableMinimization("/mod.h");
   auto StatusFull0 = DepFS.status("/mod.h");