https://github.com/qiongsiwu updated https://github.com/llvm/llvm-project/pull/128446
>From c8eda8b9192cf4bdad4121063336beeb14cbe689 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu <qiongsi...@apple.com> Date: Sun, 23 Feb 2025 16:47:18 -0800 Subject: [PATCH 1/7] Initial commit --- .../DependencyScanning/ModuleDepCollector.cpp | 17 +++++++++++++- clang/test/ClangScanDeps/modules-debug-dir.c | 22 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 clang/test/ClangScanDeps/modules-debug-dir.c diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index 1c5f4c4b50ab6..8a94535d3806c 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -492,8 +492,23 @@ static std::string getModuleContextHash(const ModuleDeps &MD, auto &FSOpts = const_cast<FileSystemOptions &>(CI.getFileSystemOpts()); if (CWD && !IgnoreCWD) HashBuilder.add(*CWD); - else + else { FSOpts.WorkingDir.clear(); + auto &CGOpts = const_cast<CodeGenOptions &>(CI.getCodeGenOpts()); + if (CGOpts.DwarfVersion && CWD) { + // It is necessary to explicitly set the DebugCompilationDir + // to a common directory (e.g. root) if IgnoreCWD is true. + // When IgnoreCWD is true, the module's content should not depend + // on the current working directory. However, if dwarf information + // is needed (when CGOpts.DwarfVersion is non-zero), and if + // CGOpts.DebugCompilationDir is not explicitly set, + // the current working directory will be automatically embedded + // in the dwarf information in the pcm, contradicting the assumption + // that it is safe to ignore the CWD. Thus in such cases, + // CGOpts.DebugCompilationDir is explicitly set to a common directory. + CGOpts.DebugCompilationDir = llvm::sys::path::root_path(*CWD); + } + } // Hash the BuildInvocation without any input files. SmallString<0> ArgVec; diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c new file mode 100644 index 0000000000000..580f72205e68f --- /dev/null +++ b/clang/test/ClangScanDeps/modules-debug-dir.c @@ -0,0 +1,22 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json +// RUN: clang-scan-deps -compilation-database %t/cdb.json -format \ +// RUN: experimental-full > %t/result.json + +//--- cdb.json.in +[{ + "directory": "DIR", + "command": "clang -g -fdebug-info-for-profiling DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/include/ -o DIR/tu.o", + "file": "DIR/tu.c" +}] + +//--- include/module.modulemap +module mod { + header "mod.h" +} + +//--- include/mod.h + +//--- tu.c +#include "mod.h" >From 4b29c19ebc360ec80a8c16e1ac485bbac78a9062 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu <qiongsi...@apple.com> Date: Mon, 24 Feb 2025 09:34:56 -0800 Subject: [PATCH 2/7] Fix the test. --- clang/test/ClangScanDeps/modules-debug-dir.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c index 580f72205e68f..1ac518985471d 100644 --- a/clang/test/ClangScanDeps/modules-debug-dir.c +++ b/clang/test/ClangScanDeps/modules-debug-dir.c @@ -3,11 +3,12 @@ // RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json // RUN: clang-scan-deps -compilation-database %t/cdb.json -format \ // RUN: experimental-full > %t/result.json +// RUN: cat %t/result.json | FileCheck %s //--- cdb.json.in [{ "directory": "DIR", - "command": "clang -g -fdebug-info-for-profiling DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/include/ -o DIR/tu.o", + "command": "clang -c -g -gmodules DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/include/ -fdebug-compilation-dir=DIR -o DIR/tu.o", "file": "DIR/tu.c" }] @@ -20,3 +21,10 @@ module mod { //--- tu.c #include "mod.h" + +// Check the -fdebug-compilation-dir used for the module is the root +// directory when current working directory optimization is in effect. +// CHECK: "modules": [ +// CHECK: "command-line": [ +// CHECK: "-fdebug-compilation-dir={{\/|C:|\/\/net}}", +// CHECK: "translation-units": [ >From 5c53c84d33ad4ceec4e79dc36638e827607709ca Mon Sep 17 00:00:00 2001 From: Qiongsi Wu <qiongsi...@apple.com> Date: Mon, 24 Feb 2025 10:34:35 -0800 Subject: [PATCH 3/7] Try to fix the test on windows. --- clang/test/ClangScanDeps/modules-debug-dir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c index 1ac518985471d..87f1bc141c73a 100644 --- a/clang/test/ClangScanDeps/modules-debug-dir.c +++ b/clang/test/ClangScanDeps/modules-debug-dir.c @@ -8,7 +8,7 @@ //--- cdb.json.in [{ "directory": "DIR", - "command": "clang -c -g -gmodules DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/include/ -fdebug-compilation-dir=DIR -o DIR/tu.o", + "command": "clang -target x86_64-apple-darwin -c -g -gmodules DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/include/ -fdebug-compilation-dir=DIR -o DIR/tu.o", "file": "DIR/tu.c" }] @@ -26,5 +26,5 @@ module mod { // directory when current working directory optimization is in effect. // CHECK: "modules": [ // CHECK: "command-line": [ -// CHECK: "-fdebug-compilation-dir={{\/|C:|\/\/net}}", +// CHECK: "-fdebug-compilation-dir=/", // CHECK: "translation-units": [ >From 7021180bec1533009a84104023770ef18e7056ea Mon Sep 17 00:00:00 2001 From: Qiongsi Wu <qiongsi...@apple.com> Date: Tue, 25 Feb 2025 12:23:49 -0800 Subject: [PATCH 4/7] Address code review and try fixing the test on windows. --- .../DependencyScanning/ModuleDepCollector.h | 2 +- .../DependencyScanning/ModuleDepCollector.cpp | 58 ++++++++++++------- clang/test/ClangScanDeps/modules-debug-dir.c | 4 +- 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h index c2071a6eadedd..b41efa254342e 100644 --- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h +++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h @@ -317,7 +317,7 @@ class ModuleDepCollector final : public DependencyCollector { /// Compute the context hash for \p Deps, and create the mapping /// \c ModuleDepsByID[Deps.ID] = &Deps. - void associateWithContextHash(const CowCompilerInvocation &CI, + void associateWithContextHash(const CowCompilerInvocation &CI, bool IgnoreCWD, ModuleDeps &Deps); }; diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index 8a94535d3806c..5f805998f61bd 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -129,6 +129,26 @@ static void optimizeDiagnosticOpts(DiagnosticOptions &Opts, Opts.Remarks.clear(); } +static void optimizeCWD(FileSystemOptions &FSOpts, CodeGenOptions &CGOpts, + const std::string &CWD) { + FSOpts.WorkingDir.clear(); + if (CGOpts.DwarfVersion) { + // It is necessary to explicitly set the DebugCompilationDir + // to a common directory (e.g. root) if IgnoreCWD is true. + // When IgnoreCWD is true, the module's content should not + // depend on the current working directory. However, if dwarf + // information is needed (when CGOpts.DwarfVersion is + // non-zero), then CGOpts.DebugCompilationDir must be + // populated, because otherwise the current working directory + // will be automatically embedded in the dwarf information in + // the pcm, contradicting the assumption that it is safe to + // ignore the CWD. Thus in such cases, + // CGOpts.DebugCompilationDir is explicitly set to a common + // directory. + CGOpts.DebugCompilationDir = llvm::sys::path::root_path(CWD); + } +} + static std::vector<std::string> splitString(std::string S, char Separator) { SmallVector<StringRef> Segments; StringRef(S).split(Segments, Separator, /*MaxSplit=*/-1, /*KeepEmpty=*/false); @@ -489,26 +509,8 @@ static std::string getModuleContextHash(const ModuleDeps &MD, HashBuilder.add(getClangFullRepositoryVersion()); HashBuilder.add(serialization::VERSION_MAJOR, serialization::VERSION_MINOR); llvm::ErrorOr<std::string> CWD = VFS.getCurrentWorkingDirectory(); - auto &FSOpts = const_cast<FileSystemOptions &>(CI.getFileSystemOpts()); if (CWD && !IgnoreCWD) HashBuilder.add(*CWD); - else { - FSOpts.WorkingDir.clear(); - auto &CGOpts = const_cast<CodeGenOptions &>(CI.getCodeGenOpts()); - if (CGOpts.DwarfVersion && CWD) { - // It is necessary to explicitly set the DebugCompilationDir - // to a common directory (e.g. root) if IgnoreCWD is true. - // When IgnoreCWD is true, the module's content should not depend - // on the current working directory. However, if dwarf information - // is needed (when CGOpts.DwarfVersion is non-zero), and if - // CGOpts.DebugCompilationDir is not explicitly set, - // the current working directory will be automatically embedded - // in the dwarf information in the pcm, contradicting the assumption - // that it is safe to ignore the CWD. Thus in such cases, - // CGOpts.DebugCompilationDir is explicitly set to a common directory. - CGOpts.DebugCompilationDir = llvm::sys::path::root_path(*CWD); - } - } // Hash the BuildInvocation without any input files. SmallString<0> ArgVec; @@ -539,9 +541,7 @@ static std::string getModuleContextHash(const ModuleDeps &MD, } void ModuleDepCollector::associateWithContextHash( - const CowCompilerInvocation &CI, ModuleDeps &Deps) { - bool IgnoreCWD = any(OptimizeArgs & ScanningOptimizations::IgnoreCWD) && - isSafeToIgnoreCWD(CI); + const CowCompilerInvocation &CI, bool IgnoreCWD, ModuleDeps &Deps) { Deps.ID.ContextHash = getModuleContextHash(Deps, CI, EagerLoadModules, IgnoreCWD, ScanInstance.getVirtualFileSystem()); @@ -741,6 +741,7 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) { MD.ModuleMapFileDeps.emplace_back(*ResolvedFilenameAsRequested); }); + bool IgnoreCWD = false; CowCompilerInvocation CI = MDC.getInvocationAdjustedForModuleBuildWithoutOutputs( MD, [&](CowCompilerInvocation &BuildInvocation) { @@ -750,13 +751,26 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) { *MDC.ScanInstance.getASTReader(), *MF, MDC.PrebuiltModuleVFSMap, MDC.OptimizeArgs); + if (any(MDC.OptimizeArgs & ScanningOptimizations::SystemWarnings)) optimizeDiagnosticOpts( BuildInvocation.getMutDiagnosticOpts(), BuildInvocation.getFrontendOpts().IsSystemModule); + + IgnoreCWD = + any(MDC.OptimizeArgs & ScanningOptimizations::IgnoreCWD) && + isSafeToIgnoreCWD(BuildInvocation); + if (IgnoreCWD) { + llvm::ErrorOr<std::string> CWD = + MDC.ScanInstance.getVirtualFileSystem() + .getCurrentWorkingDirectory(); + if (CWD) + optimizeCWD(BuildInvocation.getMutFileSystemOpts(), + BuildInvocation.getMutCodeGenOpts(), *CWD); + } }); - MDC.associateWithContextHash(CI, MD); + MDC.associateWithContextHash(CI, IgnoreCWD, MD); // Finish the compiler invocation. Requires dependencies and the context hash. MDC.addOutputPaths(CI, MD); diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c index 87f1bc141c73a..4edd30c0d91b7 100644 --- a/clang/test/ClangScanDeps/modules-debug-dir.c +++ b/clang/test/ClangScanDeps/modules-debug-dir.c @@ -8,7 +8,7 @@ //--- cdb.json.in [{ "directory": "DIR", - "command": "clang -target x86_64-apple-darwin -c -g -gmodules DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/include/ -fdebug-compilation-dir=DIR -o DIR/tu.o", + "command": "clang -c -g -gmodules DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/include/ -fdebug-compilation-dir=DIR -o DIR/tu.o", "file": "DIR/tu.c" }] @@ -26,5 +26,5 @@ module mod { // directory when current working directory optimization is in effect. // CHECK: "modules": [ // CHECK: "command-line": [ -// CHECK: "-fdebug-compilation-dir=/", +// CHECK: "-fdebug-compilation-dir={{\/|.*:}}", // CHECK: "translation-units": [ >From 75b4ff6b36a305891bb707748c82357bf9ecc48f Mon Sep 17 00:00:00 2001 From: Qiongsi Wu <qiongsi...@apple.com> Date: Tue, 25 Feb 2025 15:56:17 -0800 Subject: [PATCH 5/7] Address code review and try fixing the test on windows. --- .../DependencyScanning/ModuleDepCollector.cpp | 21 ++++++++++++------- clang/test/ClangScanDeps/modules-debug-dir.c | 6 ++++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index 5f805998f61bd..7da3ec71f2da4 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -129,10 +129,9 @@ static void optimizeDiagnosticOpts(DiagnosticOptions &Opts, Opts.Remarks.clear(); } -static void optimizeCWD(FileSystemOptions &FSOpts, CodeGenOptions &CGOpts, - const std::string &CWD) { - FSOpts.WorkingDir.clear(); - if (CGOpts.DwarfVersion) { +static void optimizeCWD(CowCompilerInvocation &BuildInvocation, StringRef CWD) { + BuildInvocation.getMutFileSystemOpts().WorkingDir.clear(); + if (BuildInvocation.getCodeGenOpts().DwarfVersion) { // It is necessary to explicitly set the DebugCompilationDir // to a common directory (e.g. root) if IgnoreCWD is true. // When IgnoreCWD is true, the module's content should not @@ -145,7 +144,16 @@ static void optimizeCWD(FileSystemOptions &FSOpts, CodeGenOptions &CGOpts, // ignore the CWD. Thus in such cases, // CGOpts.DebugCompilationDir is explicitly set to a common // directory. - CGOpts.DebugCompilationDir = llvm::sys::path::root_path(CWD); + // FIXME: It is still excessive to create a copy of + // CodeGenOpts for each module. Since we do not modify the + // CodeGenOpts otherwise per module, the following code + // ends up generating identical CodeGenOpts for each module + // with DebugCompilationDir pointing to the root directory. + // We can optimize this away by creating a _single_ copy of + // CodeGenOpts whose DebugCompilationDir points to the root + // directory and reuse it across modules. + BuildInvocation.getMutCodeGenOpts().DebugCompilationDir = + llvm::sys::path::root_path(CWD); } } @@ -765,8 +773,7 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) { MDC.ScanInstance.getVirtualFileSystem() .getCurrentWorkingDirectory(); if (CWD) - optimizeCWD(BuildInvocation.getMutFileSystemOpts(), - BuildInvocation.getMutCodeGenOpts(), *CWD); + optimizeCWD(BuildInvocation, *CWD); } }); diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c index 4edd30c0d91b7..fadec1ad52e35 100644 --- a/clang/test/ClangScanDeps/modules-debug-dir.c +++ b/clang/test/ClangScanDeps/modules-debug-dir.c @@ -1,9 +1,11 @@ +// REQUIRES: shell + // RUN: rm -rf %t // RUN: split-file %s %t // RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json // RUN: clang-scan-deps -compilation-database %t/cdb.json -format \ // RUN: experimental-full > %t/result.json -// RUN: cat %t/result.json | FileCheck %s +// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s //--- cdb.json.in [{ @@ -26,5 +28,5 @@ module mod { // directory when current working directory optimization is in effect. // CHECK: "modules": [ // CHECK: "command-line": [ -// CHECK: "-fdebug-compilation-dir={{\/|.*:}}", +// CHECK: "-fdebug-compilation-dir={{\/|.*:(\\)?}}", // CHECK: "translation-units": [ >From a9274ea7ab4af0d5f3201e9db168f6ed165fa4fd Mon Sep 17 00:00:00 2001 From: Qiongsi Wu <qiongsi...@apple.com> Date: Tue, 25 Feb 2025 19:45:04 -0800 Subject: [PATCH 6/7] Try one more time to fix the test on windows. --- clang/test/ClangScanDeps/modules-debug-dir.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c index fadec1ad52e35..7d0f529d6259c 100644 --- a/clang/test/ClangScanDeps/modules-debug-dir.c +++ b/clang/test/ClangScanDeps/modules-debug-dir.c @@ -1,5 +1,3 @@ -// REQUIRES: shell - // RUN: rm -rf %t // RUN: split-file %s %t // RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json >From 6b4435c2dd20ca0c85a3ec7fbf9fda570449ee60 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu <qiongsi...@apple.com> Date: Tue, 25 Feb 2025 23:06:27 -0800 Subject: [PATCH 7/7] Revert "Try one more time to fix the test on windows." This reverts commit a9274ea7ab4af0d5f3201e9db168f6ed165fa4fd. --- clang/test/ClangScanDeps/modules-debug-dir.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c index 7d0f529d6259c..fadec1ad52e35 100644 --- a/clang/test/ClangScanDeps/modules-debug-dir.c +++ b/clang/test/ClangScanDeps/modules-debug-dir.c @@ -1,3 +1,5 @@ +// REQUIRES: shell + // RUN: rm -rf %t // RUN: split-file %s %t // RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits