[clang][ModulesDriver] Add support for Clang modules to -fmodules-driver

clang/lib/Driver/ModulesDriver.cpp

@@ -25,10 +25,12 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/DepthFirstIterator.h"
 #include "llvm/ADT/DirectedGraph.h"
+#include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVectorExtras.h"
 #include "llvm/ADT/TypeSwitch.h"
 #include "llvm/ADT/iterator_range.h"
+#include "llvm/Option/ArgList.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/GraphWriter.h"
 #include "llvm/Support/JSON.h"
@@ -1248,20 +1250,20 @@ createClangModulePrecompileJob(Compilation &C, const Command &ImportingJob,
       /*Outputs=*/ArrayRef<InputInfo>{});
 }
 
-/// Creates a ClangModuleJobNode and its job for each unique Clang module
-/// in \p ModuleDepGraphsForScannedJobs.
+/// Creates a \c ClangModuleJobNode with associated job for each unique Clang
+/// module in \p ModuleDepGraphsForScannedJobs.
 ///
-/// Only the jobs at indices \p ScannedJobIndices in \p Jobs are expected to be
-/// non-null.
+/// \param ImportingJobs Jobs whose module dependencies were scanned.
+/// \param ModuleDepGraphsForScannedJobs Full Clang module dependency graphs
+/// corresponding to \p ImportingJobs, in order.
 static void createClangModuleJobsAndNodes(
     CompilationGraph &Graph, Compilation &C,
-    ArrayRef<std::unique_ptr<Command>> Jobs, ArrayRef<size_t> ScannedJobIndices,
+    ArrayRef<std::unique_ptr<Command>> ImportingJobs,
     SmallVectorImpl<deps::ModuleDepsGraph> &&ModuleDepGraphsForScannedJobs) {
   llvm::DenseSet<deps::ModuleID> AlreadySeen;
-  for (auto &&[ScanIndex, ModuleDepsGraph] :
-       llvm::enumerate(ModuleDepGraphsForScannedJobs)) {
-    const auto &ImportingJob = *Jobs[ScannedJobIndices[ScanIndex]];
-
+  for (auto &&[ImportingJob, ModuleDepsGraph] :
+       llvm::zip_equal(llvm::make_pointee_range(ImportingJobs),
+                       ModuleDepGraphsForScannedJobs)) {
     for (auto &MD : ModuleDepsGraph) {
       const auto Inserted = AlreadySeen.insert(MD.ID).second;
       if (!Inserted)
@@ -1274,6 +1276,31 @@ static void createClangModuleJobsAndNodes(
   }
 }
 
+/// Installs the command lines produced by the dependency scan into
+/// \p ScannedJobs.
+static void
+installScanCommandLines(Compilation &C,
+                        MutableArrayRef<std::unique_ptr<Command>> ScannedJobs,
+                        ArrayRef<InputDependencies> InputDepsForScannedJobs) {
+  for (auto &&[Job, InputDeps] : llvm::zip_equal(
+           llvm::make_pointee_range(ScannedJobs), InputDepsForScannedJobs)) {
+    const auto &BuildArgs = InputDeps.BuildArgs;
+    ArgStringList JobArgs;
+    JobArgs.reserve(BuildArgs.size());
+
+    const auto &SourceAction = Job.getSource();
+    const auto &TC = Job.getCreator().getToolChain();
+    auto &TCArgs =
+        C.getArgsForToolChain(&TC, SourceAction.getOffloadingArch(),
+                              SourceAction.getOffloadingDeviceKind());
+
+    for (const auto &Arg : BuildArgs)
+      JobArgs.push_back(TCArgs.MakeArgString(Arg));
+
+    Job.replaceArguments(std::move(JobArgs));
+  }
+}
+
 /// Creates nodes for all jobs which were scanned for dependencies.
 ///
 /// The updated command lines produced by the dependency scan are installed at a
@@ -1506,16 +1533,17 @@ void driver::modules::runModulesDriver(
       Graph, takeJobsAtIndices(Jobs, ScanResult.NonScannableJobIndices));
   auto UnusedStdlibModuleJobNodes = createNodesForUnusedStdlibModuleJobs(
       Graph, takeJobsAtIndices(Jobs, ScanResult.UnusedStdlibModuleJobIndices));
+
+  auto ScannedJobs = takeJobsAtIndices(Jobs, ScanResult.ScannedJobIndices);
   createClangModuleJobsAndNodes(
-      Graph, C, Jobs, ScanResult.ScannedJobIndices,
+      Graph, C, /*ImportingJobs*/ ScannedJobs,
       std::move(ScanResult.ModuleDepGraphsForScannedJobs));
-  createNodesForScannedJobs(
-      Graph, takeJobsAtIndices(Jobs, ScanResult.ScannedJobIndices),
-      std::move(ScanResult.InputDepsForScannedJobs));
-  createRegularEdges(Graph);
+  installScanCommandLines(C, ScannedJobs, ScanResult.InputDepsForScannedJobs);
+  createNodesForScannedJobs(Graph, std::move(ScannedJobs),
+                            std::move(ScanResult.InputDepsForScannedJobs));
 
+  createRegularEdges(Graph);
   pruneUnusedStdlibModuleJobs(Graph, UnusedStdlibModuleJobNodes);
-
   if (!createModuleDependencyEdges(Graph, Diags))
     return;
   createAndConnectRoot(Graph);
@@ -1524,12 +1552,14 @@ void driver::modules::runModulesDriver(
   if (!Diags.isLastDiagnosticIgnored())
     llvm::WriteGraph<const CompilationGraph *>(llvm::errs(), &Graph);
 
-  // TODO: Install all updated command-lines produced by the dependency scan.
   // TODO: Fix-up command-lines for named module imports.
 
-  // TODO: Sort the graph topologically before adding jobs back to the
-  // Compilation being built.
-  for (auto *N : Graph)
-    if (auto *JN = dyn_cast<JobNode>(N))
-      C.addCommand(std::move(JN->Job));
+  llvm::ReversePostOrderTraversal<CompilationGraph *> TopologicallySortedNodes(
+      &Graph);
+  assert(isa<RootNode>(*TopologicallySortedNodes.begin()) &&
+         "First node in topological order must be the root!");
+  auto TopologicallySortedJobNodes = llvm::map_range(
+      llvm::drop_begin(TopologicallySortedNodes), llvm::CastTo<JobNode>);
+  for (auto *JN : TopologicallySortedJobNodes)
+    C.addCommand(std::move(JN->Job));
 }

clang/test/Driver/modules-driver-clang-modules-only.cpp

@@ -0,0 +1,127 @@
+// Checks that -fmodules-driver correctly handles compilations using Clang modules.
+
+// RUN: split-file %s %t
+// RUN: rm -rf %t/modules-cache
+
+// RUN: %clang -std=c++23 \
+// RUN:   -fmodules-driver -Rmodules-driver \
+// RUN:   -fmodules -Rmodule-import \
+// RUN:   -fmodule-map-file=%t/module.modulemap \
+// RUN:   -fmodules-cache-path=%t/modules-cache \
+// RUN:   -fsyntax-only %t/main.cpp 2>&1 \
+// RUN:   | sed 's:\\\\\?:/:g' \
+// RUN:   | FileCheck -DPREFIX=%/t --check-prefix=CHECK-REMARKS %s
+
+// RUN: rm -rf %t/modules-cache
+// RUN: %clang -std=c++23 \
+// RUN:   -fmodules-driver \
+// RUN:   -fmodules \
+// RUN:   -fmodule-map-file=%t/module.modulemap \
+// RUN:   -fmodules-cache-path=%t/modules-cache \
+// RUN:   -fsyntax-only %t/main.cpp \
+// RUN:   -### 2>&1 \
+// RUN:   | sed 's:\\\\\?:/:g' \
+// RUN:   | FileCheck --check-prefix=CHECK-CC1 %s
+
+// The scan itself will also produce [-Rmodule-import] remarks.
+// Let's skip past them, we only care about the final -cc1 commands.
+// CHECK-REMARKS:       clang: remark: printing module dependency graph [-Rmodules-driver]
+// CHECK-REMARKS-NEXT:  digraph "Module Dependency Graph" {
+// CHECK-REMARKS:       }
+
+// CHECK-REMARKS: [[PREFIX]]/main.cpp:1:2: remark: importing module 'root' from
+// CHECK-REMARKS: [[PREFIX]]/main.cpp:1:2: remark: importing module 'direct1' into 'root'
+// CHECK-REMARKS: [[PREFIX]]/main.cpp:1:2: remark: importing module 'transitive1' into 'direct1'
+// CHECK-REMARKS: [[PREFIX]]/main.cpp:1:2: remark: importing module 'transitive2' into 'direct1'
+// CHECK-REMARKS: [[PREFIX]]/main.cpp:1:2: remark: importing module 'direct2' into 'root'
+// CHECK-REMARKS: [[PREFIX]]/main.cpp:1:2: remark: importing module 'transitive2' into 'direct2'
+
+// CHECK-CC1: "-cc1"
+// CHECK-CC1-SAME: "-o" "[[TRANSITIVE2PCM:[^"]+]]"
+// CHECK-CC1-SAME: "-emit-module"
+// CHECK-CC1-SAME: "-fmodule-name=transitive2"
+// CHECK-CC1-SAME: "-fno-implicit-modules"
+
+// CHECK-CC1: "-cc1"
+// CHECK-CC1-SAME: "-o" "[[DIRECT2PCM:[^"]+]]"
+// CHECK-CC1-SAME: "-emit-module"
+// CHECK-CC1-SAME: "-fmodule-file=transitive2=[[TRANSITIVE2PCM]]"
+// CHECK-CC1-SAME: "-fmodule-name=direct2"
+// CHECK-CC1-SAME: "-fno-implicit-modules"
+
+// CHECK-CC1: "-cc1"
+// CHECK-CC1-SAME: "-o" "[[TRANSITIVE1PCM:[^"]+]]"
+// CHECK-CC1-SAME: "-emit-module"
+// CHECK-CC1-SAME: "-fmodule-name=transitive1"
+// CHECK-CC1-SAME: "-fno-implicit-modules"
+
+// CHECK-CC1: "-cc1"
+// CHECK-CC1-SAME: "-o" "[[DIRECT1PCM:[^"]+]]"
+// CHECK-CC1-SAME: "-emit-module"
+// CHECK-CC1-SAME: "-fmodule-file=transitive1=[[TRANSITIVE1PCM]]"
+// CHECK-CC1-SAME: "-fmodule-file=transitive2=[[TRANSITIVE2PCM]]"
+// CHECK-CC1-SAME: "-fmodule-name=direct1"
+// CHECK-CC1-SAME: "-fno-implicit-modules"
+
+// CHECK-CC1: "-cc1"
+// CHECK-CC1-SAME: "-o" "[[ROOTPCM:[^"]+]]"
+// CHECK-CC1-SAME: "-emit-module"
+// CHECK-CC1-SAME: "-fmodule-file=direct1=[[DIRECT1PCM]]"
+// CHECK-CC1-SAME: "-fmodule-file=direct2=[[DIRECT2PCM]]"
+// CHECK-CC1-SAME: "-fmodule-name=root"
+// CHECK-CC1-SAME: "-fno-implicit-modules"
+
+// CHECK-CC1: "-cc1"
+// CHECK-CC1-SAME: "-fsyntax-only"
+// CHECK-CC1-SAME: "{{.*}}/main.cpp"
+// CHECK-CC1-SAME: "-fmodule-file=root=[[ROOTPCM]]"
+// CHECK-CC1-SAME: "-fno-implicit-modules"
+
+// (Because of missing include guards, this example would also run into
+// redefinition errors when compiling without modules.)
+
+/--- module.modulemap
+module root { header "root.h"}
+module transitive1 { header "transitive1.h" }
+module transitive2 { header "transitive2.h" }
+module direct1 { header "direct1.h" }
+module direct2 { header "direct2.h" }
+
+//--- root.h
+#include "direct1.h"
+#include "direct2.h"
+int fromRoot() {
+  return fromDirect1() + fromDirect2();
+}
+
+//--- direct1.h
+#include "transitive1.h"
+#include "transitive2.h"
+
+int fromDirect1() {
+  return fromTransitive1() + fromTransitive2();
+}
+
+//--- direct2.h
+#include "transitive2.h"
+
+int fromDirect2() {
+  return fromTransitive2() + 2;
+}
+
+//--- transitive1.h
+int fromTransitive1() {
+    return 20;
+}
+
+//--- transitive2.h
+int fromTransitive2() {
+    return 10;
+}
+
+//--- main.cpp
+#include "root.h"
+
+int main() {
+ fromRoot();
+}

clang/test/Driver/modules-driver-manifest-input-args.cpp

@@ -26,17 +26,15 @@
 // RUN:   %t/main.cpp \
 // RUN:   -### 2>&1 \
 // RUN:   | sed 's:\\\\\?:/:g' \
-// RUN:   | FileCheck %s -check-prefix=MAIN-CC1 -check-prefix=STDLIB-MOD-CC1 -DPREFIX=%/t
+// RUN:   | FileCheck %s -DPREFIX=%/t
 
-// MAIN-CC1: "-cc1"
-// MAIN-CC1-SAME: "main.cpp"
-// MAIN-CC1-NOT: "-Wno-reserved-module-identifier"
-// MAIN-CC1-NOT: "-internal-isystem" "[[PREFIX]]/Inputs/usr/lib/x86_64-linux-gnu/../share/libc++/v1/"
+// CHECK: "-cc1" {{.*}} "-Wno-reserved-module-identifier" {{.*}} "[[PREFIX]]/Inputs/usr/lib/x86_64-linux-gnu/../share/libc++/v1/std.cppm" {{.*}} "-internal-isystem" "[[PREFIX]]/Inputs/usr/lib/x86_64-linux-gnu/../share/libc++/v1/"
 
-// STDLIB-MOD-CC1: "-cc1"
-// STDLIB-MOD-CC1-SAME: "[[PREFIX]]/Inputs/usr/lib/x86_64-linux-gnu/../share/libc++/v1/std.cppm"
-// STDLIB-MOD-CC1-SAME: "-Wno-reserved-module-identifier"
-// STDLIB-MOD-CC1-SAME: "-internal-isystem" "[[PREFIX]]/Inputs/usr/lib/x86_64-linux-gnu/../share/libc++/v1/"
+// The adjustments should only be made for inputs from the Standard Library module manifest:
+// Check that the -cc1 command line does not contain those adjustments!
+// CHECK: "-cc1" {{.*}}main
+// CHECK-NOT: "-Wno-reserved-module-identifier"
+// CHECK-NOT: "-internal-isystem" "[[PREFIX]]/Inputs/usr/lib/x86_64-linux-gnu/../share/libc++/v1/"
 
 //--- main.cpp
 import std;