Author: Jan Svoboda Date: 2025-09-10T15:56:04-07:00 New Revision: 55bef46146f05e1911fcb98715716d914efd518c
URL: https://github.com/llvm/llvm-project/commit/55bef46146f05e1911fcb98715716d914efd518c DIFF: https://github.com/llvm/llvm-project/commit/55bef46146f05e1911fcb98715716d914efd518c.diff LOG: Reland "[clang] Delay normalization of `-fmodules-cache-path` (#150123)" This reverts commit 613caa909c78f707e88960723c6a98364656a926, essentially reapplying 4a4bddec3571d78c8073fa45b57bbabc8796d13d after moving `normalizeModuleCachePath` from clangFrontend to clangLex. This PR is part of an effort to remove file system usage from the command line parsing code. The reason for that is that it's impossible to do file system access correctly without a configured VFS, and the VFS can only be configured after the command line is parsed. I don't want to intertwine command line parsing and VFS configuration, so I decided to perform the file system access after the command line is parsed and the VFS is configured - ideally right before the file system entity is used for the first time. This patch delays normalization of the module cache path until `CompilerInstance` is asked for the cache path in the current compilation context. Added: clang/test/Modules/modules-cache-path-canonicalization-output.c Modified: clang/include/clang/Driver/Options.td clang/include/clang/Lex/HeaderSearch.h clang/lib/Frontend/CompilerInstance.cpp clang/lib/Frontend/CompilerInvocation.cpp clang/lib/Lex/HeaderSearch.cpp clang/lib/Serialization/ASTWriter.cpp clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 337460a2a0a1c..a7c514e809aa9 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3281,7 +3281,8 @@ defm declspec : BoolOption<"f", "declspec", def fmodules_cache_path : Joined<["-"], "fmodules-cache-path=">, Group<i_Group>, Flags<[]>, Visibility<[ClangOption, CC1Option]>, MetaVarName<"<directory>">, - HelpText<"Specify the module cache path">; + HelpText<"Specify the module cache path">, + MarshallingInfoString<HeaderSearchOpts<"ModuleCachePath">>; def fmodules_user_build_path : Separate<["-"], "fmodules-user-build-path">, Group<i_Group>, Flags<[]>, Visibility<[ClangOption, CC1Option]>, MetaVarName<"<directory>">, diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h index 2e0c8bec8bd8c..850aea41c4c3b 100644 --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -986,6 +986,9 @@ void ApplyHeaderSearchOptions(HeaderSearch &HS, const LangOptions &Lang, const llvm::Triple &triple); +void normalizeModuleCachePath(FileManager &FileMgr, StringRef Path, + SmallVectorImpl<char> &NormalizedPath); + } // namespace clang #endif // LLVM_CLANG_LEX_HEADERSEARCH_H diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 8d1e9d6cdf2bb..31a8d75fec4bd 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -554,9 +554,16 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) { } std::string CompilerInstance::getSpecificModuleCachePath(StringRef ModuleHash) { + assert(FileMgr && "Specific module cache path requires a FileManager"); + + if (getHeaderSearchOpts().ModuleCachePath.empty()) + return ""; + // Set up the module path, including the hash for the module-creation options. - SmallString<256> SpecificModuleCache(getHeaderSearchOpts().ModuleCachePath); - if (!SpecificModuleCache.empty() && !getHeaderSearchOpts().DisableModuleHash) + SmallString<256> SpecificModuleCache; + normalizeModuleCachePath(*FileMgr, getHeaderSearchOpts().ModuleCachePath, + SpecificModuleCache); + if (!getHeaderSearchOpts().DisableModuleHash) llvm::sys::path::append(SpecificModuleCache, ModuleHash); return std::string(SpecificModuleCache); } diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 8411d00cc7812..931766db4b0c8 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -3315,9 +3315,6 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts, if (Opts.UseLibcxx) GenerateArg(Consumer, OPT_stdlib_EQ, "libc++"); - if (!Opts.ModuleCachePath.empty()) - GenerateArg(Consumer, OPT_fmodules_cache_path, Opts.ModuleCachePath); - for (const auto &File : Opts.PrebuiltModuleFiles) GenerateArg(Consumer, OPT_fmodule_file, File.first + "=" + File.second); @@ -3420,8 +3417,7 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts, } static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args, - DiagnosticsEngine &Diags, - const std::string &WorkingDir) { + DiagnosticsEngine &Diags) { unsigned NumErrorsBefore = Diags.getNumErrors(); HeaderSearchOptions *HeaderSearchOpts = &Opts; @@ -3434,17 +3430,6 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args, if (const Arg *A = Args.getLastArg(OPT_stdlib_EQ)) Opts.UseLibcxx = (strcmp(A->getValue(), "libc++") == 0); - // Canonicalize -fmodules-cache-path before storing it. - SmallString<128> P(Args.getLastArgValue(OPT_fmodules_cache_path)); - if (!(P.empty() || llvm::sys::path::is_absolute(P))) { - if (WorkingDir.empty()) - llvm::sys::fs::make_absolute(P); - else - llvm::sys::fs::make_absolute(WorkingDir, P); - } - llvm::sys::path::remove_dots(P); - Opts.ModuleCachePath = std::string(P); - // Only the -fmodule-file=<name>=<file> form. for (const auto *A : Args.filtered(OPT_fmodule_file)) { StringRef Val = A->getValue(); @@ -5021,8 +5006,7 @@ bool CompilerInvocation::CreateFromArgsImpl( InputKind DashX = Res.getFrontendOpts().DashX; ParseTargetArgs(Res.getTargetOpts(), Args, Diags); llvm::Triple T(Res.getTargetOpts().Triple); - ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args, Diags, - Res.getFileSystemOpts().WorkingDir); + ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args, Diags); if (Res.getFrontendOpts().GenReducedBMI || Res.getFrontendOpts().ProgramAction == frontend::GenerateReducedModuleInterface || diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index ea2391f6812cf..f28a74f5d0ae5 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -2183,3 +2183,10 @@ std::string HeaderSearch::suggestPathToFileForDiagnostics( } return path::convert_to_slash(Filename); } + +void clang::normalizeModuleCachePath(FileManager &FileMgr, StringRef Path, + SmallVectorImpl<char> &NormalizedPath) { + NormalizedPath.assign(Path.begin(), Path.end()); + FileMgr.makeAbsolutePath(NormalizedPath); + llvm::sys::path::remove_dots(NormalizedPath); +} diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 8e219e54bf251..326e5e34e1ad7 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -57,6 +57,7 @@ #include "clang/Basic/TargetInfo.h" #include "clang/Basic/TargetOptions.h" #include "clang/Basic/Version.h" +#include "clang/Frontend/CompilerInstance.h" #include "clang/Lex/HeaderSearch.h" #include "clang/Lex/HeaderSearchOptions.h" #include "clang/Lex/MacroInfo.h" @@ -1710,9 +1711,13 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { const HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts(); + SmallString<256> HSOpts_ModuleCachePath; + normalizeModuleCachePath(PP.getFileManager(), HSOpts.ModuleCachePath, + HSOpts_ModuleCachePath); + AddString(HSOpts.Sysroot, Record); AddString(HSOpts.ResourceDir, Record); - AddString(HSOpts.ModuleCachePath, Record); + AddString(HSOpts_ModuleCachePath, Record); AddString(HSOpts.ModuleUserBuildPath, Record); Record.push_back(HSOpts.DisableModuleHash); Record.push_back(HSOpts.ImplicitModuleMaps); diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index f32747ddf8183..0855e6dec6158 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -449,9 +449,10 @@ class DependencyScanningAction { // Use the dependency scanning optimized file system if requested to do so. if (DepFS) { - StringRef ModulesCachePath = - ScanInstance.getHeaderSearchOpts().ModuleCachePath; - + SmallString<256> ModulesCachePath; + normalizeModuleCachePath( + *FileMgr, ScanInstance.getHeaderSearchOpts().ModuleCachePath, + ModulesCachePath); DepFS->resetBypassedPathPrefix(); if (!ModulesCachePath.empty()) DepFS->setBypassedPathPrefix(ModulesCachePath); diff --git a/clang/test/Modules/modules-cache-path-canonicalization-output.c b/clang/test/Modules/modules-cache-path-canonicalization-output.c new file mode 100644 index 0000000000000..ad71b69e5299e --- /dev/null +++ b/clang/test/Modules/modules-cache-path-canonicalization-output.c @@ -0,0 +1,18 @@ +// This checks that implicitly-built modules produce identical PCM +// files regardless of the spelling of the same module cache path. + +// RUN: rm -rf %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fsyntax-only %t/tu.c \ +// RUN: -fmodules-cache-path=%t/cache -fdisable-module-hash +// RUN: mv %t/cache/M.pcm %t/M.pcm +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fsyntax-only %t/tu.c \ +// RUN: -fmodules-cache-path=%t/./cache -fdisable-module-hash +// RUN: diff %t/./cache/M.pcm %t/M.pcm + +//--- tu.c +#include "M.h" +//--- M.h +//--- module.modulemap +module M { header "M.h" } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
