Adding a set of APIs that returns a flag instead of llvm::Erros to DepedencyScanningWorker. Fix two minor issues.

Author: qiongsiwu

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

@@ -125,7 +125,7 @@ class DependencyScanningWorker {
   /// @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 initializeCompilerInstanceWithContext(
+  llvm::Error initializeCompilerInstanceWithContextOrError(
       StringRef CWD, const std::vector<std::string> &CommandLine);
 
   /// @brief Performaces dependency scanning for the module whose name is
@@ -135,14 +135,27 @@ class DependencyScanningWorker {
   /// @param Consumer The dependency consumer that stores the results.
   /// @param Controller The controller for the dependency scanning action.
   /// @return Error if the scanner incurs errors.
-  llvm::Error
-  computeDependenciesByNameWithContext(StringRef ModuleName,
-                                       DependencyConsumer &Consumer,
-                                       DependencyActionController &Controller);
+  llvm::Error computeDependenciesByNameWithContextOrError(
+      StringRef ModuleName, DependencyConsumer &Consumer,
+      DependencyActionController &Controller);
 
   /// @brief Finalizes the diagnostics engine and deletes the compiler instance.
   /// @return Error if errors occur during finalization.
-  llvm::Error finalizeCompilerInstanceWithContext();
+  llvm::Error finalizeCompilerInstanceWithContextOrError();
+
+  /// The three methods below provides the same functionality as the
+  /// three methods above. Instead of returning `llvm::Error`s, these
+  /// three methods return a flag to indicate if the call is successful.
+  /// The initialization function asks the client for a DiagnosticsConsumer
+  /// that it direct the diagnostics to.
+  bool initializeCompilerInstanceWithContext(
+      StringRef CWD, const std::vector<std::string> &CommandLine,
+      DiagnosticConsumer *DC = nullptr);
+  bool
+  computeDependenciesByNameWithContext(StringRef ModuleName,
+                                       DependencyConsumer &Consumer,
+                                       DependencyActionController &Controller);
+  bool finalizeCompilerInstance();
 
   llvm::vfs::FileSystem &getVFS() const { return *BaseFS; }
 

clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h

@@ -342,7 +342,8 @@ class ModuleDepCollector final : public DependencyCollector {
   std::vector<P1689ModuleInfo> RequiredStdCXXModules;
 
   /// A pointer to the preprocessor callback so we can invoke it directly
-  /// if needed.
+  /// if needed. The callback is created and added to a Preprocessor instance by
+  /// attachToPreprocessor and the Preprocessor instance owns it.
   ModuleDepCollectorPP *CollectorPPPtr = nullptr;
 
   /// Checks whether the module is known as being prebuilt.

clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp

@@ -705,22 +705,26 @@ bool DependencyScanningAction::runInvocation(
   return Result;
 }
 
-llvm::Error CompilerInstanceWithContext::initialize() {
-  std::tie(OverlayFS, CommandLine) = initVFSForByNameScanning(
-      Worker.BaseFS, CommandLine, CWD, "ScanningByName");
+bool CompilerInstanceWithContext::initialize(DiagnosticConsumer *DC) {
+  if (DC) {
+    DiagConsumer = DC;
+  } else {
+    DiagPrinterWithOS =
+        std::make_unique<TextDiagnosticsPrinterWithOutput>(CommandLine);
+    DiagConsumer = &DiagPrinterWithOS->DiagPrinter;
+  }
 
-  DiagPrinterWithOS =
-      std::make_unique<TextDiagnosticsPrinterWithOutput>(CommandLine);
   DiagEngineWithCmdAndOpts = std::make_unique<DignosticsEngineWithDiagOpts>(
-      CommandLine, OverlayFS, DiagPrinterWithOS->DiagPrinter);
+      CommandLine, OverlayFS, *DiagConsumer);
+
+  std::tie(OverlayFS, CommandLine) = initVFSForByNameScanning(
+      Worker.BaseFS, CommandLine, CWD, "ScanningByName");
 
   std::tie(Driver, Compilation) = buildCompilation(
       CommandLine, *DiagEngineWithCmdAndOpts->DiagEngine, OverlayFS, Alloc);
 
-  if (!Compilation) {
-    return llvm::make_error<llvm::StringError>("Failed to build compilation",
-                                               llvm::inconvertibleErrorCode());
-  }
+  if (!Compilation)
+    return false;
 
   assert(Compilation->getJobs().size() &&
          "Must have a job list of non-zero size");
@@ -729,10 +733,7 @@ llvm::Error CompilerInstanceWithContext::initialize() {
   size_t ArgSize = CommandArgs.size();
   assert(ArgSize >= 1 && "Cannot have a command with 0 args");
   const char *FirstArg = CommandArgs[0];
-  if (StringRef(FirstArg) != "-cc1")
-    return llvm::make_error<llvm::StringError>(
-        "Incorrect compilation command, missing cc1",
-        llvm::inconvertibleErrorCode());
+  assert(StringRef(FirstArg) == "-cc1" && "Requires a cc1 job.");
   OriginalInvocation = std::make_unique<CompilerInvocation>();
 
   if (!CompilerInvocation::CreateFromArgs(
@@ -741,9 +742,7 @@ llvm::Error CompilerInstanceWithContext::initialize() {
     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());
+    return false;
   }
 
   if (any(Worker.Service.getOptimizeArgs() & ScanningOptimizations::Macros))
@@ -759,18 +758,14 @@ llvm::Error CompilerInstanceWithContext::initialize() {
 
   if (!initializeScanCompilerInstance(
           CI, OverlayFS, DiagEngineWithCmdAndOpts->DiagEngine->getClient(),
-          Worker.Service, Worker.DepFS)) {
-    return llvm::make_error<llvm::StringError>(
-        "Cannot initialize scanning compiler instance",
-        llvm::inconvertibleErrorCode());
-  }
+          Worker.Service, Worker.DepFS))
+    return false;
 
-  llvm::SmallVector<StringRef> StableDirs = getInitialStableDirs(CI);
+  StableDirs = getInitialStableDirs(CI);
   auto MaybePrebuiltModulesASTMap =
       computePrebuiltModulesASTMap(CI, StableDirs);
   if (!MaybePrebuiltModulesASTMap)
-    return llvm::make_error<llvm::StringError>(
-        "Prebuilt module scanning failed", llvm::inconvertibleErrorCode());
+    return false;
 
   PrebuiltModuleASTMap = std::move(*MaybePrebuiltModulesASTMap);
   OutputOpts = takeAndUpdateDependencyOutputOptionsFrom(CI);
@@ -782,12 +777,13 @@ llvm::Error CompilerInstanceWithContext::initialize() {
   // CompilerInstance::ExecuteAction to perform scanning.
   CI.createTarget();
 
-  return llvm::Error::success();
+  return true;
 }
 
-llvm::Error CompilerInstanceWithContext::computeDependencies(
+bool CompilerInstanceWithContext::computeDependencies(
     StringRef ModuleName, DependencyConsumer &Consumer,
     DependencyActionController &Controller) {
+  assert(CIPtr && "CIPtr must be initialized before calling this method");
   auto &CI = *CIPtr;
 
   // We create this cleanup object because computeDependencies may exit
@@ -800,7 +796,6 @@ llvm::Error CompilerInstanceWithContext::computeDependencies(
     CI.getPreprocessor().removePPCallbacks();
   });
 
-  CI.clearDependencyCollectors();
   auto MDC = initializeScanInstanceDependencyCollector(
       CI, std::make_unique<DependencyOutputOptions>(*OutputOpts), CWD, Consumer,
       Worker.Service,
@@ -861,18 +856,25 @@ llvm::Error CompilerInstanceWithContext::computeDependencies(
   CB->EndOfMainFile();
 
   if (!ModResult)
-    return llvm::make_error<llvm::StringError>(
-        DiagPrinterWithOS->DiagnosticsOS.str(), llvm::inconvertibleErrorCode());
+    return false;
 
   CompilerInvocation ModuleInvocation(*OriginalInvocation);
   MDC->applyDiscoveredDependencies(ModuleInvocation);
   Consumer.handleBuildCommand(
       {CommandLine[0], ModuleInvocation.getCC1CommandLine()});
 
-  return llvm::Error::success();
+  return true;
+}
+
+bool CompilerInstanceWithContext::finalize() {
+  DiagConsumer->finish();
+  return true;
 }
 
-llvm::Error CompilerInstanceWithContext::finalize() {
-  DiagPrinterWithOS->DiagPrinter.finish();
-  return llvm::Error::success();
+llvm::Error CompilerInstanceWithContext::handleReturnStatus(bool Success) {
+  assert(DiagPrinterWithOS && "Must use the default DiagnosticConsumer.");
+  return Success ? llvm::Error::success()
+                 : llvm::make_error<llvm::StringError>(
+                       DiagPrinterWithOS->DiagnosticsOS.str(),
+                       llvm::inconvertibleErrorCode());
 }

clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h

@@ -163,6 +163,9 @@ class CompilerInstanceWithContext {
 
   // Context - Diagnostics engine.
   std::unique_ptr<TextDiagnosticsPrinterWithOutput> DiagPrinterWithOS;
+  // DiagConsumer may points to DiagPrinterWithOS->DiagPrinter, or a custom
+  // DiagnosticConsumer passed in from initialize.
+  DiagnosticConsumer *DiagConsumer = nullptr;
   std::unique_ptr<DignosticsEngineWithDiagOpts> DiagEngineWithCmdAndOpts;
 
   // Context - compiler invocation
@@ -191,11 +194,16 @@ class CompilerInstanceWithContext {
                               const std::vector<std::string> &CMD)
       : Worker(Worker), CWD(CWD), CommandLine(CMD) {};
 
-  llvm::Error initialize();
-  llvm::Error computeDependencies(StringRef ModuleName,
-                                  DependencyConsumer &Consumer,
-                                  DependencyActionController &Controller);
-  llvm::Error finalize();
+  // The three methods below returns false when they fail, with the detail
+  // accumulated in DiagConsumer.
+  bool initialize(DiagnosticConsumer *DC);
+  bool computeDependencies(StringRef ModuleName, DependencyConsumer &Consumer,
+                           DependencyActionController &Controller);
+  bool finalize();
+
+  // The method below turns the return status from the above methods
+  // into an llvm::Error using a default DiagnosticConsumer.
+  llvm::Error handleReturnStatus(bool Success);
 };
 } // namespace dependencies
 } // namespace tooling

clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

@@ -163,13 +163,13 @@ DependencyScanningTool::getModuleDependencies(
   FullDependencyConsumer Consumer(AlreadySeen);
   CallbackActionController Controller(LookupModuleOutput);
   if (auto Error =
-          Worker.initializeCompilerInstanceWithContext(CWD, CommandLine))
+          Worker.initializeCompilerInstanceWithContextOrError(CWD, CommandLine))
     return std::move(Error);
 
-  auto Result = Worker.computeDependenciesByNameWithContext(
+  auto Result = Worker.computeDependenciesByNameWithContextOrError(
       ModuleName, Consumer, Controller);
 
-  if (auto Error = Worker.finalizeCompilerInstanceWithContext())
+  if (auto Error = Worker.finalizeCompilerInstanceWithContextOrError())
     return std::move(Error);
 
   if (Result)
@@ -180,7 +180,7 @@ DependencyScanningTool::getModuleDependencies(
 
 llvm::Error DependencyScanningTool::initializeCompilerInstanceWithContext(
     StringRef CWD, const std::vector<std::string> &CommandLine) {
-  return Worker.initializeCompilerInstanceWithContext(CWD, CommandLine);
+  return Worker.initializeCompilerInstanceWithContextOrError(CWD, CommandLine);
 }
 
 llvm::Expected<TranslationUnitDeps>
@@ -189,7 +189,7 @@ DependencyScanningTool::computeDependenciesByNameWithContext(
     LookupModuleOutputCallback LookupModuleOutput) {
   FullDependencyConsumer Consumer(AlreadySeen);
   CallbackActionController Controller(LookupModuleOutput);
-  llvm::Error Result = Worker.computeDependenciesByNameWithContext(
+  llvm::Error Result = Worker.computeDependenciesByNameWithContextOrError(
       ModuleName, Consumer, Controller);
   if (Result)
     return std::move(Result);
@@ -198,7 +198,7 @@ DependencyScanningTool::computeDependenciesByNameWithContext(
 }
 
 llvm::Error DependencyScanningTool::finalizeCompilerInstanceWithContext() {
-  return Worker.finalizeCompilerInstanceWithContext();
+  return Worker.finalizeCompilerInstanceWithContextOrError();
 }
 
 TranslationUnitDeps FullDependencyConsumer::takeTranslationUnitDeps() {

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

@@ -44,6 +44,7 @@ DependencyScanningWorker::DependencyScanningWorker(
 }
 
 DependencyScanningWorker::~DependencyScanningWorker() = default;
+DependencyActionController::~DependencyActionController() = default;
 
 llvm::Error DependencyScanningWorker::computeDependencies(
     StringRef WorkingDirectory, const std::vector<std::string> &CommandLine,
@@ -167,24 +168,43 @@ bool DependencyScanningWorker::computeDependencies(
   }
 }
 
-llvm::Error DependencyScanningWorker::initializeCompilerInstanceWithContext(
+llvm::Error
+DependencyScanningWorker::initializeCompilerInstanceWithContextOrError(
     StringRef CWD, const std::vector<std::string> &CommandLine) {
+  bool Success = initializeCompilerInstanceWithContext(CWD, CommandLine);
+  return CIWithContext->handleReturnStatus(Success);
+}
+
+llvm::Error
+DependencyScanningWorker::computeDependenciesByNameWithContextOrError(
+    StringRef ModuleName, DependencyConsumer &Consumer,
+    DependencyActionController &Controller) {
+  bool Success =
+      computeDependenciesByNameWithContext(ModuleName, Consumer, Controller);
+  return CIWithContext->handleReturnStatus(Success);
+}
+
+llvm::Error
+DependencyScanningWorker::finalizeCompilerInstanceWithContextOrError() {
+  bool Success = finalizeCompilerInstance();
+  return CIWithContext->handleReturnStatus(Success);
+}
+
+bool DependencyScanningWorker::initializeCompilerInstanceWithContext(
+    StringRef CWD, const std::vector<std::string> &CommandLine,
+    DiagnosticConsumer *DC) {
   CIWithContext =
       std::make_unique<CompilerInstanceWithContext>(*this, CWD, CommandLine);
-  return CIWithContext->initialize();
+  return CIWithContext->initialize(DC);
 }
 
-llvm::Error DependencyScanningWorker::computeDependenciesByNameWithContext(
+bool DependencyScanningWorker::computeDependenciesByNameWithContext(
     StringRef ModuleName, DependencyConsumer &Consumer,
     DependencyActionController &Controller) {
   assert(CIWithContext && "CompilerInstance with context required!");
   return CIWithContext->computeDependencies(ModuleName, Consumer, Controller);
 }
 
-llvm::Error DependencyScanningWorker::finalizeCompilerInstanceWithContext() {
-  llvm::Error E = CIWithContext->finalize();
-  CIWithContext.reset();
-  return E;
+bool DependencyScanningWorker::finalizeCompilerInstance() {
+  return CIWithContext->finalize();
 }
-
-DependencyActionController::~DependencyActionController() {}