[clang][deps] NFC: Stop going through ClangTool

[jansvoboda11]

Summary

The dependency scanner currently uses ClangTool to invoke the dependency scanning action.

However, ClangTool seems to be the wrong level of abstraction. It's intended to be run over a collection of compile commands, which we actively avoid via SingleCommandCompilationDatabase. It automatically injects -fsyntax-only and other flags, which we avoid by calling clearArgumentsAdjusters(). It deduces the resource directory based on the current executable path, which we'd like to change to deducing from argv[0].

Internally, ClangTool uses ToolInvocation which seems to be more in line with what the dependency scanner tries to achieve. This patch switches to directly using ToolInvocation instead. NFC.

[dexonsmith]

LGTM. I've independently been wondering for a while whether ClangTool was doing more harm than good for the scanning usecase.

I'm approving as-is, since this is a step forward, but I'm curious if you can clarify what ToolInvocation is providing, and whether that's worth it or if it'd make sense (eventually) to go a step further.

[dexonsmith]

In D108979#2974642, @dexonsmith wrote:

I'm approving as-is, since this is a step forward, but I'm curious if you can clarify what ToolInvocation is providing, and whether that's worth it or if it'd make sense (eventually) to go a step further.

(To be concrete, I feel any libclang API is likely to accept command-lines as something directly representable as ArrayRef<const char *>. Having to repackage that as std::vector<std::string> seems a bit unfortunate since the CompilerInvocation will want it repackaged again as ArrayRef<const char *>. But maybe ToolInvocation is doing something valuable, and certainly we could change that later.)

[jansvoboda11]

I think ToolInvocation is somewhat well-suited for the dependency scanner. It takes care of the DiagnosticOptions setup, running the driver, extracting source-reading cc1 command-line, creating CompilerInvocation and invoking the action itself. This is what we'd need to do anyways.

I agree that the std::vector<std::string> interface is unfortunate, especially since ToolInvocation::run converts that into std::vector<const char *> anyways. I think we can move that conversion into the current constructor that accepts std::vector<std::string>, add another constructor that takes ArrayRef<const char *> and use that from the dependency scanner.

[dexonsmith]

Thanks for explaining why ToolInvocation fits well here. SGTM.

In D108979#2979484, @jansvoboda11 wrote:

I think we can move that conversion into the current constructor that accepts std::vector<std::string>, add another constructor that takes ArrayRef<const char *> and use that from the dependency scanner.

Yeah, that makes sense. (No reason to block this though... can be done later.)

[jansvoboda11]

Resolve test failing on Windows by re-setting the VFS on FileManager.

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

static llvm::Error runWithDiags(
   return llvm::make_error<llvm::StringError>(DiagnosticsOS.str(),
                                              llvm::inconvertibleErrorCode());
 }
 
 llvm::Error DependencyScanningWorker::computeDependencies(
     const std::string &Input, StringRef WorkingDirectory,
     const CompilationDatabase &CDB, DependencyConsumer &Consumer,
     llvm::Optional<StringRef> ModuleName) {
+  // Reset what might have been modified in the previous worker invocation.
   RealFS->setCurrentWorkingDirectory(WorkingDirectory);
-  return runWithDiags(DiagOpts.get(), [&](DiagnosticConsumer &DC) {
-    /// Create the tool that uses the underlying file system to ensure that any
-    /// file system requests that are made by the driver do not go through the
-    /// dependency scanning filesystem.
-    tooling::ClangTool Tool(CDB,
-                            ModuleName.hasValue() ? ModuleName->str() : Input,
-                            PCHContainerOps, RealFS, Files);
-    Tool.clearArgumentsAdjusters();
-    Tool.setRestoreWorkingDir(false);
-    Tool.setPrintErrorMessage(false);
-    Tool.setDiagnosticConsumer(&DC);
-    DependencyScanningAction Action(WorkingDirectory, Consumer, DepFS,
-                                    PPSkipMappings.get(), Format, ModuleName);
+  if (Files)
+    Files->setVirtualFileSystem(RealFS);
+
+  llvm::IntrusiveRefCntPtr<FileManager> CurrentFiles =
+      Files ? Files : new FileManager(FileSystemOptions(), RealFS);
+
+  // FIXME: Avoid this copy.
+  std::vector<CompileCommand> CompileCommands = CDB.getCompileCommands(Input);
+  const std::vector<std::string> &CommandLine =
+      CompileCommands.front().CommandLine;
 
-    if (ModuleName.hasValue()) {
-      InMemoryFS->addFile(*ModuleName, 0, llvm::MemoryBuffer::getMemBuffer(""));
-      Tool.appendArgumentsAdjuster(
-          [&](const tooling::CommandLineArguments &Args, StringRef FileName) {
-            tooling::CommandLineArguments AdjustedArgs(Args);
-            AdjustedArgs.emplace_back(*ModuleName);
-            return AdjustedArgs;
-          });
-    }
+  Optional<std::vector<std::string>> ModifiedCommandLine;
+    ModifiedCommandLine = CommandLine;
+    ModifiedCommandLine->emplace_back(*ModuleName);
 
-    return !Tool.run(&Action);
+  const std::vector<std::string> &FinalCommandLine =
+      ModifiedCommandLine ? *ModifiedCommandLine : CommandLine;
+
+  return runWithDiags(DiagOpts.get(), [&](DiagnosticConsumer &DC) {
+    DependencyScanningAction Action(WorkingDirectory, Consumer, DepFS,
+                                    PPSkipMappings.get(), Format, 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 dependency scanning filesystem.
+    ToolInvocation Invocation(FinalCommandLine, &Action, CurrentFiles.get(),
+                              PCHContainerOps);
+    Invocation.setDiagnosticConsumer(&DC);
+    return Invocation.run();
   });
 }