Skip to content

Commit 31c7be3

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 31c7be3

File tree

8 files changed

+154
-129
lines changed

8 files changed

+154
-129
lines changed

include/swift/ClangImporter/ClangImporter.h

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

169169
bool requiresBuiltinHeadersInSystemModules = false;
170+
bool needSystemVFSOverlay = false;
170171

171172
ClangImporter(ASTContext &ctx, DependencyTracker *tracker,
172173
DWARFImporterDelegate *dwarfImporterDelegate);
@@ -205,6 +206,12 @@ class ClangImporter final : public ClangModuleLoader {
205206
DWARFImporterDelegate *dwarfImporterDelegate = nullptr,
206207
bool ignoreFileMapping = false);
207208

209+
static llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
210+
computeClangImporterFileSystem(
211+
const ASTContext &ctx,
212+
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> baseFS,
213+
ClangImporter *importer = nullptr, bool suppressDiagnostics = false);
214+
208215
std::vector<std::string>
209216
getClangDriverArguments(ASTContext &ctx, bool ignoreClangTarget = false);
210217

@@ -887,9 +894,8 @@ struct ClangInvocationFileMapping {
887894
/// Mapping from a file name to an existing file path.
888895
SmallVector<std::pair<std::string, std::string>, 2> redirectedFiles;
889896

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;
897+
/// MemoryBuffer that represents the file name and content to overload.
898+
SmallVector<std::unique_ptr<llvm::MemoryBuffer>, 1> overridenFiles;
893899

894900
bool requiresBuiltinHeadersInSystemModules;
895901
};
@@ -924,15 +930,6 @@ ClangInvocationFileMapping getClangInvocationFileMapping(
924930
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> vfs = nullptr,
925931
bool suppressDiagnostic = false);
926932

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-
936933
/// Information used to compute the access level of inherited C++ members.
937934
class ClangInheritanceInfo {
938935
/// The cumulative inheritance access specifier, that is used to compute the

lib/ClangImporter/ClangImporter.cpp

Lines changed: 106 additions & 16 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,14 @@ 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+
llvm::SmallString<256> overlayPath(ctx.SearchPathOpts.RuntimeResourcePath);
1009+
llvm::sys::path::append(
1010+
overlayPath, ClangImporter::Implementation::clangSystemVFSOverlayName);
1011+
invocationArgStrs.push_back(overlayPath.str().str());
1012+
}
10061013
}
10071014

10081015
bool ClangImporter::canReadPCH(StringRef PCHFilename) {
@@ -1151,6 +1158,84 @@ ClangImporter::getOrCreatePCH(const ClangImporterOptions &ImporterOptions,
11511158
return PCHFilename.value();
11521159
}
11531160

1161+
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
1162+
ClangImporter::computeClangImporterFileSystem(
1163+
const ASTContext &ctx,
1164+
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> baseFS,
1165+
ClangImporter *importer, bool suppressDiagnostics) {
1166+
// Configure ClangImporter file system. There are two situations:
1167+
// * If caching is used, thus file system is immutable, the one immutable file
1168+
// system is shared between swift frontend and ClangImporter.
1169+
// * Otherwise, ClangImporter file system is configure from scratch from
1170+
// VFS in SourceMgr using ivfsoverlay options.
1171+
if (ctx.CASOpts.HasImmutableFileSystem)
1172+
return baseFS;
1173+
1174+
ClangInvocationFileMapping fileMapping =
1175+
getClangInvocationFileMapping(ctx, nullptr, suppressDiagnostics);
1176+
1177+
fileMapping.redirectedFiles.push_back({
1178+
"/tmp",
1179+
"/my-temp",
1180+
});
1181+
1182+
auto importerOpts = ctx.ClangImporterOpts;
1183+
if (importer)
1184+
importer->requiresBuiltinHeadersInSystemModules =
1185+
fileMapping.requiresBuiltinHeadersInSystemModules;
1186+
1187+
auto fileSystem = ctx.SourceMgr.getFileSystem();
1188+
if (!fileMapping.redirectedFiles.empty()) {
1189+
if (importerOpts.DumpClangDiagnostics) {
1190+
llvm::errs() << "clang importer redirected file mappings:\n";
1191+
for (const auto &mapping : fileMapping.redirectedFiles) {
1192+
llvm::errs() << " mapping real file '" << mapping.second
1193+
<< "' to virtual file '" << mapping.first << "'\n";
1194+
}
1195+
llvm::errs() << "\n";
1196+
}
1197+
// Create a vfs overlay map for all redirects.
1198+
llvm::vfs::YAMLVFSWriter vfsWriter;
1199+
vfsWriter.setUseExternalNames(true);
1200+
for (const auto &mapping : fileMapping.redirectedFiles)
1201+
vfsWriter.addFileMapping(mapping.first, mapping.second);
1202+
1203+
std::string vfsYAML;
1204+
llvm::raw_string_ostream os(vfsYAML);
1205+
vfsWriter.write(os);
1206+
1207+
llvm::SmallString<256> overlayPath(ctx.SearchPathOpts.RuntimeResourcePath);
1208+
llvm::sys::path::append(overlayPath,
1209+
Implementation::clangSystemVFSOverlayName);
1210+
fileMapping.overridenFiles.push_back(
1211+
llvm::MemoryBuffer::getMemBufferCopy(vfsYAML, overlayPath));
1212+
1213+
if (importer)
1214+
importer->needSystemVFSOverlay = true;
1215+
}
1216+
1217+
if (!fileMapping.overridenFiles.empty()) {
1218+
auto overridenVFS =
1219+
llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
1220+
for (auto &file : fileMapping.overridenFiles) {
1221+
if (importerOpts.DumpClangDiagnostics) {
1222+
llvm::errs() << "clang importer overriding file '"
1223+
<< file->getBufferIdentifier()
1224+
<< "' with the following contents:\n";
1225+
llvm::errs() << file->getBuffer() << "\n";
1226+
}
1227+
// Note MemoryBuffer is guaranteeed to be null-terminated.
1228+
overridenVFS->addFile(file->getBufferIdentifier(), 0, std::move(file));
1229+
}
1230+
auto overlayVFS =
1231+
llvm::makeIntrusiveRefCnt<llvm::vfs::OverlayFileSystem>(fileSystem);
1232+
overlayVFS->pushOverlay(std::move(overridenVFS));
1233+
fileSystem = std::move(overlayVFS);
1234+
}
1235+
1236+
return fileSystem;
1237+
}
1238+
11541239
std::vector<std::string>
11551240
ClangImporter::getClangDriverArguments(ASTContext &ctx, bool ignoreClangTarget) {
11561241
assert(!ctx.ClangImporterOpts.DirectClangCC1ModuleBuild &&
@@ -1169,7 +1254,8 @@ ClangImporter::getClangDriverArguments(ASTContext &ctx, bool ignoreClangTarget)
11691254
break;
11701255
}
11711256
addCommonInvocationArguments(invocationArgStrs, ctx,
1172-
requiresBuiltinHeadersInSystemModules, ignoreClangTarget);
1257+
requiresBuiltinHeadersInSystemModules,
1258+
needSystemVFSOverlay, ignoreClangTarget);
11731259
return invocationArgStrs;
11741260
}
11751261

