Skip to content

[Dependency Scanning] Update Swift Interface Module's Output Path after vfs Pruning #79297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
32 changes: 15 additions & 17 deletions include/swift/AST/ModuleDependencies.h
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ class SwiftInterfaceModuleDependenciesStorage
: public ModuleDependencyInfoStorageBase {
public:
/// Destination output path
const std::string moduleOutputPath;
std::string moduleOutputPath;

/// The Swift interface file to be used to generate the module file.
const std::string swiftInterfaceFile;
Expand All @@ -275,7 +275,7 @@ class SwiftInterfaceModuleDependenciesStorage
const std::vector<std::string> compiledModuleCandidates;

/// The hash value that will be used for the generated module
const std::string contextHash;
std::string contextHash;

/// A flag that indicates this dependency is a framework
const bool isFramework;
Expand All @@ -290,22 +290,20 @@ class SwiftInterfaceModuleDependenciesStorage
const std::string userModuleVersion;

SwiftInterfaceModuleDependenciesStorage(
StringRef moduleOutputPath, StringRef swiftInterfaceFile,
StringRef swiftInterfaceFile,
ArrayRef<StringRef> compiledModuleCandidates,
ArrayRef<ScannerImportStatementInfo> moduleImports,
ArrayRef<ScannerImportStatementInfo> optionalModuleImports,
ArrayRef<StringRef> buildCommandLine, ArrayRef<LinkLibrary> linkLibraries,
StringRef contextHash, bool isFramework,
bool isStatic, StringRef RootID, StringRef moduleCacheKey,
StringRef userModuleVersion)
bool isFramework, bool isStatic, StringRef RootID,
StringRef moduleCacheKey, StringRef userModuleVersion)
: ModuleDependencyInfoStorageBase(ModuleDependencyKind::SwiftInterface,
moduleImports, optionalModuleImports,
linkLibraries, moduleCacheKey),
moduleOutputPath(moduleOutputPath),
swiftInterfaceFile(swiftInterfaceFile),
compiledModuleCandidates(compiledModuleCandidates.begin(),
compiledModuleCandidates.end()),
contextHash(contextHash), isFramework(isFramework), isStatic(isStatic),
isFramework(isFramework), isStatic(isStatic),
textualModuleDetails(buildCommandLine, RootID),
userModuleVersion(userModuleVersion) {}

Expand Down Expand Up @@ -585,21 +583,18 @@ class ModuleDependencyInfo {
/// Describe the module dependencies for a Swift module that can be
/// built from a Swift interface file (\c .swiftinterface).
static ModuleDependencyInfo forSwiftInterfaceModule(
StringRef moduleOutputPath, StringRef swiftInterfaceFile,
ArrayRef<StringRef> compiledCandidates, ArrayRef<StringRef> buildCommands,
StringRef swiftInterfaceFile, ArrayRef<StringRef> compiledCandidates,
ArrayRef<StringRef> buildCommands,
ArrayRef<ScannerImportStatementInfo> moduleImports,
ArrayRef<ScannerImportStatementInfo> optionalModuleImports,
ArrayRef<LinkLibrary> linkLibraries,
StringRef contextHash, bool isFramework, bool isStatic,
ArrayRef<LinkLibrary> linkLibraries, bool isFramework, bool isStatic,
StringRef CASFileSystemRootID, StringRef moduleCacheKey,
StringRef userModuleVersion) {
return ModuleDependencyInfo(
std::make_unique<SwiftInterfaceModuleDependenciesStorage>(
moduleOutputPath, swiftInterfaceFile, compiledCandidates,
moduleImports, optionalModuleImports,
buildCommands, linkLibraries, contextHash,
isFramework, isStatic, CASFileSystemRootID, moduleCacheKey,
userModuleVersion));
swiftInterfaceFile, compiledCandidates, moduleImports,
optionalModuleImports, buildCommands, linkLibraries, isFramework,
isStatic, CASFileSystemRootID, moduleCacheKey, userModuleVersion));
}

