[clang][Dependency Scanning][NFC] Extract the DependencyScanningTool Logic to Initialize the Compiler Instance With Context

[qiongsiwu]

#171238 moved the compiler instance with context initialization logic from DependencyScanningWorker to DependencyScanningTools. This led to a problem in the Swift fork (https://github.com/swiftlang/llvm-project/blob/905c010b8e793d85a1cfbbdb93dc29d75d06b343/clang/tools/libclang/CDependencies.cpp#L359) where we need to initialize the dependency scanning worker without an instance of the dependency scanning tool.

This patch extracts the logic that generates the cc1 command to a static method so we can facilitate this use case in Swift.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Qiongsi Wu (qiongsiwu)

Changes

#171238 moved the compiler instance with context initialization logic from DependencyScanningWorker to DependencyScanningTools. This led to a problem in the Swift fork (https://github.com/swiftlang/llvm-project/blob/905c010b8e793d85a1cfbbdb93dc29d75d06b343/clang/tools/libclang/CDependencies.cpp#L359) where we need to initialize the dependency scanning worker without an instance of the dependency scanning tool.

This patch extracts the logic that generates the cc1 command to a static method so we can facilitate this use case in Swift.

---

Full diff: https://github.com/llvm/llvm-project/pull/172345.diff

2 Files Affected:

• (modified) clang/include/clang/Tooling/DependencyScanningTool.h (+14)
• (modified) clang/lib/Tooling/DependencyScanningTool.cpp (+23-16)

diff --git a/clang/include/clang/Tooling/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanningTool.h
index e796ed648db35..415dcf17c37b9 100644
--- a/clang/include/clang/Tooling/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanningTool.h
@@ -151,6 +151,20 @@ class DependencyScanningTool {
 
   llvm::vfs::FileSystem &getWorkerVFS() const { return Worker.getVFS(); }
 
+  /// @brief Initialize the worker's compiler instance from the commandline.
+  ///        The compiler instance only takes a `-cc1` job, so this method
+  ///        builds the `-cc1` job from the CommandLine input.
+  /// @param Worker The dependency scanning worker whose compiler instance
+  ///        with context is initialized.
+  /// @param CWD The current working directory.
+  /// @param CommandLine This command line may be a driver command or a cc1
+  ///        command.
+  /// @param DC A diagnostics consumer to report error if the initialization
+  ///        fails.
+  static bool initializeWorkerCIWithContextFromCommandline(
+      clang::dependencies::DependencyScanningWorker &Worker, StringRef CWD,
+      ArrayRef<std::string> CommandLine, DiagnosticConsumer &DC);
+
 private:
   dependencies::DependencyScanningWorker Worker;
   std::unique_ptr<dependencies::TextDiagnosticsPrinterWithOutput>
diff --git a/clang/lib/Tooling/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanningTool.cpp
index 74cc6af3551f8..85f261c8ebbf1 100644
--- a/clang/lib/Tooling/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanningTool.cpp
@@ -217,19 +217,13 @@ static llvm::Error makeErrorFromDiagnosticsOS(
       DiagPrinterWithOS.DiagnosticsOS.str(), llvm::inconvertibleErrorCode());
 }
 
-llvm::Error
-DependencyScanningTool::initializeCompilerInstanceWithContextOrError(
-    StringRef CWD, ArrayRef<std::string> CommandLine) {
-  DiagPrinterWithOS =
-      std::make_unique<TextDiagnosticsPrinterWithOutput>(CommandLine);
-
+bool DependencyScanningTool::initializeWorkerCIWithContextFromCommandline(
+    DependencyScanningWorker &Worker, StringRef CWD,
+    ArrayRef<std::string> CommandLine, DiagnosticConsumer &DC) {
   if (CommandLine.size() >= 2 && CommandLine[1] == "-cc1") {
     // The input command line is already a -cc1 invocation; initialize the
     // compiler instance directly from it.
-    if (Worker.initializeCompilerInstanceWithContext(
-            CWD, CommandLine, DiagPrinterWithOS->DiagPrinter))
-      return llvm::Error::success();
-    return makeErrorFromDiagnosticsOS(*DiagPrinterWithOS);
+    return Worker.initializeCompilerInstanceWithContext(CWD, CommandLine, DC);
   }
 
   // The input command line is either a driver-style command line, or
@@ -241,18 +235,31 @@ DependencyScanningTool::initializeCompilerInstanceWithContextOrError(
   const auto &ModifiedCommandLine = OverlayFSAndArgs.second;
 
   auto DiagEngineWithCmdAndOpts =
-      std::make_unique<DiagnosticsEngineWithDiagOpts>(
-          ModifiedCommandLine, OverlayFS, DiagPrinterWithOS->DiagPrinter);
+      std::make_unique<DiagnosticsEngineWithDiagOpts>(ModifiedCommandLine,
+                                                      OverlayFS, DC);
 
   const auto MaybeFirstCC1 = getFirstCC1CommandLine(
       ModifiedCommandLine, *DiagEngineWithCmdAndOpts->DiagEngine, OverlayFS);
   if (!MaybeFirstCC1)
-    return makeErrorFromDiagnosticsOS(*DiagPrinterWithOS);
+    return false;
+
+  return Worker.initializeCompilerInstanceWithContext(
+      CWD, *MaybeFirstCC1, std::move(DiagEngineWithCmdAndOpts), OverlayFS);
+}
 
-  if (Worker.initializeCompilerInstanceWithContext(
-          CWD, *MaybeFirstCC1, std::move(DiagEngineWithCmdAndOpts), OverlayFS))
+llvm::Error
+DependencyScanningTool::initializeCompilerInstanceWithContextOrError(
+    StringRef CWD, ArrayRef<std::string> CommandLine) {
+  DiagPrinterWithOS =
+      std::make_unique<TextDiagnosticsPrinterWithOutput>(CommandLine);
+
+  bool Result = initializeWorkerCIWithContextFromCommandline(
+      Worker, CWD, CommandLine, DiagPrinterWithOS->DiagPrinter);
+
+  if (Result)
     return llvm::Error::success();
-  return makeErrorFromDiagnosticsOS(*DiagPrinterWithOS);
+  else
+    return makeErrorFromDiagnosticsOS(*DiagPrinterWithOS);
 }
 
 llvm::Expected<TranslationUnitDeps>

[naveen-seth]

LGTM!

[naveen-seth]

Sorry I didn't notice this earlier!

https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return

Suggested change

	else
	return makeErrorFromDiagnosticsOS(*DiagPrinterWithOS);
	return makeErrorFromDiagnosticsOS(*DiagPrinterWithOS);

[qiongsiwu]

Ah good catch! Thanks! I will fix this soon!