Skip to content

Commit a7299db

Browse files
jansvoboda11qiongsiwu
authored andcommitted
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 9e64628 commit a7299db

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
@@ -3586,9 +3586,6 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
35863586
if (Opts.UseLibcxx)
35873587
GenerateArg(Consumer, OPT_stdlib_EQ, "libc++");
35883588

3589-
if (!Opts.ModuleCachePath.empty())
3590-
GenerateArg(Consumer, OPT_fmodules_cache_path, Opts.ModuleCachePath);
3591-
35923589
for (const auto &File : Opts.PrebuiltModuleFiles)
35933590
GenerateArg(Consumer, OPT_fmodule_file, File.first + "=" + File.second);
35943591

@@ -3691,8 +3688,7 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
36913688
}
36923689

36933690
static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
3694-
DiagnosticsEngine &Diags,
3695-
const std::string &WorkingDir) {
3691+
DiagnosticsEngine &Diags) {
36963692
unsigned NumErrorsBefore = Diags.getNumErrors();
36973693

36983694
HeaderSearchOptions *HeaderSearchOpts = &Opts;
@@ -3705,17 +3701,6 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
37053701
if (const Arg *A = Args.getLastArg(OPT_stdlib_EQ))
37063702
Opts.UseLibcxx = (strcmp(A->getValue(), "libc++") == 0);
37073703

3708-
// Canonicalize -fmodules-cache-path before storing it.
3709-
SmallString<128> P(Args.getLastArgValue(OPT_fmodules_cache_path));
3710-
if (!(P.empty() || llvm::sys::path::is_absolute(P))) {
3711-
if (WorkingDir.empty())
3712-
llvm::sys::fs::make_absolute(P);
3713-
else
3714-
llvm::sys::fs::make_absolute(WorkingDir, P);
3715-
}
3716-
llvm::sys::path::remove_dots(P);
3717-
Opts.ModuleCachePath = std::string(P);
3718-
37193704
// Only the -fmodule-file=<name>=<file> form.
37203705
for (const auto *A : Args.filtered(OPT_fmodule_file)) {
37213706
StringRef Val = A->getValue();
@@ -5582,8 +5567,7 @@ bool CompilerInvocation::CreateFromArgsImpl(
55825567
InputKind DashX = Res.getFrontendOpts().DashX;
55835568
ParseTargetArgs(Res.getTargetOpts(), Args, Diags);
55845569
llvm::Triple T(Res.getTargetOpts().Triple);
5585-
ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args, Diags,
5586-
Res.getFileSystemOpts().WorkingDir);
5570+
ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args, Diags);
55875571
if (Res.getFrontendOpts().GenReducedBMI ||
55885572
Res.getFrontendOpts().ProgramAction ==
55895573
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"
@@ -1725,9 +1726,13 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) {
17251726
const HeaderSearchOptions &HSOpts =
17261727
PP.getHeaderSearchInfo().getHeaderSearchOpts();
17271728

1729+
SmallString<256> HSOpts_ModuleCachePath;
1730+
normalizeModuleCachePath(PP.getFileManager(), HSOpts.ModuleCachePath,
1731+
HSOpts_ModuleCachePath);
1732+
17281733
AddString(HSOpts.Sysroot, Record);
17291734
AddString(HSOpts.ResourceDir, Record);
1730-
AddString(HSOpts.ModuleCachePath, Record);
1735+
AddString(HSOpts_ModuleCachePath, Record);
17311736
AddString(HSOpts.ModuleUserBuildPath, Record);
17321737
Record.push_back(HSOpts.DisableModuleHash);
17331738
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)