From c122bd5c6ee7573224d34b91b893ccfe65ffd3a6 Mon Sep 17 00:00:00 2001 From: Naveen Seth Hanig Date: Thu, 9 Apr 2026 09:09:51 +0200 Subject: [PATCH] =?UTF-8?q?Revert=20"[clang][ModulesDriver]=20Add=20suppor?= =?UTF-8?q?t=20for=20Clang=20modules=20to=20-fmodules-dri=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 6cb4f3955424c37261e90c9970e2a0fcaae4b23f. --- clang/lib/Driver/ModulesDriver.cpp | 72 +++------- .../modules-driver-clang-modules-only.cpp | 127 ------------------ .../modules-driver-manifest-input-args.cpp | 16 ++- 3 files changed, 30 insertions(+), 185 deletions(-) delete mode 100644 clang/test/Driver/modules-driver-clang-modules-only.cpp diff --git a/clang/lib/Driver/ModulesDriver.cpp b/clang/lib/Driver/ModulesDriver.cpp index 2df40f761680f..2cd274a1f3099 100644 --- a/clang/lib/Driver/ModulesDriver.cpp +++ b/clang/lib/Driver/ModulesDriver.cpp @@ -25,12 +25,10 @@ #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" @@ -1250,20 +1248,20 @@ createClangModulePrecompileJob(Compilation &C, const Command &ImportingJob, /*Outputs=*/ArrayRef{}); } -/// Creates a \c ClangModuleJobNode with associated job for each unique Clang -/// module in \p ModuleDepGraphsForScannedJobs. +/// Creates a ClangModuleJobNode and its job for each unique Clang module +/// in \p ModuleDepGraphsForScannedJobs. /// -/// \param ImportingJobs Jobs whose module dependencies were scanned. -/// \param ModuleDepGraphsForScannedJobs Full Clang module dependency graphs -/// corresponding to \p ImportingJobs, in order. +/// Only the jobs at indices \p ScannedJobIndices in \p Jobs are expected to be +/// non-null. static void createClangModuleJobsAndNodes( CompilationGraph &Graph, Compilation &C, - ArrayRef> ImportingJobs, + ArrayRef> Jobs, ArrayRef ScannedJobIndices, SmallVectorImpl &&ModuleDepGraphsForScannedJobs) { llvm::DenseSet AlreadySeen; - for (auto &&[ImportingJob, ModuleDepsGraph] : - llvm::zip_equal(llvm::make_pointee_range(ImportingJobs), - ModuleDepGraphsForScannedJobs)) { + for (auto &&[ScanIndex, ModuleDepsGraph] : + llvm::enumerate(ModuleDepGraphsForScannedJobs)) { + const auto &ImportingJob = *Jobs[ScannedJobIndices[ScanIndex]]; + for (auto &MD : ModuleDepsGraph) { const auto Inserted = AlreadySeen.insert(MD.ID).second; if (!Inserted) @@ -1276,31 +1274,6 @@ static void createClangModuleJobsAndNodes( } } -/// Installs the command lines produced by the dependency scan into -/// \p ScannedJobs. -static void -installScanCommandLines(Compilation &C, - MutableArrayRef> ScannedJobs, - ArrayRef 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 @@ -1533,17 +1506,16 @@ 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, /*ImportingJobs*/ ScannedJobs, + Graph, C, Jobs, ScanResult.ScannedJobIndices, std::move(ScanResult.ModuleDepGraphsForScannedJobs)); - installScanCommandLines(C, ScannedJobs, ScanResult.InputDepsForScannedJobs); - createNodesForScannedJobs(Graph, std::move(ScannedJobs), - std::move(ScanResult.InputDepsForScannedJobs)); - + createNodesForScannedJobs( + Graph, takeJobsAtIndices(Jobs, ScanResult.ScannedJobIndices), + std::move(ScanResult.InputDepsForScannedJobs)); createRegularEdges(Graph); + pruneUnusedStdlibModuleJobs(Graph, UnusedStdlibModuleJobNodes); + if (!createModuleDependencyEdges(Graph, Diags)) return; createAndConnectRoot(Graph); @@ -1552,14 +1524,12 @@ void driver::modules::runModulesDriver( if (!Diags.isLastDiagnosticIgnored()) llvm::WriteGraph(llvm::errs(), &Graph); + // TODO: Install all updated command-lines produced by the dependency scan. // TODO: Fix-up command-lines for named module imports. - llvm::ReversePostOrderTraversal TopologicallySortedNodes( - &Graph); - assert(isa(*TopologicallySortedNodes.begin()) && - "First node in topological order must be the root!"); - auto TopologicallySortedJobNodes = llvm::map_range( - llvm::drop_begin(TopologicallySortedNodes), llvm::CastTo); - for (auto *JN : TopologicallySortedJobNodes) - C.addCommand(std::move(JN->Job)); + // TODO: Sort the graph topologically before adding jobs back to the + // Compilation being built. + for (auto *N : Graph) + if (auto *JN = dyn_cast(N)) + C.addCommand(std::move(JN->Job)); } diff --git a/clang/test/Driver/modules-driver-clang-modules-only.cpp b/clang/test/Driver/modules-driver-clang-modules-only.cpp deleted file mode 100644 index 37a803f9b6fc0..0000000000000 --- a/clang/test/Driver/modules-driver-clang-modules-only.cpp +++ /dev/null @@ -1,127 +0,0 @@ -// 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(); -} diff --git a/clang/test/Driver/modules-driver-manifest-input-args.cpp b/clang/test/Driver/modules-driver-manifest-input-args.cpp index 726fe03e27840..99765a1943faf 100644 --- a/clang/test/Driver/modules-driver-manifest-input-args.cpp +++ b/clang/test/Driver/modules-driver-manifest-input-args.cpp @@ -26,15 +26,17 @@ // RUN: %t/main.cpp \ // RUN: -### 2>&1 \ // RUN: | sed 's:\\\\\?:/:g' \ -// RUN: | FileCheck %s -DPREFIX=%/t +// RUN: | FileCheck %s -check-prefix=MAIN-CC1 -check-prefix=STDLIB-MOD-CC1 -DPREFIX=%/t -// 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/" +// 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/" -// 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/" +// 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/" //--- main.cpp import std;