Skip to content

Commit 0e4075e

Browse files
committed
Reland "[clang] Delay normalization of -fmodules-cache-path (llvm#150123)"
This reverts commit 613caa9, essentially reapplying 4a4bdde 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. (cherry picked from commit 55bef46)
1 parent da07f87 commit 0e4075e

File tree

8 files changed

+51
-25
lines changed

8 files changed

+51
-25
lines changed

clang/include/clang/Driver/Options.td

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3413,7 +3413,8 @@ defm declspec : BoolOption<"f", "declspec",
34133413
def fmodules_cache_path : Joined<["-"], "fmodules-cache-path=">, Group<i_Group>,
34143414
Flags<[]>, Visibility<[ClangOption, CC1Option]>,
34153415
MetaVarName<"<directory>">,
3416-
HelpText<"Specify the module cache path">;
3416+
HelpText<"Specify the module cache path">,
3417+
MarshallingInfoString<HeaderSearchOpts<"ModuleCachePath">>;
34173418
def fmodules_user_build_path : Separate<["-"], "fmodules-user-build-path">, Group<i_Group>,
34183419
Flags<[]>, Visibility<[ClangOption, CC1Option]>,
34193420
MetaVarName<"<directory>">,

clang/include/clang/Lex/HeaderSearch.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,9 @@ void ApplyHeaderSearchOptions(HeaderSearch &HS,
966966
const LangOptions &Lang,
967967
const llvm::Triple &triple);
968968

969+
void normalizeModuleCachePath(FileManager &FileMgr, StringRef Path,
970+
SmallVectorImpl<char> &NormalizedPath);
971+
969972
} // namespace clang
970973

971974
#endif // LLVM_CLANG_LEX_HEADERSEARCH_H

clang/lib/Frontend/CompilerInstance.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -559,9 +559,16 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
559559
}
560560

561561
std::string CompilerInstance::getSpecificModuleCachePath(StringRef ModuleHash) {
562+
assert(FileMgr && "Specific module cache path requires a FileManager");
563+
564+
if (getHeaderSearchOpts().ModuleCachePath.empty())
565+
return "";
566+
562567
// Set up the module path, including the hash for the module-creation options.
563-
SmallString<256> SpecificModuleCache(getHeaderSearchOpts().ModuleCachePath);
564-
if (!SpecificModuleCache.empty() && !getHeaderSearchOpts().DisableModuleHash)
568+
SmallString<256> SpecificModuleCache;
569+
normalizeModuleCachePath(*FileMgr, getHeaderSearchOpts().ModuleCachePath,
570+
SpecificModuleCache);
571+
if (!getHeaderSearchOpts().DisableModuleHash)
565572
llvm::sys::path::append(SpecificModuleCache, ModuleHash);
566573
return std::string(SpecificModuleCache);
567574
}

clang/lib/Frontend/CompilerInvocation.cpp

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3577,9 +3577,6 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
35773577
if (Opts.UseLibcxx)
35783578
GenerateArg(Consumer, OPT_stdlib_EQ, "libc++");
35793579

3580-
if (!Opts.ModuleCachePath.empty())
3581-
GenerateArg(Consumer, OPT_fmodules_cache_path, Opts.ModuleCachePath);
3582-
35833580
for (const auto &File : Opts.PrebuiltModuleFiles)
35843581
GenerateArg(Consumer, OPT_fmodule_file, File.first + "=" + File.second);
35853582

@@ -3682,8 +3679,7 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
36823679
}
36833680