/// Describe the module dependencies for a serialized or parsed Swift module.
Expand Down Expand Up @@ -983,6 +978,9 @@ class ModuleDependencyInfo {
/// Set the chained bridging header buffer.
void setChainedBridgingHeaderBuffer(StringRef path, StringRef buffer);

/// Set the output path and the context hash.
void setOutputPathAndHash(StringRef outputPath, StringRef hash);

/// Collect a map from a secondary module name to a list of cross-import
/// overlays, when this current module serves as the primary module.
llvm::StringMap<llvm::SmallSetVector<Identifier, 4>>
Expand Down
31 changes: 22 additions & 9 deletions include/swift/Frontend/ModuleInterfaceLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,22 @@ struct SwiftInterfaceInfo {
std::optional<version::Version> CompilerToolsVersion;
};

struct InterfaceSubContextDelegateImpl: InterfaceSubContextDelegate {
namespace SwiftInterfaceModuleOutputPathResolution {
struct ResultTy {
llvm::SmallString<256> outputPath;

// Hash points to a segment of outputPath.
StringRef hash;
};

using ArgListTy = std::vector<std::string>;

void setOutputPath(ResultTy &outputPath, const StringRef &moduleName,
const StringRef &interfacePath, const StringRef &sdkPath,
const CompilerInvocation &CI, const ArgListTy &extraArgs);
} // namespace SwiftInterfaceModuleOutputPathResolution

struct InterfaceSubContextDelegateImpl : InterfaceSubContextDelegate {
private:
SourceManager &SM;
public:
Expand Down Expand Up @@ -702,14 +717,12 @@ struct InterfaceSubContextDelegateImpl: InterfaceSubContextDelegate {

~InterfaceSubContextDelegateImpl() = default;

/// includes a hash of relevant key data.
StringRef computeCachedOutputPath(StringRef moduleName,
StringRef UseInterfacePath,
StringRef sdkPath,
llvm::SmallString<256> &OutPath,
StringRef &CacheHash);
std::string getCacheHash(StringRef useInterfacePath, StringRef sdkPath);
/// resolvedOutputPath includes a hash of relevant key data.
void getCachedOutputPath(
SwiftInterfaceModuleOutputPathResolution::ResultTy &resolvedOutputPath,
StringRef moduleName, StringRef interfacePath, StringRef sdkPath);
};
}

} // namespace swift

#endif
15 changes: 15 additions & 0 deletions lib/AST/ModuleDependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,21 @@ void ModuleDependencyInfo::addSourceFile(StringRef sourceFile) {
}
}

void ModuleDependencyInfo::setOutputPathAndHash(StringRef outputPath,
StringRef hash) {
switch (getKind()) {
case swift::ModuleDependencyKind::SwiftInterface: {
auto swiftInterfaceStorage =
cast<SwiftInterfaceModuleDependenciesStorage>(storage.get());
swiftInterfaceStorage->moduleOutputPath = outputPath.str();
swiftInterfaceStorage->contextHash = hash.str();
break;
}
default:
llvm_unreachable("Unexpected dependency kind");
}
}

