Skip to content

Commit 55bef46

Browse files
committed
Reland "[clang] Delay normalization of -fmodules-cache-path (#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.
1 parent d7318eb commit 55bef46

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
@@ -3281,7 +3281,8 @@ defm declspec : BoolOption<"f", "declspec",
32813281
def fmodules_cache_path : Joined<["-"], "fmodules-cache-path=">, Group<i_Group>,
32823282
Flags<[]>, Visibility<[ClangOption, CC1Option]>,
32833283
MetaVarName<"<directory>">,
3284-
HelpText<"Specify the module cache path">;
3284+
HelpText<"Specify the module cache path">,
3285+
MarshallingInfoString<HeaderSearchOpts<"ModuleCachePath">>;
32853286
def fmodules_user_build_path : Separate<["-"], "fmodules-user-build-path">, Group<i_Group>,
32863287
Flags<[]>, Visibility<[ClangOption, CC1Option]>,
32873288
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
@@ -554,9 +554,16 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
554554
}
555555

556556
std::string CompilerInstance::getSpecificModuleCachePath(StringRef ModuleHash) {
557+
assert(FileMgr && "Specific module cache path requires a FileManager");
558+
559+
if (getHeaderSearchOpts().ModuleCachePath.empty())
560+
return "";
561+
557562
// Set up the module path, including the hash for the module-creation options.
558-
SmallString<256> SpecificModuleCache(getHeaderSearchOpts().ModuleCachePath);
559-
if (!SpecificModuleCache.empty() && !getHeaderSearchOpts().DisableModuleHash)
563+
SmallString<256> SpecificModuleCache;
564+
normalizeModuleCachePath(*FileMgr, getHeaderSearchOpts().ModuleCachePath,
565+
SpecificModuleCache);
566+
if (!getHeaderSearchOpts().DisableModuleHash)
560567
llvm::sys::path::append(SpecificModuleCache, ModuleHash);
561568
return std::string(SpecificModuleCache);
562569
}

clang/lib/Frontend/CompilerInvocation.cpp

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

3318-
if (!Opts.ModuleCachePath.empty())
3319-
GenerateArg(Consumer, OPT_fmodules_cache_path, Opts.ModuleCachePath);
3320-
33213318
for (const auto &File : Opts.PrebuiltModuleFiles)
33223319
GenerateArg(Consumer, OPT_fmodule_file, File.first + "=" + File.second);
33233320

@@ -3420,8 +3417,7 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
34203417
}
34213418

34223419
static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
3423-
DiagnosticsEngine &Diags,
3424-
const std::string &WorkingDir) {
3420+
DiagnosticsEngine &Diags) {
34253421
unsigned NumErrorsBefore = Diags.getNumErrors();
34263422

34273423
HeaderSearchOptions *HeaderSearchOpts = &Opts;
@@ -3434,17 +3430,6 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
34343430
if (const Arg *A = Args.getLastArg(OPT_stdlib_EQ))
34353431
Opts.UseLibcxx = (strcmp(A->getValue(), "libc++") == 0);
34363432

3437-
// Canonicalize -fmodules-cache-path before storing it.
3438-
SmallString<128> P(Args.getLastArgValue(OPT_fmodules_cache_path));
3439-
if (!(P.empty() || llvm::sys::path::is_absolute(P))) {
3440-
if (WorkingDir.empty())
3441-
llvm::sys::fs::make_absolute(P);
3442-
else
3443-
llvm::sys::fs::make_absolute(WorkingDir, P);
3444-
}
3445-
llvm::sys::path::remove_dots(P);
3446-
Opts.ModuleCachePath = std::string(P);
3447-
34483433
// Only the -fmodule-file=<name>=<file> form.
34493434
for (const auto *A : Args.filtered(OPT_fmodule_file)) {
34503435
StringRef Val = A->getValue();
@@ -5021,8 +5006,7 @@ bool CompilerInvocation::CreateFromArgsImpl(
50215006
InputKind DashX = Res.getFrontendOpts().DashX;
50225007
ParseTargetArgs(Res.getTargetOpts(), Args, Diags);
50235008
llvm::Triple T(Res.getTargetOpts().Triple);
5024-
ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args, Diags,
5025-
Res.getFileSystemOpts().WorkingDir);
5009+
ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args, Diags);
50265010
if (Res.getFrontendOpts().GenReducedBMI ||
50275011
Res.getFrontendOpts().ProgramAction ==
50285012
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"
@@ -1710,9 +1711,13 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) {
17101711
const HeaderSearchOptions &HSOpts =
17111712
PP.getHeaderSearchInfo().getHeaderSearchOpts();
17121713

1714+
SmallString<256> HSOpts_ModuleCachePath;
1715+
normalizeModuleCachePath(PP.getFileManager(), HSOpts.ModuleCachePath,
1716+
HSOpts_ModuleCachePath);
1717+
17131718
AddString(HSOpts.Sysroot, Record);
17141719
AddString(HSOpts.ResourceDir, Record);
1715-
AddString(HSOpts.ModuleCachePath, Record);
1720+
AddString(HSOpts_ModuleCachePath, Record);
17161721
AddString(HSOpts.ModuleUserBuildPath, Record);
17171722
Record.push_back(HSOpts.DisableModuleHash);
17181723
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
@@ -449,9 +449,10 @@ class DependencyScanningAction {
449449

450450
// Use the dependency scanning optimized file system if requested to do so.
451451
if (DepFS) {
452-
StringRef ModulesCachePath =
453-
ScanInstance.getHeaderSearchOpts().ModuleCachePath;
454-
452+
SmallString<256> ModulesCachePath;
453+
normalizeModuleCachePath(
454+
*FileMgr, ScanInstance.getHeaderSearchOpts().ModuleCachePath,
455+
ModulesCachePath);
455456
DepFS->resetBypassedPathPrefix();
456457
if (!ModulesCachePath.empty())
457458
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)