[llvm-branch-commits] [clang] 5e0f3cd - [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

Tobias Hieta via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Mon Feb 13 02:28:24 PST 2023


Author: Chuanqi Xu
Date: 2023-02-13T11:27:45+01:00
New Revision: 5e0f3cd48fa640d3fa61b8b20d34e03ec4fc4831

URL: https://github.com/llvm/llvm-project/commit/5e0f3cd48fa640d3fa61b8b20d34e03ec4fc4831
DIFF: https://github.com/llvm/llvm-project/commit/5e0f3cd48fa640d3fa61b8b20d34e03ec4fc4831.diff

LOG: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

Required in https://reviews.llvm.org/D137534.

The build systems needs the information to know that "header X changed,
scanning may have changed, so please rerun scanning". Although it is
possible to get the information by running clang-scan-deps for the
second time with make format, it is not user friendly clearly.

Reviewed By: jansvoboda11

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

Added: 
    

Modified: 
    clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
    clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
    clang/test/ClangScanDeps/P1689.cppm
    clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index d0c0c99fd25ea..b1a4df141edc7 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -93,9 +93,21 @@ class DependencyScanningTool {
   getDependencyFile(const std::vector<std::string> &CommandLine, StringRef CWD,
                     std::optional<StringRef> ModuleName = std::nullopt);
 
+  /// Collect the module dependency in P1689 format for C++20 named modules.
+  ///
+  /// \param MakeformatOutput The output parameter for dependency information
+  /// in make format if the command line requires to generate make-format
+  /// dependency information by `-MD -MF <dep_file>`.
+  ///
+  /// \param MakeformatOutputPath The output parameter for the path to
+  /// \param MakeformatOutput.
+  ///
+  /// \returns A \c StringError with the diagnostic output if clang errors
+  /// occurred, P1689 dependency format rules otherwise.
   llvm::Expected<P1689Rule>
-  getP1689ModuleDependencyFile(const CompileCommand &Command,
-                               StringRef CWD);
+  getP1689ModuleDependencyFile(
+      const clang::tooling::CompileCommand &Command, StringRef CWD,
+      std::string &MakeformatOutput, std::string &MakeformatOutputPath);
 
   /// Given a Clang driver command-line for a translation unit, gather the
   /// modular dependencies and return the information needed for explicit build.

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index eb01584073cb9..ae1662237e874 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -40,67 +40,69 @@ DependencyScanningTool::DependencyScanningTool(
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
     : Worker(Service, std::move(FS)) {}
 
-llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
-    const std::vector<std::string> &CommandLine, StringRef CWD,
-    std::optional<StringRef> ModuleName) {
-  /// Prints out all of the gathered dependencies into a string.
-  class MakeDependencyPrinterConsumer : public DependencyConsumer {
-  public:
-    void handleBuildCommand(Command) override {}
+namespace {
+/// Prints out all of the gathered dependencies into a string.
+class MakeDependencyPrinterConsumer : public DependencyConsumer {
+public:
+  void handleBuildCommand(Command) override {}
+
+  void
+  handleDependencyOutputOpts(const DependencyOutputOptions &Opts) override {
+    this->Opts = std::make_unique<DependencyOutputOptions>(Opts);
+  }
 
-    void
-    handleDependencyOutputOpts(const DependencyOutputOptions &Opts) override {
-      this->Opts = std::make_unique<DependencyOutputOptions>(Opts);
-    }
+  void handleFileDependency(StringRef File) override {
+    Dependencies.push_back(std::string(File));
+  }
 
-    void handleFileDependency(StringRef File) override {
-      Dependencies.push_back(std::string(File));
-    }
+  void handlePrebuiltModuleDependency(PrebuiltModuleDep PMD) override {
+    // Same as `handleModuleDependency`.
+  }
 
-    void handlePrebuiltModuleDependency(PrebuiltModuleDep PMD) override {
-      // Same as `handleModuleDependency`.
-    }
+  void handleModuleDependency(ModuleDeps MD) override {
+    // These are ignored for the make format as it can't support the full
+    // set of deps, and handleFileDependency handles enough for implicitly
+    // built modules to work.
+  }
 
-    void handleModuleDependency(ModuleDeps MD) override {
-      // These are ignored for the make format as it can't support the full
-      // set of deps, and handleFileDependency handles enough for implicitly
-      // built modules to work.
-    }
+  void handleContextHash(std::string Hash) override {}
 
-    void handleContextHash(std::string Hash) override {}
+  std::string lookupModuleOutput(const ModuleID &ID,
+                                 ModuleOutputKind Kind) override {
+    llvm::report_fatal_error("unexpected call to lookupModuleOutput");
+  }
 
-    std::string lookupModuleOutput(const ModuleID &ID,
-                                   ModuleOutputKind Kind) override {
-      llvm::report_fatal_error("unexpected call to lookupModuleOutput");
-    }
+  void printDependencies(std::string &S) {
+    assert(Opts && "Handled dependency output options.");
 
-    void printDependencies(std::string &S) {
-      assert(Opts && "Handled dependency output options.");
-
-      class DependencyPrinter : public DependencyFileGenerator {
-      public:
-        DependencyPrinter(DependencyOutputOptions &Opts,
-                          ArrayRef<std::string> Dependencies)
-            : DependencyFileGenerator(Opts) {
-          for (const auto &Dep : Dependencies)
-            addDependency(Dep);
-        }
-
-        void printDependencies(std::string &S) {
-          llvm::raw_string_ostream OS(S);
-          outputDependencyFile(OS);
-        }
-      };
-
-      DependencyPrinter Generator(*Opts, Dependencies);
-      Generator.printDependencies(S);
-    }
+    class DependencyPrinter : public DependencyFileGenerator {
+    public:
+      DependencyPrinter(DependencyOutputOptions &Opts,
+                        ArrayRef<std::string> Dependencies)
+          : DependencyFileGenerator(Opts) {
+        for (const auto &Dep : Dependencies)
+          addDependency(Dep);
+      }
 
-  private:
-    std::unique_ptr<DependencyOutputOptions> Opts;
-    std::vector<std::string> Dependencies;
-  };
+      void printDependencies(std::string &S) {
+        llvm::raw_string_ostream OS(S);
+        outputDependencyFile(OS);
+      }
+    };
 
+    DependencyPrinter Generator(*Opts, Dependencies);
+    Generator.printDependencies(S);
+  }
+
+protected:
+  std::unique_ptr<DependencyOutputOptions> Opts;
+  std::vector<std::string> Dependencies;
+};
+} // anonymous namespace
+
+llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
+    const std::vector<std::string> &CommandLine, StringRef CWD,
+    std::optional<StringRef> ModuleName) {
   MakeDependencyPrinterConsumer Consumer;
   auto Result =
       Worker.computeDependencies(CWD, CommandLine, Consumer, ModuleName);
@@ -112,8 +114,10 @@ llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
 }
 
 llvm::Expected<P1689Rule> DependencyScanningTool::getP1689ModuleDependencyFile(
-    const CompileCommand &Command, StringRef CWD) {
-  class P1689ModuleDependencyPrinterConsumer : public DependencyConsumer {
+    const CompileCommand &Command, StringRef CWD,
+    std::string &MakeformatOutput, std::string &MakeformatOutputPath) {
+  class P1689ModuleDependencyPrinterConsumer
+      : public MakeDependencyPrinterConsumer {
   public:
     P1689ModuleDependencyPrinterConsumer(P1689Rule &Rule,
                                          const CompileCommand &Command)
@@ -121,17 +125,6 @@ llvm::Expected<P1689Rule> DependencyScanningTool::getP1689ModuleDependencyFile(
       Rule.PrimaryOutput = Command.Output;
     }
 
-    void
-    handleDependencyOutputOpts(const DependencyOutputOptions &Opts) override {}
-    void handleFileDependency(StringRef File) override {}
-    void handlePrebuiltModuleDependency(PrebuiltModuleDep PMD) override {}
-    void handleModuleDependency(ModuleDeps MD) override {}
-    void handleContextHash(std::string Hash) override {}
-    std::string lookupModuleOutput(const ModuleID &ID,
-                                   ModuleOutputKind Kind) override {
-      llvm::report_fatal_error("unexpected call to lookupModuleOutput");
-    }
-
     void handleProvidedAndRequiredStdCXXModules(
         std::optional<P1689ModuleInfo> Provided,
         std::vector<P1689ModuleInfo> Requires) override {
@@ -141,6 +134,12 @@ llvm::Expected<P1689Rule> DependencyScanningTool::getP1689ModuleDependencyFile(
       Rule.Requires = Requires;
     }
 
+    StringRef getMakeFormatDependencyOutputPath() {
+      if (Opts->OutputFormat != DependencyOutputFormat::Make)
+        return {};
+      return Opts->OutputFile;
+    }
+
   private:
     StringRef Filename;
     P1689Rule &Rule;
@@ -151,6 +150,10 @@ llvm::Expected<P1689Rule> DependencyScanningTool::getP1689ModuleDependencyFile(
   auto Result = Worker.computeDependencies(CWD, Command.CommandLine, Consumer);
   if (Result)
     return std::move(Result);
+
+  MakeformatOutputPath = Consumer.getMakeFormatDependencyOutputPath();
+  if (!MakeformatOutputPath.empty())
+    Consumer.printDependencies(MakeformatOutput);
   return Rule;
 }
 

diff  --git a/clang/test/ClangScanDeps/P1689.cppm b/clang/test/ClangScanDeps/P1689.cppm
index 22d7721c2e9bf..eb1d8d108dc49 100644
--- a/clang/test/ClangScanDeps/P1689.cppm
+++ b/clang/test/ClangScanDeps/P1689.cppm
@@ -1,3 +1,8 @@
+// It is annoying to handle 
diff erent slash direction
+// in the filesystem of Windows and Linux.
+// So we disable the test on Windows here.
+// REQUIRES: !system-windows
+//
 // RUN: rm -fr %t
 // RUN: mkdir -p %t
 // RUN: split-file %s %t
@@ -25,42 +30,50 @@
 // RUN: clang-scan-deps -format=p1689 \
 // RUN:   -- %clang++ -std=c++20 -c -fprebuilt-module-path=%t %t/User.cpp -o %t/User.o \
 // RUN:   | FileCheck %t/User.cpp -DPREFIX=%/t
+//
+// Check we can generate the make-style dependencies as expected.
+// RUN: clang-scan-deps -format=p1689 \
+// RUN:   -- %clang++ -std=c++20 -c -fprebuilt-module-path=%t %t/impl_part.cppm -o %t/impl_part.o \
+// RUN:      -MT %t/impl_part.o.ddi -MD -MF %t/impl_part.dep
+// RUN:   cat %t/impl_part.dep | FileCheck %t/impl_part.cppm -DPREFIX=%/t --check-prefix=CHECK-MAKE
+//
+// Check that we can generate multiple make-style dependency information with compilation database.
+// RUN: cat %t/P1689.dep | FileCheck %t/Checks.cpp -DPREFIX=%/t --check-prefix=CHECK-MAKE
 
 //--- P1689.json.in
 [
 {
   "directory": "DIR",
-  "command": "clang++ -std=c++20 DIR/M.cppm -c -o DIR/M.o",
+  "command": "clang++ -std=c++20 DIR/M.cppm -c -o DIR/M.o -MT DIR/M.o.ddi -MD -MF DIR/P1689.dep",
   "file": "DIR/M.cppm",
   "output": "DIR/M.o"
 },
 {
   "directory": "DIR",
-  "command": "clang++ -std=c++20 DIR/Impl.cpp -c -o DIR/Impl.o",
+  "command": "clang++ -std=c++20 DIR/Impl.cpp -c -o DIR/Impl.o -MT DIR/Impl.o.ddi -MD -MF DIR/P1689.dep",
   "file": "DIR/Impl.cpp",
   "output": "DIR/Impl.o"
 },
 {
   "directory": "DIR",
-  "command": "clang++ -std=c++20 DIR/impl_part.cppm -c -o DIR/impl_part.o",
+  "command": "clang++ -std=c++20 DIR/impl_part.cppm -c -o DIR/impl_part.o -MT DIR/impl_part.o.ddi -MD -MF DIR/P1689.dep",
   "file": "DIR/impl_part.cppm",
   "output": "DIR/impl_part.o"
 },
 {
   "directory": "DIR",
-  "command": "clang++ -std=c++20 DIR/interface_part.cppm -c -o DIR/interface_part.o",
+  "command": "clang++ -std=c++20 DIR/interface_part.cppm -c -o DIR/interface_part.o -MT DIR/interface_part.o.ddi -MD -MF DIR/P1689.dep",
   "file": "DIR/interface_part.cppm",
   "output": "DIR/interface_part.o"
 },
 {
   "directory": "DIR",
-  "command": "clang++ -std=c++20 DIR/User.cpp -c -o DIR/User.o",
+  "command": "clang++ -std=c++20 DIR/User.cpp -c -o DIR/User.o -MT DIR/User.o.ddi -MD -MF DIR/P1689.dep",
   "file": "DIR/User.cpp",
   "output": "DIR/User.o"
 }
 ]
 
-
 //--- M.cppm
 export module M;
 export import :interface_part;
@@ -275,4 +288,17 @@ int main() {
 // CHECK-NEXT:   "version": 1
 // CHECK-NEXT: }
 
+// CHECK-MAKE-DAG: [[PREFIX]]/impl_part.o.ddi: \
+// CHECK-MAKE-DAG-NEXT:   [[PREFIX]]/impl_part.cppm \
+// CHECK-MAKE-DAG-NEXT:   [[PREFIX]]/header.mock
+// CHECK-MAKE-DAG: [[PREFIX]]/interface_part.o.ddi: \
+// CHECK-MAKE-DAG-NEXT:   [[PREFIX]]/interface_part.cppm
+// CHECK-MAKE-DAG: [[PREFIX]]/M.o.ddi: \
+// CHECK-MAKE-DAG-NEXT:   [[PREFIX]]/M.cppm
+// CHECK-MAKE-DAG: [[PREFIX]]/User.o.ddi: \
+// CHECK-MAKE-DAG-NEXT:   [[PREFIX]]/User.cpp
+// CHECK-MAKE-DAG: [[PREFIX]]/Impl.o.ddi: \
+// CHECK-MAKE-DAG-NEXT:   [[PREFIX]]/Impl.cpp \
+// CHECK-MAKE-DAG-NEXT:   [[PREFIX]]/header.mock
+
 //--- header.mock

diff  --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index e283a41987904..9c7d454c9efd2 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -769,10 +769,46 @@ int main(int argc, const char **argv) {
                                              Errs))
             HadErrors = true;
         } else if (Format == ScanningOutputFormat::P1689) {
-          auto MaybeRule =
-              WorkerTools[I]->getP1689ModuleDependencyFile(*Input, CWD);
-          if (handleP1689DependencyToolResult(Filename, MaybeRule, PD, Errs))
-            HadErrors = true;
+          // It is useful to generate the make-format dependency output during
+          // the scanning for P1689. Otherwise the users need to scan again for
+          // it. We will generate the make-format dependency output if we find
+          // `-MF` in the command lines.
+          std::string MakeformatOutputPath;
+          std::string MakeformatOutput;
+
+          auto MaybeRule = WorkerTools[I]->getP1689ModuleDependencyFile(
+              *Input, CWD, MakeformatOutput, MakeformatOutputPath);
+          HadErrors =
+              handleP1689DependencyToolResult(Filename, MaybeRule, PD, Errs);
+
+          if (!MakeformatOutputPath.empty() && !MakeformatOutput.empty() &&
+              !HadErrors) {
+            static std::mutex Lock;
+            // With compilation database, we may open 
diff erent files
+            // concurrently or we may write the same file concurrently. So we
+            // use a map here to allow multiple compile commands to write to the
+            // same file. Also we need a lock here to avoid data race.
+            static llvm::StringMap<llvm::raw_fd_ostream> OSs;
+            std::unique_lock<std::mutex> LockGuard(Lock);
+
+            auto OSIter = OSs.find(MakeformatOutputPath);
+            if (OSIter == OSs.end()) {
+              std::error_code EC;
+              OSIter = OSs.try_emplace(MakeformatOutputPath,
+                                       MakeformatOutputPath, EC)
+                           .first;
+              if (EC)
+                llvm::errs()
+                    << "Failed to open P1689 make format output file \""
+                    << MakeformatOutputPath << "\" for " << EC.message()
+                    << "\n";
+            }
+
+            SharedStream MakeformatOS(OSIter->second);
+            llvm::Expected<std::string> MaybeOutput(MakeformatOutput);
+            HadErrors = handleMakeDependencyToolResult(Filename, MaybeOutput,
+                                                       MakeformatOS, Errs);
+          }
         } else if (DeprecatedDriverCommand) {
           auto MaybeFullDeps =
               WorkerTools[I]->getFullDependenciesLegacyDriverCommand(


        


More information about the llvm-branch-commits mailing list