SwiftDependencyScanningService::SwiftDependencyScanningService() {
ClangScanningService.emplace(
clang::tooling::dependencies::ScanningMode::DependencyDirectivesScan,
Expand Down
10 changes: 5 additions & 5 deletions lib/DependencyScan/ModuleDependencyCacheSerialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -585,12 +585,12 @@ bool ModuleDependenciesCacheDeserializer::readGraph(

// Form the dependencies storage object
auto moduleDep = ModuleDependencyInfo::forSwiftInterfaceModule(
outputModulePath.value(), optionalSwiftInterfaceFile.value(),
compiledCandidatesRefs, buildCommandRefs, importStatements,
optionalImportStatements, linkLibraries, *contextHash,
isFramework, isStatic, *rootFileSystemID, *moduleCacheKey,
*userModuleVersion);
optionalSwiftInterfaceFile.value(), compiledCandidatesRefs,
buildCommandRefs, importStatements, optionalImportStatements,
linkLibraries, isFramework, isStatic, *rootFileSystemID,
*moduleCacheKey, *userModuleVersion);

moduleDep.setOutputPathAndHash(*outputModulePath, *contextHash);
addCommonDependencyInfo(moduleDep);
addSwiftCommonDependencyInfo(moduleDep);
addSwiftTextualDependencyInfo(
Expand Down
93 changes: 80 additions & 13 deletions lib/DependencyScan/ScanDependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ namespace {
class ExplicitModuleDependencyResolver {
public:
ExplicitModuleDependencyResolver(
ModuleDependencyID moduleID, ModuleDependenciesCache &cache,
const ModuleDependencyID &moduleID, ModuleDependenciesCache &cache,
CompilerInstance &instance, std::optional<SwiftDependencyTracker> tracker)
: moduleID(moduleID), cache(cache), instance(instance),
resolvingDepInfo(cache.findKnownDependency(moduleID)),
Expand Down Expand Up @@ -169,7 +169,11 @@ class ExplicitModuleDependencyResolver {
}
}

pruneUnusedVFSOverlay();
SwiftInterfaceModuleOutputPathResolution::ResultTy swiftInterfaceOutputPath;
if (resolvingDepInfo.isSwiftInterfaceModule()) {
pruneUnusedVFSOverlay(swiftInterfaceOutputPath);
updateSwiftInterfaceModuleOutputPath(swiftInterfaceOutputPath);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel you want to make to two separate function call here, especially when nothing is pruned, there is no need to go through and update everything.

The rough break down for how the updating process works is finalize() will update the module info to avoid repeated updating for the command-line for minor changes since that is not efficient. Everything before that is information collecting to help the finalize function to do the final update, and Resolver class itself has private fields to hold the information collected.

Copy link
Contributor Author

@qiongsiwu qiongsiwu Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel you want to make to two separate function call here, especially when nothing is pruned, there is no need to go through and update everything.

I can fold updateSwiftInterfaceModuleOutputPath into pruneUnusedVFSOverlay to avoid two function calls. I am a bit confused by the comment when nothing is pruned, there is no need to go through and update everything. I think in the case of swift interface modules, we no longer set its output path and context hash until here (relevant discussion). My understanding is that we want to update the command line only after we compute the correct output path, whether we have some options to prune or not. This way we don't end up with a path that can potentially points to nothing. Could you help me understand in which cases I can avoid the update?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be confused a little bit when I review the code because I thought the function is adding the output path to the command-line. I am a bit confused who is in charged of updating the -o flag on the command-line for building interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that this is where we compute the output path and add it to the command line.

Args.push_back(outputPathBase.str().str());

I think after this PR lands, I shall refactor this code. I don't think we need to calculate -o command here. We can directly calculate the output path when we are in the resolver.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Posted #79752 to refactor how we add -o to the command line.

}

// Update the dependency in the cache with the modified command-line.
if (resolvingDepInfo.isSwiftInterfaceModule() ||
Expand All @@ -182,7 +186,7 @@ class ExplicitModuleDependencyResolver {
}

auto dependencyInfoCopy = resolvingDepInfo;
if (auto err = finalize(dependencyInfoCopy))
if (auto err = finalize(dependencyInfoCopy, swiftInterfaceOutputPath))
return err;

dependencyInfoCopy.setIsFinalized(true);
Expand All @@ -192,14 +196,21 @@ class ExplicitModuleDependencyResolver {

private:
// Finalize the resolving dependency info.
llvm::Error finalize(ModuleDependencyInfo &depInfo) {
llvm::Error finalize(ModuleDependencyInfo &depInfo,
const SwiftInterfaceModuleOutputPathResolution::ResultTy
&swiftInterfaceModuleOutputPath) {
if (resolvingDepInfo.isSwiftPlaceholderModule())
return llvm::Error::success();

if (resolvingDepInfo.isSwiftInterfaceModule())
depInfo.setOutputPathAndHash(
swiftInterfaceModuleOutputPath.outputPath.str().str(),
swiftInterfaceModuleOutputPath.hash.str());

// Add macros.
for (auto &macro : macros)
depInfo.addMacroDependency(
macro.first(), macro.second.LibraryPath, macro.second.ExecutablePath);
depInfo.addMacroDependency(macro.first(), macro.second.LibraryPath,
macro.second.ExecutablePath);

for (auto &macro : depInfo.getMacroDependencies()) {
std::string arg = macro.second.LibraryPath + "#" +
Expand All @@ -213,8 +224,7 @@ class ExplicitModuleDependencyResolver {
return err;

if (!bridgingHeaderBuildCmd.empty())
depInfo.updateBridgingHeaderCommandLine(
bridgingHeaderBuildCmd);
depInfo.updateBridgingHeaderCommandLine(bridgingHeaderBuildCmd);
if (!resolvingDepInfo.isSwiftBinaryModule()) {
depInfo.updateCommandLine(commandline);
if (auto err = updateModuleCacheKey(depInfo))
Expand Down Expand Up @@ -376,16 +386,18 @@ class ExplicitModuleDependencyResolver {
}
}

void pruneUnusedVFSOverlay() {
void pruneUnusedVFSOverlay(
SwiftInterfaceModuleOutputPathResolution::ResultTy &outputPath) {
// Pruning of unused VFS overlay options for Clang dependencies is performed
// by the Clang dependency scanner.
if (moduleID.Kind == ModuleDependencyKind::Clang)
return;

// Prune the command line.
std::vector<std::string> resolvedCommandLine;
size_t skip = 0;
for (auto it = commandline.begin(), end = commandline.end();
it != end; it++) {
for (auto it = commandline.begin(), end = commandline.end(); it != end;
it++) {
if (skip) {
skip--;
continue;
Expand All @@ -402,7 +414,62 @@ class ExplicitModuleDependencyResolver {
}
resolvedCommandLine.push_back(*it);
}

commandline = std::move(resolvedCommandLine);

// Prune the clang impoter options. We do not need to deal with -Xcc because
// these are clang options.
const auto &CI = instance.getInvocation();

SwiftInterfaceModuleOutputPathResolution::ArgListTy extraArgsList;
const auto &clangImporterOptions =
CI.getClangImporterOptions()
.getReducedExtraArgsForSwiftModuleDependency();

skip = 0;
for (auto it = clangImporterOptions.begin(),
end = clangImporterOptions.end();
it != end; it++) {
if (skip) {
skip = 0;
continue;
}

if ((it + 1) != end && isVFSOverlayFlag(*it)) {
if (!usedVFSOverlayPaths.contains(*(it + 1))) {
skip = 1;
continue;
}
}

extraArgsList.push_back(*it);
}

auto swiftTextualDeps = resolvingDepInfo.getAsSwiftInterfaceModule();
auto &interfacePath = swiftTextualDeps->swiftInterfaceFile;
auto sdkPath = instance.getASTContext().SearchPathOpts.getSDKPath();
SwiftInterfaceModuleOutputPathResolution::setOutputPath(
outputPath, moduleID.ModuleName, interfacePath, sdkPath, CI,
extraArgsList);

return;
}

void updateSwiftInterfaceModuleOutputPath(
const SwiftInterfaceModuleOutputPathResolution::ResultTy &outputPath) {
StringRef outputName = outputPath.outputPath.str();

bool isOutputPath = false;
for (auto &A : commandline) {
if (isOutputPath) {
A = outputName.str();
break;
} else if (A == "-o") {
isOutputPath = true;
}
}

return;
}

llvm::Error collectCASDependencies(ModuleDependencyInfo &dependencyInfoCopy) {
Expand Down Expand Up @@ -524,7 +591,7 @@ class ExplicitModuleDependencyResolver {
}

private:
ModuleDependencyID moduleID;
const ModuleDependencyID &moduleID;
ModuleDependenciesCache &cache;
CompilerInstance &instance;
const ModuleDependencyInfo &resolvingDepInfo;
Expand All @@ -540,7 +607,7 @@ class ExplicitModuleDependencyResolver {
};

static llvm::Error resolveExplicitModuleInputs(
ModuleDependencyID moduleID,
const ModuleDependencyID &moduleID,
const std::set<ModuleDependencyID> &dependencies,
ModuleDependenciesCache &cache, CompilerInstance &instance,
std::optional<std::set<ModuleDependencyID>> bridgingHeaderDeps,
Expand Down
Loading