Skip to content

Commit 500d2e5

Browse files
artemcmartemcm
committed
[Dependency Scanning] Only omit Clang VFS overlays from Swift dependencies if unused by Clang dependencies
We previously blanket omitted `-Xcc -vfsoverlay` flags from Swift module dependencies' command-line recipes. This is incorrect as the Swift module must have an exact matching VFS overlay that its Clang dependencies use, in order to load said Clang dependnecies successfully and resolve their headers as expected and as was done during the scan. Resolves rdar://122667530
1 parent 9607e9d commit 500d2e5

File tree

4 files changed

+247
-5
lines changed

4 files changed

+247
-5
lines changed

‎lib/Basic/LangOptions.cpp

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,7 @@ DiagnosticBehavior LangOptions::getAccessNoteFailureLimit() const {
697697
}
698698

699699
namespace {
700-
static constexpr std::array<std::string_view, 16> knownSearchPathPrefiexes =
700+
constexpr std::array<std::string_view, 16> knownSearchPathPrefiexes =
701701
{"-I",
702702
"-F",
703703
"-fmodule-map-file=",
@@ -714,6 +714,23 @@ namespace {
714714
"-ivfsoverlay",
715715
"-working-directory=",
716716
"-working-directory"};
717+
718+
constexpr std::array<std::string_view, 15> knownClangDependencyIgnorablePrefiexes =
719+
{"-I",
720+
"-F",
721+
"-fmodule-map-file=",
722+
"-iquote",
723+
"-idirafter",
724+
"-iframeworkwithsysroot",
725+
"-iframework",
726+
"-iprefix",
727+
"-iwithprefixbefore",
728+
"-iwithprefix",
729+
"-isystemafter",
730+
"-isystem",
731+
"-isysroot",
732+
"-working-directory=",
733+
"-working-directory"};
717734
}
718735

719736
std::vector<std::string> ClangImporterOptions::getRemappedExtraArgs(
@@ -756,7 +773,7 @@ std::vector<std::string> ClangImporterOptions::getRemappedExtraArgs(
756773
std::vector<std::string>
757774
ClangImporterOptions::getReducedExtraArgsForSwiftModuleDependency() const {
758775
auto matchIncludeOption = [](StringRef &arg) {
759-
for (const auto &option : knownSearchPathPrefiexes)
776+
for (const auto &option : knownClangDependencyIgnorablePrefiexes)
760777
if (arg.consume_front(option))
761778
return true;
762779
return false;

‎lib/DependencyScan/ScanDependencies.cpp

Lines changed: 99 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,97 @@ static llvm::Error resolveExplicitModuleInputs(
415415
return llvm::Error::success();
416416
}
417417

418+
static llvm::Error pruneUnusedVFSOverlays(
419+
ModuleDependencyID moduleID, const ModuleDependencyInfo &resolvingDepInfo,
420+
const std::set<ModuleDependencyID> &dependencies,
421+
ModuleDependenciesCache &cache, CompilerInstance &instance) {
422+
auto isVFSOverlayFlag = [](StringRef arg) {
423+
return arg == "-ivfsoverlay" || arg == "-vfsoverlay";
424+
};
425+
auto isXCCArg = [](StringRef arg) {
426+
return arg == "-Xcc";
427+
};
428+
429+
// Pruning of unused VFS overlay options for Clang dependencies
430+
// is performed by the Clang dependency scanner.
431+
if (!resolvingDepInfo.isSwiftModule())
432+
return llvm::Error::success();
433+
434+
// If this Swift dependency contains any VFS overlay paths,
435+
// then attempt to prune the ones not used by any of the Clang dependencies.
436+
if (!llvm::any_of(resolvingDepInfo.getCommandline(),
437+
[&isVFSOverlayFlag](const std::string &arg) {
438+
return isVFSOverlayFlag(arg);
439+
}))
440+
return llvm::Error::success();
441+
442+
// 1. For each Clang dependency, gather its ivfsoverlay path arguments
443+
// to keep track of which overlays are actually used and were not
444+
// pruned by the Clang dependency scanner.
445+
llvm::StringSet<> usedVFSOverlayPaths;
446+
for (const auto &depModuleID : dependencies) {
447+
const auto optionalDepInfo = cache.findDependency(depModuleID);
448+
assert(optionalDepInfo.has_value());
449+
const auto depInfo = optionalDepInfo.value();
450+
if (auto clangDepDetails = depInfo->getAsClangModule()) {
451+
const auto &depCommandLine = clangDepDetails->buildCommandLine;
452+
// true if the previous argument was the dash-option of an option pair
453+
bool getNext = false;
454+
for (const auto &A : depCommandLine) {
455+
StringRef arg(A);
456+
if (isXCCArg(arg))
457+
continue;
458+
if (getNext) {
459+
getNext = false;
460+
usedVFSOverlayPaths.insert(arg);
461+
} else if (isVFSOverlayFlag(arg))
462+
getNext = true;
463+
}
464+
}
465+
}
466+
467+
// 2. Each -Xcc VFS overlay path on the resolving command-line which is not used by
468+
// any of the Clang dependencies can be removed from the command-line.
469+
const std::vector<std::string> &currentCommandLine =
470+
resolvingDepInfo.getCommandline();
471+
std::vector<std::string> resolvedCommandLine;
472+
size_t skip = 0;
473+
for (auto it = currentCommandLine.begin(), end = currentCommandLine.end();
474+
it != end; it++) {
475+
if (skip) {
476+
skip--;
477+
continue;
478+
}
479+
// If this VFS overlay was not used across any of the dependencies, skip it.
480+
if ((it+1) != end && isXCCArg(*it) && isVFSOverlayFlag(*(it + 1))) {
481+
assert(it + 2 != end); // Extra -Xcc
482+
assert(it + 3 != end); // Actual VFS overlay path argument
483+
if (!usedVFSOverlayPaths.contains(*(it + 3))) {
484+
skip = 3;
485+
continue;
486+
}
487+
}
488+
resolvedCommandLine.push_back(*it);
489+
}
490+
491+
// 3. Update the dependency in the cache if the command-line has been modified.
492+
if (currentCommandLine.size() != resolvedCommandLine.size()) {
493+
auto dependencyInfoCopy = resolvingDepInfo;
494+
dependencyInfoCopy.updateCommandLine(resolvedCommandLine);
495+
496+
// Update the CAS cache key for the new command-line
497+
if (instance.getInvocation().getCASOptions().EnableCaching) {
498+
auto &CAS = cache.getScanService().getSharedCachingFS().getCAS();
499+
auto Key = updateModuleCacheKey(dependencyInfoCopy, cache, CAS);
500+
if (!Key)
501+
return Key.takeError();
502+
}
503+
cache.updateDependency(moduleID, dependencyInfoCopy);
504+
}
505+
506+
return llvm::Error::success();
507+
}
508+
418509
namespace {
419510
std::string quote(StringRef unquoted) {
420511
llvm::SmallString<128> buffer;
@@ -1658,7 +1749,7 @@ swift::dependencies::createEncodedModuleKindAndName(ModuleDependencyID id) {
16581749
}
16591750
}
16601751

1661-
static void resolveDependencyInputCommandLineArguments(
1752+
static void resolveDependencyCommandLineArguments(
16621753
CompilerInstance &instance, ModuleDependenciesCache &cache,
16631754
const std::vector<ModuleDependencyID> &topoSortedModuleList) {
16641755
auto moduleTransitiveClosures =
@@ -1676,6 +1767,11 @@ static void resolveDependencyInputCommandLineArguments(
16761767
cache, instance))
16771768
instance.getDiags().diagnose(SourceLoc(), diag::error_cas,
16781769
toString(std::move(E)));
1770+
1771+
if (auto E = pruneUnusedVFSOverlays(modID, *deps, dependencyClosure,
1772+
cache, instance))
1773+
instance.getDiags().diagnose(SourceLoc(), diag::error_cas,
1774+
toString(std::move(E)));
16791775
}
16801776
}
16811777

@@ -1756,8 +1852,8 @@ swift::dependencies::performModuleScan(CompilerInstance &instance,
17561852

17571853
auto topologicallySortedModuleList =
17581854
computeTopologicalSortOfExplicitDependencies(allModules, cache);
1759-
resolveDependencyInputCommandLineArguments(instance, cache,
1760-
topologicallySortedModuleList);
1855+
resolveDependencyCommandLineArguments(instance, cache,
1856+
topologicallySortedModuleList);
17611857

17621858
updateDependencyTracker(instance, cache, allModules);
17631859
return generateFullDependencyGraph(instance, cache,

‎test/ScanDependencies/eliminate_unused_vfs.swift

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// REQUIRES: objc_interop
2+
// RUN: %empty-directory(%t)
3+
// RUN: %empty-directory(%t/module-cache)
4+
// RUN: %empty-directory(%t/redirects)
5+
// RUN: split-file %s %t
6+
7+
// RUN: sed -e "s|OUT_DIR|%t/redirects|g" -e "s|IN_DIR|%S/Inputs/CHeaders|g" %t/overlay_template.yaml > %t/overlay.yaml
8+
9+
// RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t/module-cache %t/test.swift -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -Xcc -ivfsoverlay -Xcc %t/overlay.yaml
10+
// RUN: %validate-json %t/deps.json | %FileCheck %s
11+
12+
//--- overlay_template.yaml
13+
{
14+
'version': 0,
15+
'use-external-names': false,
16+
'roots': [
17+
{
18+
'name': 'IN_DIR', 'type': 'directory',
19+
'contents': [
20+
]
21+
},
22+
]
23+
}
24+
25+
//--- test.swift
26+
import F
27+
28+
// CHECK: "mainModuleName": "deps"
29+
/// --------Main module
30+
// CHECK-LABEL: "modulePath": "deps.swiftmodule",
31+
// CHECK-NEXT: sourceFiles
32+
// CHECK-NEXT: test.swift
33+
// CHECK-NEXT: ],
34+
// CHECK-NEXT: "directDependencies": [
35+
// CHECK-DAG: "swift": "F"
36+
// CHECK-DAG: "swift": "Swift"
37+
// CHECK-DAG: "swift": "SwiftOnoneSupport"
38+
// CHECK: ],
39+
40+
// Ensure that the VFS overlay command-line flag is pruned on the Swift module dependency
41+
// that uses a Clang module which has optimized it away as un-used.
42+
/// --------Swift module F
43+
// CHECK-LABEL: "modulePath": "{{.*}}{{/|\\}}F-{{.*}}.swiftmodule",
44+
45+
// CHECK: "directDependencies": [
46+
// CHECK-NEXT: {
47+
// CHECK-DAG: "clang": "F"
48+
// CHECK-DAG: "swift": "Swift"
49+
// CHECK-DAG: "swift": "SwiftOnoneSupport"
50+
// CHECK-NEXT: }
51+
// CHECK-NEXT: ],
52+
53+
// CHECK: "commandLine": [
54+
// CHECK: "-compile-module-from-interface"
55+
// CHECK-NOT: "-ivfsoverlay",
56+
// CHECK-NOT: "{{.*}}{{/|\\}}preserve_used_vfs.swift.tmp{{/|\\}}overlay.yaml",
57+
// CHECK: ],
58+
59+
/// --------Clang module F
60+
// CHECK-LABEL: "modulePath": "{{.*}}{{/|\\}}F-{{.*}}.pcm",
61+
// CHECK-NOT: "-ivfsoverlay",
62+
// CHECK-NOT: "{{.*}}{{/|\\}}preserve_used_vfs.swift.tmp{{/|\\}}overlay.yaml",

‎test/ScanDependencies/preserve_used_vfs.swift

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// REQUIRES: objc_interop
2+
// RUN: %empty-directory(%t)
3+
// RUN: %empty-directory(%t/module-cache)
4+
// RUN: %empty-directory(%t/redirects)
5+
// RUN: split-file %s %t
6+
7+
// RUN: sed -e "s|OUT_DIR|%t/redirects|g" -e "s|IN_DIR|%S/Inputs/CHeaders|g" %t/overlay_template.yaml > %t/overlay.yaml
8+
9+
// RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t/module-cache %t/test.swift -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -Xcc -ivfsoverlay -Xcc %t/overlay.yaml
10+
// RUN: %validate-json %t/deps.json | %FileCheck %s
11+
12+
//--- redirects/RedirectedF.h
13+
void funcRedirectedF(void);
14+
15+
//--- overlay_template.yaml
16+
{
17+
'version': 0,
18+
'use-external-names': false,
19+
'roots': [
20+
{
21+
'name': 'IN_DIR', 'type': 'directory',
22+
'contents': [
23+
{ 'name': 'F.h', 'type': 'file',
24+
'external-contents': 'OUT_DIR/RedirectedF.h'
25+
}
26+
]
27+
},
28+
]
29+
}
30+
31+
//--- test.swift
32+
import F
33+
34+
// CHECK: "mainModuleName": "deps"
35+
/// --------Main module
36+
// CHECK-LABEL: "modulePath": "deps.swiftmodule",
37+
// CHECK-NEXT: sourceFiles
38+
// CHECK-NEXT: test.swift
39+
// CHECK-NEXT: ],
40+
// CHECK-NEXT: "directDependencies": [
41+
// CHECK-DAG: "swift": "F"
42+
// CHECK-DAG: "swift": "Swift"
43+
// CHECK-DAG: "swift": "SwiftOnoneSupport"
44+
// CHECK: ],
45+
46+
// Ensure that the VFS overlay command-line flag is preserved on the Swift module dependency
47+
// that uses a Clang module affected by this overlay
48+
/// --------Swift module F
49+
// CHECK-LABEL: "modulePath": "{{.*}}{{/|\\}}F-{{.*}}.swiftmodule",
50+
51+
// CHECK: "directDependencies": [
52+
// CHECK-NEXT: {
53+
// CHECK-DAG: "clang": "F"
54+
// CHECK-DAG: "swift": "Swift"
55+
// CHECK-DAG: "swift": "SwiftOnoneSupport"
56+
// CHECK-NEXT: }
57+
// CHECK-NEXT: ],
58+
59+
// CHECK: "commandLine": [
60+
// CHECK: "-compile-module-from-interface"
61+
// CHECK: "-ivfsoverlay",
62+
// CHECK-NEXT: "-Xcc",
63+
// CHECK-NEXT: "{{.*}}{{/|\\}}preserve_used_vfs.swift.tmp{{/|\\}}overlay.yaml",
64+
// CHECK: ],
65+
66+
/// --------Clang module F
67+
// CHECK-LABEL: "modulePath": "{{.*}}{{/|\\}}F-{{.*}}.pcm",

0 commit comments

Comments
 (0)