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 #78870 and rdar://142433039.

Author: tshortli

lib/AST/NameLookup.cpp

@@ -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)) {
@@ -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;
 }
@@ -2567,6 +2571,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;
@@ -2593,6 +2598,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);
@@ -2625,7 +2633,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);
     }
 
@@ -2762,6 +2771,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;
@@ -2794,7 +2806,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);
   }
 

test/ClangImporter/Security_test.swift

@@ -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
 

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