From 0b0d2e912c147b5f58a6f77a04ba57af731d73b8 Mon Sep 17 00:00:00 2001
From: Allan Shortlidge <shortlidge@apple.com>
Date: Thu, 23 Jan 2025 19:27:56 -0800
Subject: [PATCH] AST: Only apply MemberImportVisibility to lookups from the
 main module.

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 https://github.com/swiftlang/swift/issues/78870 and rdar://142433039.
---
 lib/AST/NameLookup.cpp                        | 38 ++++++++++++-------
 .../ClangImporter/cfobject-conformances.swift | 23 +++++++++++
 2 files changed, 48 insertions(+), 13 deletions(-)
 create mode 100644 validation-test/ClangImporter/cfobject-conformances.swift

diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp
index 90511ae6ed7..f5e7e36b79c 100644
--- a/lib/AST/NameLookup.cpp
+++ b/lib/AST/NameLookup.cpp
@@ -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)) {
@@ -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))
@@ -2620,6 +2624,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);
@@ -2654,7 +2661,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);
     }
 
@@ -2792,6 +2800,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;
@@ -2824,7 +2835,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);
   }
 
diff --git a/validation-test/ClangImporter/cfobject-conformances.swift b/validation-test/ClangImporter/cfobject-conformances.swift
new file mode 100644
index 00000000000..41f038f79fc
--- /dev/null
+++ b/validation-test/ClangImporter/cfobject-conformances.swift
@@ -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)
+}