Skip to content

Commit

Permalink
AST: Only apply MemberImportVisibility to lookups from the main module.
Browse files Browse the repository at this point in the history
MemberImportVisibility rules should only apply to source code in the main
module. The rules were being applied when resolving witnesses for synthesized
Hashable conformances on CF types imported by ClangImporter, which caused the
lookups to fail and bad conformances to be generated.

Resolves swiftlang#78870 and rdar://142433039.
  • Loading branch information
tshortli committed Jan 24, 2025
1 parent 95dca81 commit 5f56847
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 13 deletions.
39 changes: 26 additions & 13 deletions lib/AST/NameLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2340,22 +2340,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 @@ -2383,10 +2388,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;
}

// Check that it has the appropriate ABI role.
if (!ABIRoleInfo(decl).matchesOptions(options))
Expand Down Expand Up @@ -2594,6 +2598,7 @@ QualifiedLookupRequest::evaluate(Evaluator &eval, const DeclContext *DC,
DeclNameRef member, NLOptions options) const {
using namespace namelookup;
QualifiedLookupResult decls;
auto &ctx = DC->getASTContext();

// Tracking for the nominal types we'll visit.
SmallVector<NominalTypeDecl *, 4> stack;
Expand All @@ -2620,6 +2625,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 @@ -2654,7 +2662,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 @@ -2792,6 +2801,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 @@ -2824,7 +2836,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
1 change: 1 addition & 0 deletions test/ClangImporter/Security_test.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck %s -verify
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck %s -enable-upcoming-feature MemberImportVisibility -verify

// REQUIRES: objc_interop

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)
}

0 comments on commit 5f56847

Please sign in to comment.