Skip to content

Commit f732280

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 a78458b commit f732280

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
@@ -3399,7 +3399,8 @@ defm declspec : BoolOption<"f", "declspec",
33993399
def fmodules_cache_path : Joined<["-"], "fmodules-cache-path=">, Group<i_Group>,
34003400
Flags<[]>, Visibility<[ClangOption, CC1Option]>,
34013401
MetaVarName<"<directory>">,
3402-
HelpText<"Specify the module cache path">;
3402+
HelpText<"Specify the module cache path">,
3403+
MarshallingInfoString<HeaderSearchOpts<"ModuleCachePath">>;
34033404
def fmodules_user_build_path : Separate<["-"], "fmodules-user-build-path">, Group<i_Group>,
34043405
Flags<[]>, Visibility<[ClangOption, CC1Option]>,
34053406
MetaVarName<"<directory>">,

clang/include/clang/Lex/HeaderSearch.h

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

989+
void normalizeModuleCachePath(FileManager &FileMgr, StringRef Path,
990+
SmallVectorImpl<char> &NormalizedPath);
991+
989992
} // namespace clang
990993

991994
#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
@@ -2183,3 +2183,10 @@ std::string HeaderSearch::suggestPathToFileForDiagnostics(
21832183
}
21842184
return path::convert_to_slash(Filename);
21852185
}
2186+
2187+
void clang::normalizeModuleCachePath(FileManager &FileMgr, StringRef Path,
2188+
SmallVectorImpl<char> &NormalizedPath) {
2189+
NormalizedPath.assign(Path.begin(), Path.end());
2190+
FileMgr.makeAbsolutePath(NormalizedPath);
2191+
llvm::sys::path::remove_dots(NormalizedPath);
2192+
}

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
@@ -609,9 +609,10 @@ class DependencyScanningAction : public tooling::ToolAction {
609609

610610
// Use the dependency scanning optimized file system if requested to do so.
611611
if (DepFS) {
612-
StringRef ModulesCachePath =
613-
ScanInstance.getHeaderSearchOpts().ModuleCachePath;
614-
612+
SmallString<256> ModulesCachePath;
613+
normalizeModuleCachePath(
614+
*FileMgr, ScanInstance.getHeaderSearchOpts().ModuleCachePath,
615+
ModulesCachePath);
615616
DepFS->resetBypassedPathPrefix();
616617
if (!ModulesCachePath.empty())
617618
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)