@@ -1235,6 +1321,15 @@ std::optional<std::vector<std::string>> ClangImporter::getClangCC1Arguments(
12351321
CI->getTargetOpts().DarwinTargetVariantTriple = ctx.LangOpts.TargetVariant->str();
12361322
}
12371323

1324+
if (needSystemVFSOverlay) {
1325+
llvm::SmallString<256> overlayPath(
1326+
ctx.SearchPathOpts.RuntimeResourcePath);
1327+
llvm::sys::path::append(overlayPath,
1328+
Implementation::clangSystemVFSOverlayName);
1329+
CI->getHeaderSearchOpts().AddVFSOverlayFile(overlayPath);
1330+
1331+
}
1332+
12381333
// Forward the index store path. That information is not passed to scanner
12391334
// and it is cached invariant so we don't want to re-scan if that changed.
12401335
CI->getFrontendOpts().IndexStorePath = ctx.ClangImporterOpts.IndexStorePath;
@@ -1348,18 +1443,13 @@ std::unique_ptr<ClangImporter> ClangImporter::create(
13481443
}
13491444
}
13501445

1351-
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS =
1352-
ctx.SourceMgr.getFileSystem();
1353-
1354-
ClangInvocationFileMapping fileMapping =
1355-
applyClangInvocationMapping(ctx, nullptr, VFS, ignoreFileMapping);
1356-
1357-
importer->requiresBuiltinHeadersInSystemModules =
1358-
fileMapping.requiresBuiltinHeadersInSystemModules;
1446+
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> vfs =
1447+
computeClangImporterFileSystem(ctx, ctx.SourceMgr.getFileSystem(),
1448+
importer.get(), ignoreFileMapping);
13591449

13601450
// Create a new Clang compiler invocation.
13611451
{
1362-
if (auto ClangArgs = importer->getClangCC1Arguments(ctx, VFS))
1452+
if (auto ClangArgs = importer->getClangCC1Arguments(ctx, vfs))
13631453
importer->Impl.ClangArgs = *ClangArgs;
13641454
else
13651455
return nullptr;
@@ -1373,7 +1463,7 @@ std::unique_ptr<ClangImporter> ClangImporter::create(
13731463
llvm::errs() << "'\n";
13741464
}
13751465
importer->Impl.Invocation = createClangInvocation(
1376-
importer.get(), importerOpts, VFS, importer->Impl.ClangArgs);
1466+
importer.get(), importerOpts, vfs, importer->Impl.ClangArgs);
13771467
if (!importer->Impl.Invocation)
13781468
return nullptr;
13791469
}
@@ -1424,7 +1514,7 @@ std::unique_ptr<ClangImporter> ClangImporter::create(
14241514
auto actualDiagClient = std::make_unique<ClangDiagnosticConsumer>(
14251515
importer->Impl, instance.getDiagnosticOpts(),
14261516
importerOpts.DumpClangDiagnostics);
1427-
instance.createVirtualFileSystem(std::move(VFS), actualDiagClient.get());
1517+
instance.createVirtualFileSystem(std::move(vfs), actualDiagClient.get());
14281518
instance.createFileManager();
14291519
instance.createDiagnostics(actualDiagClient.release());
14301520

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)