Skip to content

[Dependency Scanning] Refactor the scanner to resolve unqualified module imports #62696

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.

Sign up for GitHub

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
merged 1 commit into from
Jan 6, 2023
Merged

[Dependency Scanning] Refactor the scanner to resolve unqualified module imports #62696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions include/swift/AST/ASTContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -991,18 +991,20 @@ class ASTContext final {

/// Retrieve the module dependencies for the module with the given name.
///
/// \param isUnderlyingClangModule When true, only look for a Clang module
/// with the given name, ignoring any Swift modules.
Optional<ModuleDependencyInfo> getModuleDependencies(
Optional<const ModuleDependencyInfo*> getModuleDependencies(
StringRef moduleName,
bool isUnderlyingClangModule,
ModuleDependenciesCache &cache,
InterfaceSubContextDelegate &delegate,
bool cacheOnly = false,
llvm::Optional<std::pair<std::string, swift::ModuleDependencyKind>> dependencyOf = None);

/// Retrieve the module dependencies for the Clang module with the given name.
Optional<const ModuleDependencyInfo*> getClangModuleDependencies(
StringRef moduleName,
ModuleDependenciesCache &cache,
InterfaceSubContextDelegate &delegate);

/// Retrieve the module dependencies for the Swift module with the given name.
Optional<ModuleDependencyInfo> getSwiftModuleDependencies(
Optional<const ModuleDependencyInfo*> getSwiftModuleDependencies(
StringRef moduleName,
ModuleDependenciesCache &cache,
InterfaceSubContextDelegate &delegate);
Expand Down
142 changes: 85 additions & 57 deletions include/swift/AST/ModuleDependencies.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,29 +74,44 @@ enum class ModuleDependencyKind : int8_t {
LastKind = SwiftPlaceholder + 1
};

using ModuleDependencyID = std::pair<std::string, ModuleDependencyKind>;
using ModuleDependencyIDSetVector =
llvm::SetVector<ModuleDependencyID, std::vector<ModuleDependencyID>,
std::set<ModuleDependencyID>>;

struct ModuleDependencyKindHash {
std::size_t operator()(ModuleDependencyKind k) const {
using UnderlyingType = std::underlying_type<ModuleDependencyKind>::type;
return std::hash<UnderlyingType>{}(static_cast<UnderlyingType>(k));
}
};

namespace dependencies {
std::string createEncodedModuleKindAndName(ModuleDependencyID id);
}

/// Base class for the variant storage of ModuleDependencyInfo.
///
/// This class is mostly an implementation detail for \c ModuleDependencyInfo.
class ModuleDependencyInfoStorageBase {
public:
const ModuleDependencyKind dependencyKind;

ModuleDependencyInfoStorageBase(ModuleDependencyKind dependencyKind)
: dependencyKind(dependencyKind) { }
ModuleDependencyInfoStorageBase(ModuleDependencyKind dependencyKind,
bool resolved = false)
: dependencyKind(dependencyKind), resolved(resolved) { }

virtual ModuleDependencyInfoStorageBase *clone() const = 0;

virtual ~ModuleDependencyInfoStorageBase();

/// The set of modules on which this module depends.
std::vector<std::string> moduleDependencies;
std::vector<std::string> moduleImports;

/// The set of modules on which this module depends, resolved
/// to Module IDs, qualified by module kind: Swift, Clang, etc.
std::vector<ModuleDependencyID> resolvedModuleDependencies;
bool resolved;
};

struct CommonSwiftTextualModuleDependencyDetails {
Expand Down Expand Up @@ -400,9 +415,29 @@ class ModuleDependencyInfo {
compiledModulePath, moduleDocPath, sourceInfoPath));
}

/// Retrieve the module-level dependencies.
ArrayRef<std::string> getModuleDependencies() const {
return storage->moduleDependencies;
/// Retrieve the module-level imports.
ArrayRef<std::string> getModuleImports() const {
return storage->moduleImports;
}

/// Retreive the module-level dependencies.
const ArrayRef<ModuleDependencyID> getModuleDependencies() const {
assert(storage->resolved);
return storage->resolvedModuleDependencies;
}

/// Resolve a dependency's set of `imports` with qualified Module IDs
void resolveDependencies(const std::vector<ModuleDependencyID> &dependencyIDs) {
assert(!storage->resolved && "Resolving an already-resolved dependency");
storage->resolved = true;
storage->resolvedModuleDependencies.assign(dependencyIDs.begin(), dependencyIDs.end());
}

bool isResolved() const {
return storage->resolved;
}
void setIsResolved(bool isResolved) {
storage->resolved = isResolved;
}

/// Whether the dependencies are for a Swift module: either Textual, Source, Binary, or Placeholder.
Expand Down Expand Up @@ -444,19 +479,22 @@ class ModuleDependencyInfo {
getAsPlaceholderDependencyModule() const;

/// Add a dependency on the given module, if it was not already in the set.
void addModuleDependency(StringRef module,
llvm::StringSet<> *alreadyAddedModules = nullptr);
void addModuleImport(StringRef module,
llvm::StringSet<> *alreadyAddedModules = nullptr);

/// Add a dependency on the given module, if it was not already in the set.
void addModuleDependency(ImportPath::Module module,
llvm::StringSet<> *alreadyAddedModules = nullptr) {
addModuleDependency(module.front().Item.str(), alreadyAddedModules);
void addModuleImport(ImportPath::Module module,
llvm::StringSet<> *alreadyAddedModules = nullptr) {
addModuleImport(module.front().Item.str(), alreadyAddedModules);
}

/// Add all of the module dependencies for the imports in the given source
/// file to the set of module dependencies.
void addModuleDependencies(const SourceFile &sf,
llvm::StringSet<> &alreadyAddedModules);
/// Add all of the module imports in the given source
/// file to the set of module imports.
void addModuleImport(const SourceFile &sf,
llvm::StringSet<> &alreadyAddedModules);
/// Add a kind-qualified module dependency ID to the set of
/// module dependencies.
void addModuleDependency(ModuleDependencyID dependencyID);

/// Get the bridging header.
Optional<std::string> getBridgingHeader() const;
Expand All @@ -477,11 +515,9 @@ class ModuleDependencyInfo {
/// 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>>
collectCrossImportOverlayNames(ASTContext &ctx, StringRef moduleName);
collectCrossImportOverlayNames(ASTContext &ctx, StringRef moduleName) const;
};

using ModuleDependencyID = std::pair<std::string, ModuleDependencyKind>;
using ModuleDependenciesVector = llvm::SmallVector<ModuleDependencyInfo, 1>;
using ModuleNameToDependencyMap = llvm::StringMap<ModuleDependencyInfo>;
using ModuleDependenciesKindMap =
std::unordered_map<ModuleDependencyKind,
Expand All @@ -505,6 +541,9 @@ class SwiftDependencyScanningService {
/// encountered.
std::vector<ModuleDependencyID> AllModules;

/// Set containing all of the Clang modules that have already been seen.
llvm::StringSet<> alreadySeenClangModules;

/// Dependencies for modules that have already been computed.
/// This maps a dependency kind to a map of a module's name to a Dependency
/// object
Expand Down Expand Up @@ -565,6 +604,10 @@ class SwiftDependencyScanningService {
return *SharedFilesystemCache;
}

llvm::StringSet<>& getAlreadySeenClangModules() {
return getCurrentCache()->alreadySeenClangModules;
}

/// Wrap the filesystem on the specified `CompilerInstance` with a
/// caching `DependencyScanningWorkerFilesystem`
void overlaySharedFilesystemCacheForCompilation(CompilerInstance &Instance);
Expand All @@ -587,8 +630,8 @@ class SwiftDependencyScanningService {
}

/// Whether we have cached dependency information for the given module.
bool hasDependencies(StringRef moduleName,
Optional<ModuleDependencyKind> kind) const;
bool hasDependency(StringRef moduleName,
Optional<ModuleDependencyKind> kind) const;

/// Return a pointer to the context-specific cache state of the current
/// scanning action.
Expand All @@ -599,22 +642,22 @@ class SwiftDependencyScanningService {
getCacheForScanningContextHash(StringRef scanningContextHash) const;

/// Look for source-based module dependency details
Optional<ModuleDependencyInfo>
Optional<const ModuleDependencyInfo*>
findSourceModuleDependency(StringRef moduleName) const;

/// Look for module dependencies for a module with the given name
///
/// \returns the cached result, or \c None if there is no cached entry.
Optional<ModuleDependencyInfo>
findDependencies(StringRef moduleName,
Optional<ModuleDependencyKind> kind) const;
Optional<const ModuleDependencyInfo*>
findDependency(StringRef moduleName,
Optional<ModuleDependencyKind> kind) const;

/// Record dependencies for the given module.
const ModuleDependencyInfo *recordDependencies(StringRef moduleName,
const ModuleDependencyInfo *recordDependency(StringRef moduleName,
Copy link
Contributor

Choose a reason for hiding this comment

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

One thing I'm not quite following is the de-pluralization of these method names. Aren't there potentially multiple dependencies of moduleID in the given ModuleDependencyInfo collection?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While there are, indeed, multiple dependencies of moduleID in the given ModuleDependencyInfo, the thing being recorded here is the existence of a ModuleDependencyInfo itself for this moduleID. Which does contain a collection of its own dependencies' IDs, each of which will then correspond to a ModuleDependencyInfo to be queried via these same API, but also a bunch of other metadata we need to know about this dependency.

I can see the logic behind the pluralization but it's been bugging me for a while and it seems to be clearer to be explicit about the fact that these API are vending these info values that describe a particular module, not just that module's dependencies.

For example, before this patch hasDependencies("foo") would indicate whether or not a module "foo" has been discovered as a dependency during this scan. To your point, it does also indicate that we know dependencies of foo in its respective info, but the use of these queries is broader than querying dependencies of this module as it also covers getting a dependency info about this module for other purposes as well. It seems conceptually more-accurate to have some thing like findDependencies("foo") return an actual list of dependencies of foo, rather than an info describing foo as a discovered dependency itself.

I'm having to really make an argument here so I can see it either way and can be convinced otherwise, but this seemed more appropriate as I was already restructuring things!

ModuleDependencyInfo dependencies);

/// Update stored dependencies for the given module.
const ModuleDependencyInfo *updateDependencies(ModuleDependencyID moduleID,
const ModuleDependencyInfo *updateDependency(ModuleDependencyID moduleID,
ModuleDependencyInfo dependencies);

/// Reference the list of all module dependencies that are not source-based
Expand Down Expand Up @@ -642,23 +685,14 @@ class SwiftDependencyScanningService {
class ModuleDependenciesCache {
private:
SwiftDependencyScanningService &globalScanningService;

/// References to data in `globalCache` for Swift dependencies and
/// `clangModuleDependencies` for Clang dependencies accimulated during
/// the current scanning action.
/// References to data in the `globalScanningService` for module dependencies
ModuleDependenciesKindRefMap ModuleDependenciesMap;

/// Name of the module under scan
std::string mainScanModuleName;
/// The context hash of the current scanning invocation
std::string scannerContextHash;

/// Set containing all of the Clang modules that have already been seen.
llvm::StringSet<> alreadySeenClangModules;
/// The Clang dependency scanner tool
clang::tooling::dependencies::DependencyScanningTool clangScanningTool;
/// Discovered Clang modules are only cached locally.
llvm::StringMap<ModuleDependenciesVector> clangModuleDependencies;

/// Retrieve the dependencies map that corresponds to the given dependency
/// kind.
Expand All @@ -667,17 +701,6 @@ class ModuleDependenciesCache {
const llvm::StringMap<const ModuleDependencyInfo *> &
getDependencyReferencesMap(ModuleDependencyKind kind) const;

/// Local cache results lookup, only for modules which were discovered during
/// the current scanner invocation.
bool hasDependenciesLocalOnly(StringRef moduleName,
Optional<ModuleDependencyKind> kind) const;

/// Local cache results lookup, only for modules which were discovered during
/// the current scanner invocation.
Optional<const ModuleDependencyInfo *>
findDependenciesLocalOnly(StringRef moduleName,
Optional<ModuleDependencyKind> kind) const;

public:
ModuleDependenciesCache(SwiftDependencyScanningService &globalScanningService,
std::string mainScanModuleName,
Expand All @@ -687,31 +710,36 @@ class ModuleDependenciesCache {

public:
/// Whether we have cached dependency information for the given module.
bool hasDependencies(StringRef moduleName,
Optional<ModuleDependencyKind> kind) const;
bool hasDependency(StringRef moduleName,
Optional<ModuleDependencyKind> kind) const;

/// Produce a reference to the Clang scanner tool associated with this cache
clang::tooling::dependencies::DependencyScanningTool& getClangScannerTool() {
return clangScanningTool;
}
llvm::StringSet<>& getAlreadySeenClangModules() {
return alreadySeenClangModules;
return globalScanningService.getAlreadySeenClangModules();
}

/// Look for module dependencies for a module with the given name
///
/// \returns the cached result, or \c None if there is no cached entry.
Optional<ModuleDependencyInfo>
findDependencies(StringRef moduleName,
Optional<ModuleDependencyKind> kind) const;
Optional<const ModuleDependencyInfo*>
findDependency(StringRef moduleName,
Optional<ModuleDependencyKind> kind) const;

/// Record dependencies for the given module.
void recordDependencies(StringRef moduleName,
ModuleDependencyInfo dependencies);
void recordDependency(StringRef moduleName,
ModuleDependencyInfo dependencies);

/// Update stored dependencies for the given module.
void updateDependencies(ModuleDependencyID moduleID,
ModuleDependencyInfo dependencies);
void updateDependency(ModuleDependencyID moduleID,
ModuleDependencyInfo dependencies);

/// Resolve a dependency module's set of imports
/// to a kind-qualified set of module IDs.
void resolveDependencyImports(ModuleDependencyID moduleID,
const std::vector<ModuleDependencyID> &dependencyIDs);

const std::vector<ModuleDependencyID> &getAllSourceModules() const {
return globalScanningService.getAllSourceModules();
Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/ModuleLoader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ class ModuleLoader {

/// Retrieve the dependencies for the given, named module, or \c None
/// if no such module exists.
virtual Optional<ModuleDependencyInfo> getModuleDependencies(
virtual Optional<const ModuleDependencyInfo*> getModuleDependencies(
StringRef moduleName,
ModuleDependenciesCache &cache,
InterfaceSubContextDelegate &delegate) = 0;
Expand Down
2 changes: 1 addition & 1 deletion include/swift/ClangImporter/ClangImporter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ class ClangImporter final : public ClangModuleLoader {
ModuleDependenciesCache &cache,
const clang::tooling::dependencies::FullDependenciesResult &clangDependencies);

Optional<ModuleDependencyInfo> getModuleDependencies(
Optional<const ModuleDependencyInfo*> getModuleDependencies(
StringRef moduleName, ModuleDependenciesCache &cache,
InterfaceSubContextDelegate &delegate) override;

Expand Down
Loading