don't use Clang modules in the "only re-export public symbols" check (swiftlang#66610)

rdar://110399757

Author: QuietMisdreavus

lib/SymbolGraphGen/SymbolGraph.cpp

@@ -718,7 +718,10 @@ bool SymbolGraph::isImplicitlyPrivate(const Decl *D,
 
     // Symbols from exported-imported modules should only be included if they
     // were originally public.
-    if (Walker.isFromExportedImportedModule(D) &&
+    // We force compiler-equality here to ensure that the presence of an underlying
+    // Clang module does not prevent internal Swift symbols from being emitted when
+    // MinimumAccessLevel is set to `internal` or below.
+    if (Walker.isFromExportedImportedModule(D, /*countUnderlyingClangModule*/false) &&
         VD->getFormalAccess() < AccessLevel::Public) {
       return true;
     }

lib/SymbolGraphGen/SymbolGraphASTWalker.cpp

@@ -28,8 +28,17 @@ namespace {
 ///
 /// This does a by-name comparison to consider a module's underlying Clang module to be equivalent
 /// to the wrapping module of the same name.
-bool areModulesEqual(const ModuleDecl *lhs, const ModuleDecl *rhs) {
-  return lhs->getNameStr() == rhs->getNameStr();
+///
+/// If the `isClangEqual` argument is set to `false`, the modules must also be from the same
+/// compiler, i.e. a Swift module and its underlying Clang module would be considered not equal.
+bool areModulesEqual(const ModuleDecl *lhs, const ModuleDecl *rhs, bool isClangEqual = true) {
+  if (lhs->getNameStr() != rhs->getNameStr())
+    return false;
+
+  if (!isClangEqual && (lhs->isNonSwiftModule() != rhs->isNonSwiftModule()))
+    return false;
+
+  return true;
 }
 
 } // anonymous namespace
@@ -303,9 +312,9 @@ bool SymbolGraphASTWalker::isConsideredExportedImported(const Decl *D) const {
   return false;
 }
 
-bool SymbolGraphASTWalker::isFromExportedImportedModule(const Decl* D) const {
+bool SymbolGraphASTWalker::isFromExportedImportedModule(const Decl* D, bool countUnderlyingClangModule) const {
   auto *M = D->getModuleContext();
-  return isQualifiedExportedImport(D) || isExportedImportedModule(M);
+  return isQualifiedExportedImport(D) || isExportedImportedModule(M, countUnderlyingClangModule);
 }
 
 bool SymbolGraphASTWalker::isQualifiedExportedImport(const Decl *D) const {
@@ -314,9 +323,9 @@ bool SymbolGraphASTWalker::isQualifiedExportedImport(const Decl *D) const {
   });
 }
 
-bool SymbolGraphASTWalker::isExportedImportedModule(const ModuleDecl *M) const {
-  return llvm::any_of(ExportedImportedModules, [&M](const auto *MD) {
-    return areModulesEqual(M, MD->getModuleContext());
+bool SymbolGraphASTWalker::isExportedImportedModule(const ModuleDecl *M, bool countUnderlyingClangModule) const {
+  return llvm::any_of(ExportedImportedModules, [&M, countUnderlyingClangModule](const auto *MD) {
+    return areModulesEqual(M, MD->getModuleContext(), /*isClangEqual*/countUnderlyingClangModule);
   });
 }
 

lib/SymbolGraphGen/SymbolGraphASTWalker.h

@@ -103,13 +103,19 @@ struct SymbolGraphASTWalker : public SourceEntityWalker {
   virtual bool isConsideredExportedImported(const Decl *D) const;
 
   /// Returns whether the given declaration comes from an `@_exported import` module.
-  virtual bool isFromExportedImportedModule(const Decl *D) const;
+  ///
+  /// If `countUnderlyingClangModule` is `false`, decls from Clang modules will not be considered
+  /// re-exported unless the Clang module was itself directly re-exported.
+  virtual bool isFromExportedImportedModule(const Decl *D, bool countUnderlyingClangModule = true) const;
 
   /// Returns whether the given declaration was imported via an `@_exported import <type>` declaration.
   virtual bool isQualifiedExportedImport(const Decl *D) const;
 
   /// Returns whether the given module is an `@_exported import` module.
-  virtual bool isExportedImportedModule(const ModuleDecl *M) const;
+  ///
+  /// If `countUnderlyingClangModule` is `false`, Clang modules will not be considered re-exported
+  /// unless the Clang module itself was directly re-exported.
+  virtual bool isExportedImportedModule(const ModuleDecl *M, bool countUnderlyingClangModule = true) const;
 
   /// Returns whether the given module is the main module, or is an `@_exported import` module.
   virtual bool isOurModule(const ModuleDecl *M) const;

test/SymbolGraph/ClangImporter/MinimumAccessLevel.swift

@@ -0,0 +1,20 @@
+// RUN: %empty-directory(%t)
+// RUN: cp -r %S/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework %t
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -enable-objc-interop -emit-module-path %t/EmitWhileBuilding.framework/Modules/EmitWhileBuilding.swiftmodule/%target-swiftmodule-name -import-underlying-module -F %t -module-name EmitWhileBuilding -disable-objc-attr-requires-foundation-module %s %S/Inputs/EmitWhileBuilding/Extra.swift -emit-symbol-graph -emit-symbol-graph-dir %t -symbol-graph-minimum-access-level internal
+// RUN: %FileCheck %s --input-file %t/EmitWhileBuilding.symbols.json
+// RUN: %{python} -c 'import os.path; import sys; sys.exit(1 if os.path.exists(sys.argv[1]) else 0)' %t/[email protected]
+
+// RUN: %target-swift-symbolgraph-extract -sdk %clang-importer-sdk -module-name EmitWhileBuilding -F %t -output-dir %t -pretty-print -v -minimum-access-level internal
+// RUN: %FileCheck %s --input-file %t/EmitWhileBuilding.symbols.json
+// RUN: %{python} -c 'import os.path; import sys; sys.exit(1 if os.path.exists(sys.argv[1]) else 0)' %t/[email protected]
+
+// REQUIRES: objc_interop
+
+// Ensure that having an underlying Clang module does not override the
+// `-symbol-graph-minimum-access-level` flag (rdar://110399757)
+
+// CHECK: "s:17EmitWhileBuilding9innerFuncSSyF"
+
+internal func innerFunc() -> String { "sup" }
+
+public func someFunc() -> String { innerFunc() }