Skip to content

Revert "[NFC] Clarify semantics of getImportedModules." #34410

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 3 additions & 29 deletions include/swift/AST/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -593,8 +593,7 @@ class ModuleDecl : public DeclContext, public TypeDecl {
Default = 1 << 1,
/// Include imports declared with `@_implementationOnly`.
ImplementationOnly = 1 << 2,
/// Include imports of SPIs declared with `@_spi`. Non-SPI imports are
/// included whether or not this flag is specified.
/// Include imports of SPIs declared with `@_spi`
SPIAccessControl = 1 << 3,
/// Include imports shadowed by a cross-import overlay. Unshadowed imports
/// are included whether or not this flag is specified.
Expand All @@ -605,33 +604,8 @@ class ModuleDecl : public DeclContext, public TypeDecl {

/// Looks up which modules are imported by this module.
///
/// \p filter controls which imports are included in the list.
///
/// There are three axes for categorizing imports:
/// 1. Privacy: Exported/Private/ImplementationOnly (mutually exclusive).
/// 2. SPI/non-SPI: An import of any privacy level may be @_spi("SPIName").
/// 3. Shadowed/Non-shadowed: An import of any privacy level may be shadowed
/// by a cross-import overlay.
///
/// It is also possible for SPI imports to be shadowed by a cross-import
/// overlay.
///
/// If \p filter contains multiple privacy levels, modules at all the privacy
/// levels are included.
///
/// If \p filter contains \c ImportFilterKind::SPIAccessControl, then both
/// SPI and non-SPI imports are included. Otherwise, only non-SPI imports are
/// included.
///
/// If \p filter contains \c ImportFilterKind::ShadowedByCrossImportOverlay,
/// both shadowed and non-shadowed imports are included. Otherwise, only
/// non-shadowed imports are included.
///
/// Clang modules have some additional complexities; see the implementation of
/// \c ClangModuleUnit::getImportedModules for details.
///
/// \pre \p filter must contain at least one privacy level, i.e. one of
/// \c Exported or \c Private or \c ImplementationOnly.
/// \p filter controls whether public, private, or any imports are included
/// in this list.
void getImportedModules(SmallVectorImpl<ImportedModule> &imports,
ImportFilter filter = ImportFilterKind::Exported) const;

Expand Down
9 changes: 0 additions & 9 deletions include/swift/Basic/OptionSet.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

#include "llvm/ADT/None.h"

#include <cassert>
#include <type_traits>
#include <cstdint>
#include <initializer_list>
Expand Down Expand Up @@ -99,14 +98,6 @@ class OptionSet {
return Storage == set.Storage;
}

/// Check if this option set contains any options from \p set.
///
/// \pre \p set must be non-empty.
bool containsAny(OptionSet set) const {
assert((bool)set && "argument must be non-empty");
return (bool)((*this) & set);
}

// '==' and '!=' are deliberately not defined because they provide a pitfall
// where someone might use '==' but really want 'contains'. If you actually
// want '==' behavior, use 'containsOnly'.
Expand Down
2 changes: 0 additions & 2 deletions lib/AST/ImportCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ ImportSet &ImportCache::getImportSet(const DeclContext *dc) {
imports.emplace_back(ImportPath::Access(), mod);

if (file) {
// Should include both SPI & non-SPI.
file->getImportedModules(imports,
{ModuleDecl::ImportFilterKind::Default,
ModuleDecl::ImportFilterKind::ImplementationOnly,
Expand Down Expand Up @@ -259,7 +258,6 @@ ImportCache::getAllAccessPathsNotShadowedBy(const ModuleDecl *mod,
stack.emplace_back(ImportPath::Access(), currentMod);

if (auto *file = dyn_cast<FileUnit>(dc)) {
// Should include both SPI & non-SPI
file->getImportedModules(stack,
{ModuleDecl::ImportFilterKind::Default,
ModuleDecl::ImportFilterKind::ImplementationOnly,
Expand Down
34 changes: 13 additions & 21 deletions lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ void BuiltinUnit::LookupCache::lookupValue(
SmallVectorImpl<ValueDecl*> &Result) {
// Only qualified lookup ever finds anything in the builtin module.
if (LookupKind != NLKind::QualifiedLookup) return;

ValueDecl *&Entry = Cache[Name];
ASTContext &Ctx = M.getParentModule()->getASTContext();
if (!Entry) {
Expand Down Expand Up @@ -175,7 +175,7 @@ class swift::SourceLookupCache {

/// Throw away as much memory as possible.
void invalidate();

void lookupValue(DeclName Name, NLKind LookupKind,
SmallVectorImpl<ValueDecl*> &Result);

Expand Down Expand Up @@ -203,13 +203,13 @@ class swift::SourceLookupCache {
void lookupVisibleDecls(ImportPath::Access AccessPath,
VisibleDeclConsumer &Consumer,
NLKind LookupKind);

void populateMemberCache(const SourceFile &SF);
void populateMemberCache(const ModuleDecl &Mod);

void lookupClassMembers(ImportPath::Access AccessPath,
VisibleDeclConsumer &consumer);

void lookupClassMember(ImportPath::Access accessPath,
DeclName name,
SmallVectorImpl<ValueDecl*> &results);
Expand Down Expand Up @@ -331,7 +331,7 @@ void SourceLookupCache::lookupValue(DeclName Name, NLKind LookupKind,
SmallVectorImpl<ValueDecl*> &Result) {
auto I = TopLevelValues.find(Name);
if (I == TopLevelValues.end()) return;

Result.reserve(I->second.size());
for (ValueDecl *Elt : I->second)
Result.push_back(Elt);
Expand Down Expand Up @@ -398,7 +398,7 @@ void SourceLookupCache::lookupVisibleDecls(ImportPath::Access AccessPath,
void SourceLookupCache::lookupClassMembers(ImportPath::Access accessPath,
VisibleDeclConsumer &consumer) {
assert(accessPath.size() <= 1 && "can only refer to top-level decls");

if (!accessPath.empty()) {
for (auto &member : ClassMembers) {
// Non-simple names are also stored under their simple name, so make
Expand Down Expand Up @@ -432,11 +432,11 @@ void SourceLookupCache::lookupClassMember(ImportPath::Access accessPath,
DeclName name,
SmallVectorImpl<ValueDecl*> &results) {
assert(accessPath.size() <= 1 && "can only refer to top-level decls");

auto iter = ClassMembers.find(name);
if (iter == ClassMembers.end())
return;

if (!accessPath.empty()) {
for (ValueDecl *vd : iter->second) {
auto *nominal = vd->getDeclContext()->getSelfNominalTypeDecl();
Expand Down Expand Up @@ -1163,13 +1163,6 @@ void SourceFile::lookupPrecedenceGroupDirect(

void ModuleDecl::getImportedModules(SmallVectorImpl<ImportedModule> &modules,
ModuleDecl::ImportFilter filter) const {
assert(filter.containsAny(ImportFilter({
ModuleDecl::ImportFilterKind::Exported,
ModuleDecl::ImportFilterKind::Default,
ModuleDecl::ImportFilterKind::ImplementationOnly}))
&& "filter should have at least one of Exported|Private|ImplementationOnly"
);

FORWARD(getImportedModules, (modules, filter));
}

Expand All @@ -1194,12 +1187,11 @@ SourceFile::getImportedModules(SmallVectorImpl<ImportedModule> &modules,
requiredFilter |= ModuleDecl::ImportFilterKind::Exported;
else if (desc.options.contains(ImportFlags::ImplementationOnly))
requiredFilter |= ModuleDecl::ImportFilterKind::ImplementationOnly;
else if (desc.options.contains(ImportFlags::SPIAccessControl))
requiredFilter |= ModuleDecl::ImportFilterKind::SPIAccessControl;
else
requiredFilter |= ModuleDecl::ImportFilterKind::Default;

if (desc.options.contains(ImportFlags::SPIAccessControl))
requiredFilter |= ModuleDecl::ImportFilterKind::SPIAccessControl;

if (!separatelyImportedOverlays.lookup(desc.module.importedModule).empty())
requiredFilter |= ModuleDecl::ImportFilterKind::ShadowedByCrossImportOverlay;

Expand Down Expand Up @@ -2374,7 +2366,7 @@ bool FileUnit::walk(ASTWalker &walker) {
#ifndef NDEBUG
PrettyStackTraceDecl debugStack("walking into decl", D);
#endif

if (D->walk(walker))
return true;

Expand Down Expand Up @@ -2513,15 +2505,15 @@ SourceFile::lookupOpaqueResultType(StringRef MangledName) {
auto found = ValidatedOpaqueReturnTypes.find(MangledName);
if (found != ValidatedOpaqueReturnTypes.end())
return found->second;

// If there are unvalidated decls with opaque types, go through and validate
// them now.
(void) getOpaqueReturnTypeDecls();

found = ValidatedOpaqueReturnTypes.find(MangledName);
if (found != ValidatedOpaqueReturnTypes.end())
return found->second;

// Otherwise, we don't have a matching opaque decl.
return nullptr;
}
Expand Down
3 changes: 1 addition & 2 deletions lib/Frontend/ModuleInterfaceSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,7 @@ static void printImports(raw_ostream &out,

SmallVector<ImportedModule, 4> ioiImport;
M->getImportedModules(ioiImport,
{ModuleDecl::ImportFilterKind::ImplementationOnly,
ModuleDecl::ImportFilterKind::SPIAccessControl});
ModuleDecl::ImportFilterKind::ImplementationOnly);
ioiImportSet.insert(ioiImport.begin(), ioiImport.end());
}

Expand Down
6 changes: 1 addition & 5 deletions lib/Serialization/Serialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1055,11 +1055,7 @@ void Serializer::writeInputBlock(const SerializationOptions &options) {
ImportSet privateImportSet =
getImportsAsSet(M, ModuleDecl::ImportFilterKind::Default);
ImportSet spiImportSet =
getImportsAsSet(M, {
ModuleDecl::ImportFilterKind::Exported,
ModuleDecl::ImportFilterKind::Default,
ModuleDecl::ImportFilterKind::SPIAccessControl
});
getImportsAsSet(M, ModuleDecl::ImportFilterKind::SPIAccessControl);

auto clangImporter =
static_cast<ClangImporter *>(M->getASTContext().getClangModuleLoader());
Expand Down