Skip to content

[6.1] AST: Only apply MemberImportVisibility to lookups from the main module #78877

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
38 changes: 25 additions & 13 deletions lib/AST/NameLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2330,22 +2330,27 @@ ObjCCategoryNameMap ClassDecl::getObjCCategoryNameMap() {
ObjCCategoryNameMap());
}

static bool missingImportForMemberDecl(const DeclContext *dc, ValueDecl *decl) {
// Only require explicit imports for members when MemberImportVisibility is
// enabled.
auto &ctx = dc->getASTContext();
/// Determines whether MemberImportVisiblity should be enforced for lookups in
/// the given context.
static bool shouldRequireImportsInContext(const DeclContext *lookupContext) {
auto &ctx = lookupContext->getASTContext();
if (!ctx.LangOpts.hasFeature(Feature::MemberImportVisibility))
return false;

return !dc->isDeclImported(decl);
// Code outside of the main module (which is often synthesized) isn't subject
// to MemberImportVisibility rules.
if (lookupContext->getParentModule() != ctx.MainModule)
return false;

return true;
}

/// Determine whether the given declaration is an acceptable lookup
/// result when searching from the given DeclContext.
static bool isAcceptableLookupResult(const DeclContext *dc,
NLOptions options,
static bool isAcceptableLookupResult(const DeclContext *dc, NLOptions options,
ValueDecl *decl,
bool onlyCompleteObjectInits) {
bool onlyCompleteObjectInits,
bool requireImport) {
// Filter out designated initializers, if requested.
if (onlyCompleteObjectInits) {
if (auto ctor = dyn_cast<ConstructorDecl>(decl)) {
Expand Down Expand Up @@ -2373,10 +2378,9 @@ static bool isAcceptableLookupResult(const DeclContext *dc,

// Check that there is some import in the originating context that makes this
// decl visible.
if (!(options & NL_IgnoreMissingImports)) {
if (missingImportForMemberDecl(dc, decl))
if (requireImport && !(options & NL_IgnoreMissingImports))
if (!dc->isDeclImported(decl))
return false;
}

return true;
}
Expand Down Expand Up @@ -2593,6 +2597,9 @@ QualifiedLookupRequest::evaluate(Evaluator &eval, const DeclContext *DC,
// Whether we only want to return complete object initializers.
bool onlyCompleteObjectInits = false;

// Whether to enforce MemberImportVisibility import restrictions.
bool requireImport = shouldRequireImportsInContext(DC);

// Visit all of the nominal types we know about, discovering any others
// we need along the way.
bool wantProtocolMembers = (options & NL_ProtocolMembers);
Expand Down Expand Up @@ -2625,7 +2632,8 @@ QualifiedLookupRequest::evaluate(Evaluator &eval, const DeclContext *DC,
if ((options & NL_OnlyMacros) && !isa<MacroDecl>(decl))
continue;

if (isAcceptableLookupResult(DC, options, decl, onlyCompleteObjectInits))
if (isAcceptableLookupResult(DC, options, decl, onlyCompleteObjectInits,
requireImport))
decls.push_back(decl);
}

Expand Down Expand Up @@ -2762,6 +2770,9 @@ AnyObjectLookupRequest::evaluate(Evaluator &evaluator, const DeclContext *dc,
member.getFullName(), allDecls);
}

/// Whether to enforce MemberImportVisibility import restrictions.
bool requireImport = shouldRequireImportsInContext(dc);

// For each declaration whose context is not something we've
// already visited above, add it to the list of declarations.
llvm::SmallPtrSet<ValueDecl *, 4> knownDecls;
Expand Down Expand Up @@ -2794,7 +2805,8 @@ AnyObjectLookupRequest::evaluate(Evaluator &evaluator, const DeclContext *dc,
// result, add it to the list.
if (knownDecls.insert(decl).second &&
isAcceptableLookupResult(dc, options, decl,
/*onlyCompleteObjectInits=*/false))
/*onlyCompleteObjectInits=*/false,
requireImport))
decls.push_back(decl);
}

Expand Down
23 changes: 23 additions & 0 deletions validation-test/ClangImporter/cfobject-conformances.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// RUN: %target-swift-frontend -emit-sil %s -verify
// RUN: %target-swift-frontend -emit-sil %s -enable-upcoming-feature MemberImportVisibility -verify

// REQUIRES: VENDOR=apple
// REQUIRES: swift_feature_MemberImportVisibility

import CoreFoundation

public func takesHashable<T: Hashable>(_ t: T) {}

public func takesCFObjects(
_ string: CFString,
_ number: CFNumber,
_ date: CFDate,
_ data: CFData,
_ set: CFSet,
) {
takesHashable(string)
takesHashable(number)
takesHashable(date)
takesHashable(data)
takesHashable(set)
}