Fix SR-13785

Author: scentini

lib/Sema/ResilienceDiagnostics.cpp

@@ -21,6 +21,7 @@
 #include "swift/AST/Decl.h"
 #include "swift/AST/DeclContext.h"
 #include "swift/AST/Initializer.h"
+#include "swift/AST/ModuleNameLookup.h"
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/AST/SourceFile.h"
 #include "swift/AST/TypeDeclFinder.h"
@@ -160,6 +161,34 @@ static bool diagnoseDeclExportability(SourceLoc loc, const ValueDecl *D,
   if (originKind == DisallowedOriginKind::None)
     return false;
 
+  // Even if the current module is @_implementationOnly, Swift should
+  // not report an error in the cases where the symbol is also exported from
+  // a non @_implementationOnly module. Thus, we look at all the imported
+  // modules and see if we can find the symbol in a non @_implementationOnly
+  // module.
+  SmallVector<ImportedModule, 4> importedModules;
+  userSF.getImportedModules(
+      importedModules,
+      ModuleDecl::ImportFilter(
+          {ModuleDecl::ImportFilterKind::Exported,
+           ModuleDecl::ImportFilterKind::Default,
+           ModuleDecl::ImportFilterKind::SPIAccessControl,
+           ModuleDecl::ImportFilterKind::ShadowedByCrossImportOverlay}));
+  auto nlOptions = NL_UnqualifiedDefault | NL_IncludeUsableFromInline;
+
+  for (auto &importedModule : importedModules) {
+    SmallVector<ValueDecl *, 4> decls;
+    namelookup::lookupInModule(importedModule.importedModule, D->getName(),
+                               decls, NLKind::UnqualifiedLookup,
+                               namelookup::ResolutionKind::Overloadable,
+                               importedModule.importedModule, nlOptions);
+
+    for (auto *decl : decls) {
+      if (decl->getFormalAccess() >= AccessLevel::Public) {
+        return false;
+      }
+    }
+  }
   // TODO: different diagnostics
   ASTContext &ctx = definingModule->getASTContext();
   ctx.Diags.diagnose(loc, diag::inlinable_decl_ref_from_hidden_module,

test/Interop/C/implementation-only-imports/Inputs/use-module-a.swift

@@ -0,0 +1 @@
+@_exported import UserA

test/Interop/C/implementation-only-imports/Inputs/use-module-b.swift

@@ -0,0 +1 @@
+@_exported import UserB

test/Interop/C/implementation-only-imports/lookup-visible-symbols-recursively-inversed.swift

@@ -0,0 +1,26 @@
+// RUN: %empty-directory(%t)
+// RUN: mkdir %t/use_module_a %t/use_module_b
+// RUN: %target-swift-frontend -enable-library-evolution -swift-version 5 -emit-module -o %t/use_module_a/UseModuleA.swiftmodule %S/Inputs/use-module-a.swift -I %S/Inputs -enable-cxx-interop
+// RUN: %target-swift-frontend -enable-library-evolution -swift-version 5 -emit-module -o %t/use_module_b/UseModuleB.swiftmodule %S/Inputs/use-module-b.swift -I %S/Inputs -enable-cxx-interop
+
+// RUN: %target-swift-frontend -typecheck -swift-version 5 -I %t/use_module_a -I %t/use_module_b -I %S/Inputs -enable-cxx-interop %s
+
+
+// If a symbol comes from two modules, one of which is marked as
+// @_implementationOnly, Swift may choose the @_implementationOnly source
+// and then error out due to the symbol being hidden.
+
+// Swift should consider all sources for the symbol and recognize that the
+// symbol is not hidden behind @_implementationOnly in all modules.
+
+// This test, as well as the `lookup-visible-symbols-recursively.swift` check
+// that the `getFortyTwo` symbol can be found when at least one of the
+// modules is not `@_implementationOnly`.
+
+@_implementationOnly import UseModuleA
+import UseModuleB
+
+@inlinable
+public func callFortyTwo() -> CInt {
+    return getFortyTwo()
+}

test/Interop/C/implementation-only-imports/lookup-visible-symbols-recursively.swift

@@ -0,0 +1,26 @@
+// RUN: %empty-directory(%t)
+// RUN: mkdir %t/use_module_a %t/use_module_b
+// RUN: %target-swift-frontend -enable-library-evolution -swift-version 5 -emit-module -o %t/use_module_a/UseModuleA.swiftmodule %S/Inputs/use-module-a.swift -I %S/Inputs -enable-cxx-interop
+// RUN: %target-swift-frontend -enable-library-evolution -swift-version 5 -emit-module -o %t/use_module_b/UseModuleB.swiftmodule %S/Inputs/use-module-b.swift -I %S/Inputs -enable-cxx-interop
+
+// RUN: %target-swift-frontend -typecheck -swift-version 5 -I %t/use_module_a -I %t/use_module_b -I %S/Inputs -enable-cxx-interop %s
+
+
+// If a symbol comes from two modules, one of which is marked as
+// @_implementationOnly, Swift may choose the @_implementationOnly source
+// and then error out due to the symbol being hidden.
+
+// Swift should consider all sources for the symbol and recognize that the
+// symbol is not hidden behind @_implementationOnly in all modules.
+
+// This test, as well as the `lookup-visible-symbols-recursively-inversed.swift
+// check that the `getFortyTwo` symbol can be found when at least one of the
+// modules is not `@_implementationOnly`.
+
+import UseModuleA
+@_implementationOnly import UseModuleB
+
+@inlinable
+public func callFortyTwo() -> CInt {
+    return getFortyTwo()
+}

test/Interop/C/implementation-only-imports/prefer-a-visible-symbol-over-implementation-only-ones-inversed.swift

@@ -0,0 +1,22 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -o %t/FortyTwo.swiftmodule -I %S/Inputs %s
+
+// If a symbol comes from two modules, one of which is marked as
+// @_implementationOnly, Swift may choose the @_implementationOnly source
+// and then error out due to the symbol being hidden.
+
+// Swift should consider all sources for the symbol and recognize that the
+// symbol is not hidden behind @_implementationOnly in all modules.
+
+// In this test case, UserA and UserB both textually include `helper.h`,
+// therefore both export `getFortyTwo()`.
+// This test verifies that even if Swift chooses UserB.getFortyTwo(), it
+// doesn't error out, because the symbol is also exported from UserA.
+
+import UserA
+@_implementationOnly import UserB
+
+@_inlineable
+public func callFortyTwo() -> CInt {
+    return getFortyTwo()
+}

test/Interop/C/implementation-only-imports/prefer-a-visible-symbol-over-implementation-only-ones.swift

@@ -1,22 +1,17 @@
 // RUN: %empty-directory(%t)
 // RUN: %target-swift-frontend -emit-module -o %t/FortyTwo.swiftmodule -I %S/Inputs %s
 
-// REQUIRES: SR-13785
-
-// TODO: Fix @_implementationOnly to consider all symbol sources
-
 // If a symbol comes from two modules, one of which is marked as
 // @_implementationOnly, Swift may choose the @_implementationOnly source
 // and then error out due to the symbol being hidden.
 
 // Swift should consider all sources for the symbol and recognize that the
 // symbol is not hidden behind @_implementationOnly in all modules.
 
-// E.g:
 // In this test case, UserA and UserB both textually include `helper.h`,
 // therefore both export `getFortyTwo()`.
-// This test verifies that even though Swift chooses UserA.getFortyTwo(), we
-// shouldn't get an error, because the symbol is also exported from UserB.
+// This test verifies that even if Swift chooses UserA.getFortyTwo(), it
+// doesn't error out, because the symbol is also exported from UserB.
 
 @_implementationOnly import UserA
 import UserB