Skip to content

Commit 57c160f

Browse files
[DependencyScan] Correct setup clang VFS for dependency scanning
Currently, dependency scanner is not reporting the redirecting files that are baked inside swift-frontend for platform support. This causes dependency scanner returns virtual path for those files, and swift-driver/build-system will not be able to correct validate the files on incremental build, causing incremental build to be almost clean builds. This behavior issue is caused by the dependency scanning file system layer inside clang dependency scanner that caches stats. If the redirecting files are created underneath the layer, the real path is lost. This fixes the issue by moving the redirecting files above the caching layer using `-ivfsoverlay` option. In addition to that, this commit also unifies how clang importer and clang dependency scanner initiate the VFS, making the logic much simpler.
1 parent edfe1a1 commit 57c160f

File tree

7 files changed

+139
-126
lines changed

7 files changed

+139
-126
lines changed

include/swift/ClangImporter/ClangImporter.h

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ class ClangImporter final : public ClangModuleLoader {
167167
Implementation &Impl;
168168

169169
bool requiresBuiltinHeadersInSystemModules = false;
170+
bool needSystemVFSOverlay = false;
171+
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> baseFS = nullptr;
170172

171173
ClangImporter(ASTContext &ctx, DependencyTracker *tracker,
172174
DWARFImporterDelegate *dwarfImporterDelegate);
@@ -205,6 +207,10 @@ class ClangImporter final : public ClangModuleLoader {
205207
DWARFImporterDelegate *dwarfImporterDelegate = nullptr,
206208
bool ignoreFileMapping = false);
207209

210+
static llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
211+
computeClangImporterFileSystem(const ASTContext &ctx, ClangImporter &importer,
212+
bool suppressDiagnostics = false);
213+
208214
std::vector<std::string>
209215
getClangDriverArguments(ASTContext &ctx, bool ignoreClangTarget = false);
210216

@@ -531,6 +537,10 @@ class ClangImporter final : public ClangModuleLoader {
531537

532538
std::string getClangModuleHash() const;
533539

540+
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> getBaseFileSystem() const {
541+
return baseFS;
542+
}
543+
534544
/// Get clang import creation cc1 args for swift explicit module build.
535545
std::vector<std::string> getSwiftExplicitModuleDirectCC1Args() const;
536546

@@ -887,9 +897,8 @@ struct ClangInvocationFileMapping {
887897
/// Mapping from a file name to an existing file path.
888898
SmallVector<std::pair<std::string, std::string>, 2> redirectedFiles;
889899

890-
/// Mapping from a file name to a string of characters that represents the
891-
/// contents of the file.
892-
SmallVector<std::pair<std::string, std::string>, 1> overridenFiles;
900+
/// MemoryBuffer that represents the file name and content to overload.
901+
SmallVector<std::unique_ptr<llvm::MemoryBuffer>, 1> overridenFiles;
893902

894903
bool requiresBuiltinHeadersInSystemModules;
895904
};
@@ -924,15 +933,6 @@ ClangInvocationFileMapping getClangInvocationFileMapping(
924933
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> vfs = nullptr,
925934
bool suppressDiagnostic = false);
926935

927-
/// Apply the given file mapping to the specified 'fileSystem', used
928-
/// primarily to inject modulemaps on platforms with non-modularized
929-
/// platform libraries.
930-
ClangInvocationFileMapping applyClangInvocationMapping(
931-
const ClangInvocationFileMappingContext &ctx,
932-
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> baseVFS,
933-
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> &fileSystem,
934-
bool suppressDiagnostics = false);
935-
936936
/// Information used to compute the access level of inherited C++ members.
937937
class ClangInheritanceInfo {
938938
/// The cumulative inheritance access specifier, that is used to compute the

lib/ClangImporter/ClangImporter.cpp

Lines changed: 92 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -828,10 +828,9 @@ getEmbedBitcodeInvocationArguments(std::vector<std::string> &invocationArgStrs,
828828
});
829829
}
830830

831-
void
832-
importer::addCommonInvocationArguments(
833-
std::vector<std::string> &invocationArgStrs,
834-
ASTContext &ctx, bool requiresBuiltinHeadersInSystemModules,
831+
void importer::addCommonInvocationArguments(
832+
std::vector<std::string> &invocationArgStrs, ASTContext &ctx,
833+
bool requiresBuiltinHeadersInSystemModules, bool needSystemVFSOverlay,
835834
bool ignoreClangTarget) {
836835
using ImporterImpl = ClangImporter::Implementation;
837836
llvm::Triple triple = ctx.LangOpts.Target;
@@ -1003,6 +1002,12 @@ importer::addCommonInvocationArguments(
10031002
invocationArgStrs.push_back("-Xclang");
10041003
invocationArgStrs.push_back("-fbuiltin-headers-in-system-modules");
10051004
}
1005+
1006+
if (needSystemVFSOverlay) {
1007+
invocationArgStrs.push_back("-ivfsoverlay");
1008+
invocationArgStrs.push_back(
1009+
ClangImporter::Implementation::clangSystemVFSOverlayName);
1010+
}
10061011
}
10071012

10081013
bool ClangImporter::canReadPCH(StringRef PCHFilename) {
@@ -1151,6 +1156,70 @@ ClangImporter::getOrCreatePCH(const ClangImporterOptions &ImporterOptions,
11511156
return PCHFilename.value();
11521157
}
11531158

1159+
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
1160+
ClangImporter::computeClangImporterFileSystem(const ASTContext &ctx,
1161+
ClangImporter &importer,
1162+
bool suppressDiagnostics) {
1163+
assert(!ctx.CASOpts.HasImmutableFileSystem && "Use immutable file system");
1164+
1165+
ClangInvocationFileMapping fileMapping =
1166+
getClangInvocationFileMapping(ctx, nullptr, suppressDiagnostics);
1167+
1168+
auto importerOpts = ctx.ClangImporterOpts;
1169+
importer.requiresBuiltinHeadersInSystemModules =
1170+
fileMapping.requiresBuiltinHeadersInSystemModules;
1171+
1172+
auto fileSystem = llvm::vfs::getRealFileSystem();
1173+
if (!fileMapping.redirectedFiles.empty()) {
1174+
if (importerOpts.DumpClangDiagnostics) {
1175+
llvm::errs() << "clang importer redirected file mappings:\n";
1176+
for (const auto &mapping : fileMapping.redirectedFiles) {
1177+
llvm::errs() << " mapping real file '" << mapping.second
1178+
<< "' to virtual file '" << mapping.first << "'\n";
1179+
}
1180+
llvm::errs() << "\n";
1181+
}
1182+
// Create a vfs overlay map for all redirects.
1183+
llvm::vfs::YAMLVFSWriter vfsWriter;
1184+
vfsWriter.setUseExternalNames(true);
1185+
for (const auto &mapping : fileMapping.redirectedFiles)
1186+
vfsWriter.addFileMapping(mapping.first, mapping.second);
1187+
1188+
std::string vfsYAML;
1189+
llvm::raw_string_ostream os(vfsYAML);
1190+
vfsWriter.write(os);
1191+
1192+
llvm::SmallString<256> overlayPath(ctx.SearchPathOpts.RuntimeResourcePath);
1193+
llvm::sys::path::append(overlayPath,
1194+
Implementation::clangSystemVFSOverlayName);
1195+
fileMapping.overridenFiles.push_back(
1196+
llvm::MemoryBuffer::getMemBufferCopy(vfsYAML, overlayPath));
1197+
1198+
importer.needSystemVFSOverlay = true;
1199+
}
1200+
1201+
if (!fileMapping.overridenFiles.empty()) {
1202+
llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> overridenVFS =
1203+
new llvm::vfs::InMemoryFileSystem();
1204+
for (auto &file : fileMapping.overridenFiles) {
1205+
if (importerOpts.DumpClangDiagnostics) {
1206+
llvm::errs() << "clang importer overriding file '"
1207+
<< file->getBufferIdentifier()
1208+
<< "' with the following contents:\n";
1209+
llvm::errs() << file->getBuffer() << "\n";
1210+
}
1211+
// Note MemoryBuffer is guaranteeed to be null-terminated.
1212+
overridenVFS->addFile(file->getBufferIdentifier(), 0, std::move(file));
1213+
}
1214+
llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> overlayVFS =
1215+
new llvm::vfs::OverlayFileSystem(fileSystem);
1216+
fileSystem = overlayVFS;
1217+
overlayVFS->pushOverlay(overridenVFS);
1218+
}
1219+
1220+
return fileSystem;
1221+
}
1222+
11541223
std::vector<std::string>
11551224
ClangImporter::getClangDriverArguments(ASTContext &ctx, bool ignoreClangTarget) {
11561225
assert(!ctx.ClangImporterOpts.DirectClangCC1ModuleBuild &&
@@ -1169,7 +1238,8 @@ ClangImporter::getClangDriverArguments(ASTContext &ctx, bool ignoreClangTarget)
11691238
break;
11701239
}
11711240
addCommonInvocationArguments(invocationArgStrs, ctx,
1172-
requiresBuiltinHeadersInSystemModules, ignoreClangTarget);
1241+
requiresBuiltinHeadersInSystemModules,
1242+
needSystemVFSOverlay, ignoreClangTarget);
11731243
return invocationArgStrs;
11741244
}
11751245

@@ -1235,6 +1305,10 @@ std::optional<std::vector<std::string>> ClangImporter::getClangCC1Arguments(
12351305
CI->getTargetOpts().DarwinTargetVariantTriple = ctx.LangOpts.TargetVariant->str();
12361306
}
12371307

1308+
if (needSystemVFSOverlay)
1309+
CI->getHeaderSearchOpts().AddVFSOverlayFile(
1310+
Implementation::clangSystemVFSOverlayName);
1311+
12381312
// Forward the index store path. That information is not passed to scanner
12391313
// and it is cached invariant so we don't want to re-scan if that changed.
12401314
CI->getFrontendOpts().IndexStorePath = ctx.ClangImporterOpts.IndexStorePath;
@@ -1348,18 +1422,21 @@ std::unique_ptr<ClangImporter> ClangImporter::create(
13481422
}
13491423
}
13501424

1351-
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS =
1352-
ctx.SourceMgr.getFileSystem();
1425+
// Configure ClangImporter file system. There are two situations:
1426+
// * If caching is used, thus file system is immutable, the one immutable file
1427+
// system is shared between swift frontend and ClangImporter.
1428+
// * Otherwise, ClangImporter file system is configure from scratch from
1429+
// RealFileSystem using ivfsoverlay options.
1430+
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> vfs =
1431+
ctx.CASOpts.HasImmutableFileSystem
1432+
? ctx.SourceMgr.getFileSystem()
1433+
: computeClangImporterFileSystem(ctx, *importer, ignoreFileMapping);
13531434

1354-
ClangInvocationFileMapping fileMapping =
1355-
applyClangInvocationMapping(ctx, nullptr, VFS, ignoreFileMapping);
1356-
1357-
importer->requiresBuiltinHeadersInSystemModules =
1358-
fileMapping.requiresBuiltinHeadersInSystemModules;
1435+
importer->baseFS = vfs;
13591436

13601437
// Create a new Clang compiler invocation.
13611438
{
1362-
if (auto ClangArgs = importer->getClangCC1Arguments(ctx, VFS))
1439+
if (auto ClangArgs = importer->getClangCC1Arguments(ctx, vfs))
13631440
importer->Impl.ClangArgs = *ClangArgs;
13641441
else
13651442
return nullptr;
@@ -1373,7 +1450,7 @@ std::unique_ptr<ClangImporter> ClangImporter::create(
13731450
llvm::errs() << "'\n";
13741451
}
13751452
importer->Impl.Invocation = createClangInvocation(
1376-
importer.get(), importerOpts, VFS, importer->Impl.ClangArgs);
1453+
importer.get(), importerOpts, vfs, importer->Impl.ClangArgs);
13771454
if (!importer->Impl.Invocation)
13781455
return nullptr;
13791456
}
@@ -1424,7 +1501,7 @@ std::unique_ptr<ClangImporter> ClangImporter::create(
14241501
auto actualDiagClient = std::make_unique<ClangDiagnosticConsumer>(
14251502
importer->Impl, instance.getDiagnosticOpts(),
14261503
importerOpts.DumpClangDiagnostics);
1427-
instance.createVirtualFileSystem(std::move(VFS), actualDiagClient.get());
1504+
instance.createVirtualFileSystem(std::move(vfs), actualDiagClient.get());
14281505
instance.createFileManager();
14291506
instance.createDiagnostics(actualDiagClient.release());
14301507

lib/ClangImporter/ClangIncludePaths.cpp

Lines changed: 18 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -450,26 +450,26 @@ static void getLibStdCxxFileMapping(
450450
includeHeaderInModuleMap(additionalFile);
451451
os << contents.substr(headerInjectionPoint);
452452

453-
fileMapping.overridenFiles.push_back(
454-
{std::string(injectedModuleMapPath), std::move(os.str())});
453+
fileMapping.overridenFiles.push_back(llvm::MemoryBuffer::getMemBufferCopy(
454+
additionalHeaderDirectives, injectedModuleMapPath));
455455
}
456456

457457
namespace {
458-
std::string
459-
GetPlatformAuxiliaryFile(StringRef Platform, StringRef File,
460-
const SearchPathOptions &Options) {
458+
std::string GetPlatformAuxiliaryFile(StringRef Platform, StringRef File,
459+
llvm::vfs::FileSystem &VFS,
460+
const SearchPathOptions &Options) {
461461
StringRef SDKPath = Options.getSDKPath();
462462
if (!SDKPath.empty()) {
463463
llvm::SmallString<261> path{SDKPath};
464464
llvm::sys::path::append(path, "usr", "share", File);
465-
if (llvm::sys::fs::exists(path))
465+
if (VFS.exists(path))
466466
return path.str().str();
467467
}
468468

469469
if (!Options.RuntimeResourcePath.empty()) {
470470
llvm::SmallString<261> path{Options.RuntimeResourcePath};
471471
llvm::sys::path::append(path, Platform, File);
472-
if (llvm::sys::fs::exists(path))
472+
if (VFS.exists(path))
473473
return path.str().str();
474474
}
475475

@@ -514,7 +514,7 @@ void GetWindowsFileMappings(
514514
llvm::sys::path::append(WinSDKInjection, "module.modulemap");
515515

516516
AuxiliaryFile = GetPlatformAuxiliaryFile("windows", "winsdk_um.modulemap",
517-
SearchPathOpts);
517+
VFS, SearchPathOpts);
518518
if (!AuxiliaryFile.empty())
519519
fileMapping.redirectedFiles.emplace_back(std::string(WinSDKInjection),
520520
AuxiliaryFile);
@@ -523,7 +523,7 @@ void GetWindowsFileMappings(
523523
llvm::sys::path::remove_filename(WinSDKInjection);
524524
llvm::sys::path::append(WinSDKInjection, "shared", "module.modulemap");
525525
AuxiliaryFile = GetPlatformAuxiliaryFile(
526-
"windows", "winsdk_shared.modulemap", SearchPathOpts);
526+
"windows", "winsdk_shared.modulemap", VFS, SearchPathOpts);
527527
if (!AuxiliaryFile.empty())
528528
fileMapping.redirectedFiles.emplace_back(std::string(WinSDKInjection),
529529
AuxiliaryFile);
@@ -540,8 +540,8 @@ void GetWindowsFileMappings(
540540
llvm::sys::path::append(UCRTInjection, "Include", UCRTSDK.Version, "ucrt");
541541
llvm::sys::path::append(UCRTInjection, "module.modulemap");
542542

543-
AuxiliaryFile =
544-
GetPlatformAuxiliaryFile("windows", "ucrt.modulemap", SearchPathOpts);
543+
AuxiliaryFile = GetPlatformAuxiliaryFile("windows", "ucrt.modulemap", VFS,
544+
SearchPathOpts);
545545
if (!AuxiliaryFile.empty()) {
546546
// The ucrt module map has the C standard library headers all together.
547547
// That leads to module cycles with the clang _Builtin_ modules. e.g.
@@ -578,18 +578,16 @@ void GetWindowsFileMappings(
578578
llvm::sys::path::append(VCToolsInjection, "include");
579579

580580
llvm::sys::path::append(VCToolsInjection, "module.modulemap");
581-
AuxiliaryFile =
582-
GetPlatformAuxiliaryFile("windows", "vcruntime.modulemap",
583-
SearchPathOpts);
581+
AuxiliaryFile = GetPlatformAuxiliaryFile("windows", "vcruntime.modulemap",
582+
VFS, SearchPathOpts);
584583
if (!AuxiliaryFile.empty())
585584
fileMapping.redirectedFiles.emplace_back(std::string(VCToolsInjection),
586585
AuxiliaryFile);
587586

588587
llvm::sys::path::remove_filename(VCToolsInjection);
589588
llvm::sys::path::append(VCToolsInjection, "vcruntime.apinotes");
590-
AuxiliaryFile =
591-
GetPlatformAuxiliaryFile("windows", "vcruntime.apinotes",
592-
SearchPathOpts);
589+
AuxiliaryFile = GetPlatformAuxiliaryFile("windows", "vcruntime.apinotes",
590+
VFS, SearchPathOpts);
593591
if (!AuxiliaryFile.empty())
594592
fileMapping.redirectedFiles.emplace_back(std::string(VCToolsInjection),
595593
AuxiliaryFile);
@@ -610,9 +608,9 @@ void GetWindowsFileMappings(
610608
for (const char * const header : kInjectedHeaders) {
611609
llvm::sys::path::remove_filename(VCToolsInjection);
612610
llvm::sys::path::append(VCToolsInjection, header);
613-
if (!llvm::sys::fs::exists(VCToolsInjection))
614-
fileMapping.overridenFiles.emplace_back(std::string{VCToolsInjection},
615-
"");
611+
if (!VFS.exists(VCToolsInjection))
612+
fileMapping.overridenFiles.emplace_back(
613+
llvm::MemoryBuffer::getMemBufferCopy("", VCToolsInjection));
616614
}
617615
}
618616
}
@@ -695,49 +693,3 @@ ClangInvocationFileMapping swift::getClangInvocationFileMapping(
695693
result.requiresBuiltinHeadersInSystemModules);
696694
return result;
697695
}
698-
699-
ClangInvocationFileMapping swift::applyClangInvocationMapping(
700-
const ClangInvocationFileMappingContext &ctx,
701-
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> baseVFS,
702-
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> &fileSystem,
703-
bool suppressDiagnostics) {
704-
if (ctx.CASOpts.HasImmutableFileSystem)
705-
return ClangInvocationFileMapping();
706-
707-
ClangInvocationFileMapping fileMapping =
708-
getClangInvocationFileMapping(ctx, baseVFS, suppressDiagnostics);
709-
710-
auto importerOpts = ctx.ClangImporterOpts;
711-
// Wrap Swift's FS to allow Clang to override the working directory
712-
fileSystem = llvm::vfs::RedirectingFileSystem::create(
713-
fileMapping.redirectedFiles, true, *fileSystem);
714-
if (importerOpts.DumpClangDiagnostics) {
715-
llvm::errs() << "clang importer redirected file mappings:\n";
716-
for (const auto &mapping : fileMapping.redirectedFiles) {
717-
llvm::errs() << " mapping real file '" << mapping.second
718-
<< "' to virtual file '" << mapping.first << "'\n";
719-
}
720-
llvm::errs() << "\n";
721-
}
722-
723-
if (!fileMapping.overridenFiles.empty()) {
724-
llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> overridenVFS =
725-
new llvm::vfs::InMemoryFileSystem();
726-
for (const auto &file : fileMapping.overridenFiles) {
727-
if (importerOpts.DumpClangDiagnostics) {
728-
llvm::errs() << "clang importer overriding file '" << file.first
729-
<< "' with the following contents:\n";
730-
llvm::errs() << file.second << "\n";
731-
}
732-
// Note MemoryBuffer is guaranteeed to be null-terminated.
733-
overridenVFS->addFile(file.first, 0,
734-
llvm::MemoryBuffer::getMemBufferCopy(file.second));
735-
}
736-
llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> overlayVFS =
737-
new llvm::vfs::OverlayFileSystem(fileSystem);
738-
fileSystem = overlayVFS;
739-
overlayVFS->pushOverlay(overridenVFS);
740-
}
741-
742-
return fileMapping;
743-
}

0 commit comments

Comments
 (0)