Address code review comments.

Author: qiongsiwu

clang/include/clang/Frontend/FrontendActions.h

@@ -320,15 +320,6 @@ class PrintPreprocessedAction : public PreprocessorFrontendAction {
   bool hasPCHSupport() const override { return true; }
 };
 
-class GetDependenciesByModuleNameAction : public PreprocessOnlyAction {
-  StringRef ModuleName;
-  void ExecuteAction() override;
-
-public:
-  GetDependenciesByModuleNameAction(StringRef ModuleName)
-      : ModuleName(ModuleName) {}
-};
-
 //===----------------------------------------------------------------------===//
 // HLSL Specific Actions
 //===----------------------------------------------------------------------===//

clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h

@@ -162,7 +162,7 @@ class DependencyScanningTool {
   /// @param CWD The current working directory used during the scan.
   /// @param CommandLine The commandline used for the scan.
   /// @return Error if the initializaiton fails.
-  llvm::Error initializeCompilerInstacneWithContext(
+  llvm::Error initializeCompilerInstanceWithContext(
       StringRef CWD, const std::vector<std::string> &CommandLine);
 
   /// @brief Computes the dependeny for the module named ModuleName.

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

@@ -13,7 +13,6 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Frontend/PCHContainerOperations.h"
-#include "clang/Tooling/DependencyScanning/DependencyScannerImpl.h"
 #include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
 #include "clang/Tooling/DependencyScanning/ModuleDepCollector.h"
 #include "llvm/Support/Error.h"
@@ -30,6 +29,7 @@ namespace tooling {
 namespace dependencies {
 
 class DependencyScanningWorkerFilesystem;
+class CompilerInstanceWithContext;
 
 /// A command-line tool invocation that is part of building a TU.
 ///
@@ -90,6 +90,8 @@ class DependencyScanningWorker {
   DependencyScanningWorker(DependencyScanningService &Service,
                            llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS);
 
+  ~DependencyScanningWorker();
+
   /// Run the dependency scanning tool for a given clang driver command-line,
   /// and report the discovered dependencies to the provided consumer. If
   /// TUBuffer is not nullopt, it is used as TU input for the dependency

clang/lib/Frontend/FrontendActions.cpp

@@ -1233,20 +1233,6 @@ void PrintDependencyDirectivesSourceMinimizerAction::ExecuteAction() {
                                     llvm::outs());
 }
 
-void GetDependenciesByModuleNameAction::ExecuteAction() {
-  CompilerInstance &CI = getCompilerInstance();
-  Preprocessor &PP = CI.getPreprocessor();
-  SourceManager &SM = PP.getSourceManager();
-  FileID MainFileID = SM.getMainFileID();
-  SourceLocation FileStart = SM.getLocForStartOfFile(MainFileID);
-  SmallVector<IdentifierLoc, 2> Path;
-  IdentifierInfo *ModuleID = PP.getIdentifierInfo(ModuleName);
-  Path.emplace_back(FileStart, ModuleID);
-  auto ModResult = CI.loadModule(FileStart, Path, Module::Hidden, false);
-  PPCallbacks *CB = PP.getPPCallbacks();
-  CB->moduleImport(SourceLocation(), Path, ModResult);
-}
-
 //===----------------------------------------------------------------------===//
 // HLSL Specific Actions
 //===----------------------------------------------------------------------===//

clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp

@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/Tooling/DependencyScanning/DependencyScannerImpl.h"
+#include "DependencyScannerImpl.h"
 #include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Basic/DiagnosticSerialization.h"
 #include "clang/Driver/Driver.h"
@@ -490,9 +490,6 @@ bool initializeScanCompilerInstance(
     IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
     DiagnosticConsumer *DiagConsumer, DependencyScanningService &Service,
     IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS) {
-  // TODO: the commented out code here should be un-commented when
-  // we enable CAS.
-  // ScanInstance.getInvocation().getCASOpts() = Worker.CASOpts;
   ScanInstance.setBuildingModule(false);
 
   ScanInstance.createVirtualFileSystem(FS, DiagConsumer);
@@ -728,20 +725,22 @@ llvm::Error CompilerInstanceWithContext::initialize() {
                                                llvm::inconvertibleErrorCode());
   }
 
+  assert(Compilation->getJobs().size() &&
+         "Must have a job list of non-zero size");
   const driver::Command &Command = *(Compilation->getJobs().begin());
   const auto &CommandArgs = Command.getArguments();
   size_t ArgSize = CommandArgs.size();
   assert(ArgSize >= 1 && "Cannot have a command with 0 args");
   const char *FirstArg = CommandArgs[0];
-  if (strcmp(FirstArg, "-cc1"))
+  if (StringRef(FirstArg) != "-cc1")
     return llvm::make_error<llvm::StringError>(
         "Incorrect compilation command, missing cc1",
         llvm::inconvertibleErrorCode());
-  Invocation = std::make_unique<CompilerInvocation>();
+  OriginalInvocation = std::make_unique<CompilerInvocation>();
 
-  if (!CompilerInvocation::CreateFromArgs(*Invocation, Command.getArguments(),
-                                          *DiagEngineWithCmdAndOpts->DiagEngine,
-                                          Command.getExecutable())) {
+  if (!CompilerInvocation::CreateFromArgs(
+          *OriginalInvocation, Command.getArguments(),
+          *DiagEngineWithCmdAndOpts->DiagEngine, Command.getExecutable())) {
     DiagEngineWithCmdAndOpts->DiagEngine->Report(
         diag::err_fe_expected_compiler_job)
         << llvm::join(CommandLine, " ");
@@ -750,36 +749,15 @@ llvm::Error CompilerInstanceWithContext::initialize() {
         llvm::inconvertibleErrorCode());
   }
 
-  // TODO: CMDArgsStrVector is making string copies. We should optimize later
-  // and avoid the copies.
-  std::vector<std::string> CMDArgsStrVector(ArgSize + 1);
-  CMDArgsStrVector.push_back(Command.getExecutable());
-  llvm::transform(CommandArgs, CMDArgsStrVector.begin() + 1,
-                  [](const char *s) { return std::string(s); });
-
-  Invocation = createCompilerInvocation(CMDArgsStrVector,
-                                        *DiagEngineWithCmdAndOpts->DiagEngine);
-  if (!Invocation) {
-    DiagEngineWithCmdAndOpts->DiagEngine->Report(
-        diag::err_fe_expected_compiler_job)
-        << llvm::join(CommandLine, " ");
-    return llvm::make_error<llvm::StringError>(
-        "Cannot create CompilerInvocation from Args",
-        llvm::inconvertibleErrorCode());
-  }
-
-  Invocation->getFrontendOpts().DisableFree = false;
-  Invocation->getCodeGenOpts().DisableFree = false;
-
   if (any(Worker.Service.getOptimizeArgs() & ScanningOptimizations::Macros))
-    canonicalizeDefines(Invocation->getPreprocessorOpts());
+    canonicalizeDefines(OriginalInvocation->getPreprocessorOpts());
 
   // Create the CompilerInstance.
   IntrusiveRefCntPtr<ModuleCache> ModCache =
       makeInProcessModuleCache(Worker.Service.getModuleCacheEntries());
   CIPtr = std::make_unique<CompilerInstance>(
-      std::make_shared<CompilerInvocation>(*Invocation), Worker.PCHContainerOps,
-      ModCache.get());
+      std::make_shared<CompilerInvocation>(*OriginalInvocation),
+      Worker.PCHContainerOps, ModCache.get());
   auto &CI = *CIPtr;
 
   if (!initializeScanCompilerInstance(
@@ -801,7 +779,6 @@ llvm::Error CompilerInstanceWithContext::initialize() {
   OutputOpts = takeDependencyOutputOptionsFrom(CI);
 
   CI.createTarget();
-  // CI.initializeDelayedInputFileFromCAS();
 
   return llvm::Error::success();
 }
@@ -810,21 +787,19 @@ llvm::Error CompilerInstanceWithContext::computeDependencies(
     StringRef ModuleName, DependencyConsumer &Consumer,
     DependencyActionController &Controller) {
   auto &CI = *CIPtr;
-  CompilerInvocation Inv(*Invocation);
+  CompilerInvocation Inv(*OriginalInvocation);
 
   CI.clearDependencyCollectors();
   auto MDC = initializeScanInstanceDependencyCollector(
       CI, std::make_unique<DependencyOutputOptions>(*OutputOpts), CWD, Consumer,
-      Worker.Service, *Invocation, Controller, PrebuiltModuleASTMap,
-      StableDirs);
+      Worker.Service, Inv, Controller, PrebuiltModuleASTMap, StableDirs);
 
   if (!SrcLocOffset) {
     // When SrcLocOffset is zero, we are at the beginning of the fake source
     // file. In this case, we call BeginSourceFile to initialize.
     std::unique_ptr<FrontendAction> Action =
-        std::make_unique<GetDependenciesByModuleNameAction>(ModuleName);
+        std::make_unique<PreprocessOnlyAction>();
     auto InputFile = CI.getFrontendOpts().Inputs.begin();
-
     Action->BeginSourceFile(CI, *InputFile);
   }
 
@@ -872,14 +847,6 @@ llvm::Error CompilerInstanceWithContext::computeDependencies(
   MDC->applyDiscoveredDependencies(Inv);
   Consumer.handleBuildCommand({CommandLine[0], Inv.getCC1CommandLine()});
 
-  // TODO: enable CAS
-  //   std::string ID = Inv.getFileSystemOpts().CASFileSystemRootID;
-  //   if (!ID.empty())
-  //     Consumer.handleCASFileSystemRootID(std::move(ID));
-  //   ID = Inv.getFrontendOpts().CASIncludeTreeID;
-  //   if (!ID.empty())
-  //     Consumer.handleIncludeTreeID(std::move(ID));
-
   // Remove the PPCallbacks since they are going out of scope.
   CI.getPreprocessor().removePPCallbacks();
   return llvm::Error::success();

clang/include/clang/Tooling/DependencyScanning/DependencyScannerImpl.h renamed to clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h

@@ -10,7 +10,6 @@
 #define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNER_H
 
 #include "clang/Driver/Compilation.h"
-#include "clang/Driver/Driver.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
@@ -170,8 +169,7 @@ class CompilerInstanceWithContext {
   // Context - compiler invocation
   std::unique_ptr<clang::driver::Driver> Driver;
   std::unique_ptr<clang::driver::Compilation> Compilation;
-  std::unique_ptr<CompilerInvocation> Invocation;
-  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFSFromCompilerInvocation;
+  std::unique_ptr<CompilerInvocation> OriginalInvocation;
 
   // Context - output options
   std::unique_ptr<DependencyOutputOptions> OutputOpts;

clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

@@ -155,7 +155,7 @@ DependencyScanningTool::getTranslationUnitDependencies(
   return Consumer.takeTranslationUnitDeps();
 }
 
-llvm::Error DependencyScanningTool::initializeCompilerInstacneWithContext(
+llvm::Error DependencyScanningTool::initializeCompilerInstanceWithContext(
     StringRef CWD, const std::vector<std::string> &CommandLine) {
   return Worker.initializeCompierInstanceWithContext(CWD, CommandLine);
 }

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
+#include "DependencyScannerImpl.h"
 #include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/Tool.h"
@@ -42,6 +43,8 @@ DependencyScanningWorker::DependencyScanningWorker(
   }
 }
 
+DependencyScanningWorker::~DependencyScanningWorker() = default;
+
 llvm::Error DependencyScanningWorker::computeDependencies(
     StringRef WorkingDirectory, const std::vector<std::string> &CommandLine,
     DependencyConsumer &Consumer, DependencyActionController &Controller,

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

@@ -1085,7 +1085,7 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
             HadErrors = true;
         }
       } else if (ModuleName) {
-        if (llvm::Error Err = WorkerTool.initializeCompilerInstacneWithContext(
+        if (llvm::Error Err = WorkerTool.initializeCompilerInstanceWithContext(
                 CWD, Input->CommandLine)) {
           handleErrorWithInfoString(
               "Compiler instance with context setup error", std::move(Err),