36843681
static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
3685-
DiagnosticsEngine &Diags,
3686-
const std::string &WorkingDir) {
3682+
DiagnosticsEngine &Diags) {
36873683
unsigned NumErrorsBefore = Diags.getNumErrors();
36883684

36893685
HeaderSearchOptions *HeaderSearchOpts = &Opts;
@@ -3696,17 +3692,6 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
36963692
if (const Arg *A = Args.getLastArg(OPT_stdlib_EQ))
36973693
Opts.UseLibcxx = (strcmp(A->getValue(), "libc++") == 0);
36983694

3699-
// Canonicalize -fmodules-cache-path before storing it.
3700-
SmallString<128> P(Args.getLastArgValue(OPT_fmodules_cache_path));
3701-
if (!(P.empty() || llvm::sys::path::is_absolute(P))) {
3702-
if (WorkingDir.empty())
3703-
llvm::sys::fs::make_absolute(P);
3704-
else
3705-
llvm::sys::fs::make_absolute(WorkingDir, P);
3706-
}
3707-
llvm::sys::path::remove_dots(P);
3708-
Opts.ModuleCachePath = std::string(P);
3709-
37103695
// Only the -fmodule-file=<name>=<file> form.
37113696
for (const auto *A : Args.filtered(OPT_fmodule_file)) {
37123697
StringRef Val = A->getValue();
@@ -5561,8 +5546,7 @@ bool CompilerInvocation::CreateFromArgsImpl(
55615546
InputKind DashX = Res.getFrontendOpts().DashX;
55625547
ParseTargetArgs(Res.getTargetOpts(), Args, Diags);
55635548
llvm::Triple T(Res.getTargetOpts().Triple);
5564-
ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args, Diags,
5565-
Res.getFileSystemOpts().WorkingDir);
5549+
ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args, Diags);
55665550
if (Res.getFrontendOpts().GenReducedBMI ||
55675551
Res.getFrontendOpts().ProgramAction ==
55685552
frontend::GenerateReducedModuleInterface ||

clang/lib/Lex/HeaderSearch.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2095,3 +2095,10 @@ std::string HeaderSearch::suggestPathToFileForDiagnostics(
20952095
}
20962096
return path::convert_to_slash(Filename);
20972097
}
2098+
2099+
void clang::normalizeModuleCachePath(FileManager &FileMgr, StringRef Path,
2100+
SmallVectorImpl<char> &NormalizedPath) {
2101+
NormalizedPath.assign(Path.begin(), Path.end());
2102+
FileMgr.makeAbsolutePath(NormalizedPath);
2103+
llvm::sys::path::remove_dots(NormalizedPath);
2104+
}

clang/lib/Serialization/ASTWriter.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
#include "clang/Basic/TargetInfo.h"
5858
#include "clang/Basic/TargetOptions.h"
5959
#include "clang/Basic/Version.h"
60+
#include "clang/Frontend/CompilerInstance.h"
6061
#include "clang/Lex/HeaderSearch.h"
6162
#include "clang/Lex/HeaderSearchOptions.h"
6263
#include "clang/Lex/MacroInfo.h"
@@ -1731,9 +1732,13 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) {
17311732
const HeaderSearchOptions &HSOpts =
17321733
PP.getHeaderSearchInfo().getHeaderSearchOpts();
17331734

1735+
SmallString<256> HSOpts_ModuleCachePath;
1736+
normalizeModuleCachePath(PP.getFileManager(), HSOpts.ModuleCachePath,
1737+
HSOpts_ModuleCachePath);
1738+
17341739
AddString(HSOpts.Sysroot, Record);
17351740
AddString(HSOpts.ResourceDir, Record);
1736-
AddString(HSOpts.ModuleCachePath, Record);
1741+
AddString(HSOpts_ModuleCachePath, Record);
17371742
AddString(HSOpts.ModuleUserBuildPath, Record);
17381743
Record.push_back(HSOpts.DisableModuleHash);
17391744
Record.push_back(HSOpts.ImplicitModuleMaps);

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -606,9 +606,10 @@ class DependencyScanningAction {
606606

607607
// Use the dependency scanning optimized file system if requested to do so.
608608
if (DepFS) {
609-
StringRef ModulesCachePath =
610-
ScanInstance.getHeaderSearchOpts().ModuleCachePath;
611-
609+
SmallString<256> ModulesCachePath;
610+
normalizeModuleCachePath(
611+
*FileMgr, ScanInstance.getHeaderSearchOpts().ModuleCachePath,
612+
ModulesCachePath);
612613
DepFS->resetBypassedPathPrefix();
613614
if (!ModulesCachePath.empty())
614615
DepFS->setBypassedPathPrefix(ModulesCachePath);
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// This checks that implicitly-built modules produce identical PCM
2+
// files regardless of the spelling of the same module cache path.
3+
4+
// RUN: rm -rf %t
5+
// RUN: split-file %s %t
6+
7+
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fsyntax-only %t/tu.c \
8+
// RUN: -fmodules-cache-path=%t/cache -fdisable-module-hash
9+
// RUN: mv %t/cache/M.pcm %t/M.pcm
10+
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fsyntax-only %t/tu.c \
11+
// RUN: -fmodules-cache-path=%t/./cache -fdisable-module-hash
12+
// RUN: diff %t/./cache/M.pcm %t/M.pcm
13+
14+
//--- tu.c
15+
#include "M.h"
16+
//--- M.h
17+
//--- module.modulemap
18+
module M { header "M.h" }

0 commit comments

Comments
 (0)