[5.6][AST] scan @_exported imports of source files for display decls

include/swift/AST/FileUnit.h

@@ -16,6 +16,7 @@
 #include "swift/AST/Module.h"
 #include "swift/AST/RawComment.h"
 #include "swift/Basic/BasicSourceInfo.h"
+#include "swift/Basic/Debug.h"
 
 #include "llvm/ADT/PointerIntPair.h"
 
@@ -188,6 +189,8 @@ class FileUnit : public DeclContext, public ASTAllocated<FileUnit> {
   virtual Identifier
   getDiscriminatorForPrivateValue(const ValueDecl *D) const = 0;
 
+  virtual bool shouldCollectDisplayDecls() const { return true; }
+
   /// Finds all top-level decls in this file.
   ///
   /// This does a simple local lookup, not recursively looking through imports.
@@ -243,7 +246,7 @@ class FileUnit : public DeclContext, public ASTAllocated<FileUnit> {
   ///
   /// This can differ from \c getTopLevelDecls, e.g. it returns decls from a
   /// shadowed clang module.
-  virtual void getDisplayDecls(SmallVectorImpl<Decl*> &results) const {
+  virtual void getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recursive = false) const {
     getTopLevelDecls(results);
   }
 
@@ -308,6 +311,9 @@ class FileUnit : public DeclContext, public ASTAllocated<FileUnit> {
     return getParentModule()->getRealName().str();
   }
 
+  SWIFT_DEBUG_DUMPER(dumpDisplayDecls());
+  SWIFT_DEBUG_DUMPER(dumpTopLevelDecls());
+
   /// Traverse the decls within this file.
   ///
   /// \returns true if traversal was aborted, false if it completed

include/swift/AST/Module.h

@@ -26,6 +26,7 @@
 #include "swift/AST/Type.h"
 #include "swift/Basic/BasicSourceInfo.h"
 #include "swift/Basic/Compiler.h"
+#include "swift/Basic/Debug.h"
 #include "swift/Basic/OptionSet.h"
 #include "swift/Basic/STLExtras.h"
 #include "swift/Basic/SourceLoc.h"
@@ -759,6 +760,15 @@ class ModuleDecl
   /// The order of the results is not guaranteed to be meaningful.
   void getPrecedenceGroups(SmallVectorImpl<PrecedenceGroupDecl*> &Results) const;
 
+  /// Determines whether this module should be recursed into when calling
+  /// \c getDisplayDecls.
+  ///
+  /// Some modules should not call \c getDisplayDecls, due to assertions
+  /// in their implementation. These are usually implicit imports that would be
+  /// recursed into for parsed modules. This function provides a guard against
+  /// recusing into modules that should not have decls collected.
+  bool shouldCollectDisplayDecls() const;
+
   /// Finds all top-level decls that should be displayed to a client of this
   /// module.
   ///
@@ -770,7 +780,7 @@ class ModuleDecl
   /// shadowed clang module. It does not force synthesized top-level decls that
   /// should be printed to be added; use \c swift::getTopLevelDeclsForDisplay()
   /// for that.
-  void getDisplayDecls(SmallVectorImpl<Decl*> &results) const;
+  void getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recursive = false) const;
 
   using LinkLibraryCallback = llvm::function_ref<void(LinkLibrary)>;
 
@@ -856,6 +866,9 @@ class ModuleDecl
   /// transferred from module files to the dSYMs, remove this.
   bool isExternallyConsumed() const;
 
+  SWIFT_DEBUG_DUMPER(dumpDisplayDecls());
+  SWIFT_DEBUG_DUMPER(dumpTopLevelDecls());
+
   SourceRange getSourceRange() const { return SourceRange(); }
 
   static bool classof(const DeclContext *DC) {

include/swift/AST/SourceFile.h

@@ -288,6 +288,10 @@ class SourceFile final : public FileUnit {
 
   ~SourceFile();
 
+  bool hasImports() const {
+    return Imports.hasValue();
+  }
+
   /// Retrieve an immutable view of the source file's imports.
   ArrayRef<AttributedImport<ImportedModule>> getImports() const {
     return *Imports;

include/swift/ClangImporter/ClangModule.h

@@ -87,9 +87,11 @@ class ClangModuleUnit final : public LoadedFile {
          ObjCSelector selector,
          SmallVectorImpl<AbstractFunctionDecl *> &results) const override;
 
+  virtual bool shouldCollectDisplayDecls() const override;
+
   virtual void getTopLevelDecls(SmallVectorImpl<Decl*> &results) const override;
 
-  virtual void getDisplayDecls(SmallVectorImpl<Decl*> &results) const override;
+  virtual void getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recursive = false) const override;
 
   virtual void
   getImportedModules(SmallVectorImpl<ImportedModule> &imports,

include/swift/Sema/IDETypeChecking.h

@@ -145,7 +145,7 @@ namespace swift {
   /// \c ModuleDecl::getDisplayDecls() would only return if previous
   /// work happened to have synthesized them.
   void
-  getTopLevelDeclsForDisplay(ModuleDecl *M, SmallVectorImpl<Decl*> &Results);
+  getTopLevelDeclsForDisplay(ModuleDecl *M, SmallVectorImpl<Decl*> &Results, bool Recursive = false);
 
   struct ExtensionInfo {
     // The extension with the declarations to apply.

include/swift/Serialization/SerializedModuleLoader.h

@@ -433,7 +433,7 @@ class SerializedASTFile final : public LoadedFile {
   virtual void
   getOpaqueReturnTypeDecls(SmallVectorImpl<OpaqueTypeDecl*> &results) const override;
 
-  virtual void getDisplayDecls(SmallVectorImpl<Decl*> &results) const override;
+  virtual void getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recursive = false) const override;
 
   virtual void
   getImportedModules(SmallVectorImpl<ImportedModule> &imports,

lib/AST/Module.cpp

@@ -780,6 +780,30 @@ void SourceFile::lookupObjCMethods(
   results.append(known->second.begin(), known->second.end());
 }
 
+bool ModuleDecl::shouldCollectDisplayDecls() const {
+  for (const FileUnit *file : Files) {
+    if (!file->shouldCollectDisplayDecls())
+      return false;
+  }
+  return true;
+}
+
+static void collectParsedExportedImports(const ModuleDecl *M, SmallPtrSetImpl<ModuleDecl *> &Imports) {
+  for (const FileUnit *file : M->getFiles()) {
+    if (const SourceFile *source = dyn_cast<SourceFile>(file)) {
+      if (source->hasImports()) {
+        for (auto import : source->getImports()) {
+          if (import.options.contains(ImportFlags::Exported) &&
+              !Imports.contains(import.module.importedModule) &&
+              import.module.importedModule->shouldCollectDisplayDecls()) {
+            Imports.insert(import.module.importedModule);
+          }
+        }
+      }
+    }
+  }
+}
+
 void ModuleDecl::getLocalTypeDecls(SmallVectorImpl<TypeDecl*> &Results) const {
   FORWARD(getLocalTypeDecls, (Results));
 }
@@ -788,6 +812,24 @@ void ModuleDecl::getTopLevelDecls(SmallVectorImpl<Decl*> &Results) const {
   FORWARD(getTopLevelDecls, (Results));
 }
 
+void ModuleDecl::dumpDisplayDecls() const {
+  SmallVector<Decl *, 32> Decls;
+  getDisplayDecls(Decls);
+  for (auto *D : Decls) {
+    D->dump(llvm::errs());
+    llvm::errs() << "\n";
+  }
+}
+
+void ModuleDecl::dumpTopLevelDecls() const {
+  SmallVector<Decl *, 32> Decls;
+  getTopLevelDecls(Decls);
+  for (auto *D : Decls) {
+    D->dump(llvm::errs());
+    llvm::errs() << "\n";
+  }
+}
+
 void ModuleDecl::getExportedPrespecializations(
     SmallVectorImpl<Decl *> &Results) const {
   FORWARD(getExportedPrespecializations, (Results));
@@ -907,9 +949,34 @@ SourceFile::getExternalRawLocsForDecl(const Decl *D) const {
   return Result;
 }
 
-void ModuleDecl::getDisplayDecls(SmallVectorImpl<Decl*> &Results) const {
+void ModuleDecl::getDisplayDecls(SmallVectorImpl<Decl*> &Results, bool Recursive) const {
+  if (Recursive && isParsedModule(this)) {
+    SmallPtrSet<ModuleDecl *, 4> Modules;
+    collectParsedExportedImports(this, Modules);
+    for (const ModuleDecl *import : Modules) {
+      import->getDisplayDecls(Results, Recursive);
+    }
+  }
   // FIXME: Should this do extra access control filtering?
   FORWARD(getDisplayDecls, (Results));
+
+#ifndef NDEBUG
+  if (Recursive) {
+    llvm::DenseSet<Decl *> visited;
+    for (auto *D : Results) {
+      // decls synthesized from implicit clang decls may appear multiple times;
+      // e.g. if multiple modules with underlying clang modules are re-exported.
+      // including duplicates of these is harmless, so skip them when counting
+      // this assertion
+      if (const auto *CD = D->getClangDecl()) {
+        if (CD->isImplicit()) continue;
+      }
+
+      auto inserted = visited.insert(D).second;
+      assert(inserted && "there should be no duplicate decls");
+    }
+  }
+#endif
 }
 
 ProtocolConformanceRef
@@ -3065,6 +3132,22 @@ void FileUnit::getTopLevelDeclsWhereAttributesMatch(
   Results.erase(newEnd, Results.end());
 }
 
+void FileUnit::dumpDisplayDecls() const {
+  SmallVector<Decl *, 32> Decls;
+  getDisplayDecls(Decls);
+  for (auto *D : Decls) {
+    D->dump(llvm::errs());
+  }
+}
+
+void FileUnit::dumpTopLevelDecls() const {
+  SmallVector<Decl *, 32> Decls;
+  getTopLevelDecls(Decls);
+  for (auto *D : Decls) {
+    D->dump(llvm::errs());
+  }
+}
+
 void swift::simple_display(llvm::raw_ostream &out, const FileUnit *file) {
   if (!file) {
     out << "(null)";

lib/ClangImporter/ClangImporter.cpp

@@ -2998,6 +2998,9 @@ class VectorDeclPtrConsumer : public swift::VisibleDeclConsumer {
 };
 } // unnamed namespace
 
+// FIXME: should submodules still be crawled for the symbol graph? (SR-15753)
+bool ClangModuleUnit::shouldCollectDisplayDecls() const { return isTopLevel(); }
+
 void ClangModuleUnit::getTopLevelDecls(SmallVectorImpl<Decl*> &results) const {
   VectorDeclPtrConsumer consumer(results);
   FilteringDeclaredDeclConsumer filterConsumer(consumer, this);
@@ -3118,7 +3121,7 @@ static void getImportDecls(ClangModuleUnit *ClangUnit, const clang::Module *M,
   }
 }
 
-void ClangModuleUnit::getDisplayDecls(SmallVectorImpl<Decl*> &results) const {
+void ClangModuleUnit::getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recursive) const {
   if (clangModule)
     getImportDecls(const_cast<ClangModuleUnit *>(this), clangModule, results);
   getTopLevelDecls(results);

lib/ClangImporter/DWARFImporter.cpp

@@ -64,7 +64,7 @@ class DWARFModuleUnit final : public LoadedFile {
   getTopLevelDecls(SmallVectorImpl<Decl *> &results) const override {}
 
   virtual void
-  getDisplayDecls(SmallVectorImpl<Decl *> &results) const override {}
+  getDisplayDecls(SmallVectorImpl<Decl *> &results, bool recursive = false) const override {}
 
   virtual void
   getImportedModules(SmallVectorImpl<ImportedModule> &imports,

lib/IDE/IDETypeChecking.cpp

@@ -35,9 +35,10 @@ using namespace swift;
 
 void
 swift::getTopLevelDeclsForDisplay(ModuleDecl *M,
-                                  SmallVectorImpl<Decl*> &Results) {
+                                  SmallVectorImpl<Decl*> &Results,
+                                  bool Recursive) {
   auto startingSize = Results.size();
-  M->getDisplayDecls(Results);
+  M->getDisplayDecls(Results, Recursive);
 
   // Force Sendable on all types, which might synthesize some extensions.
   // FIXME: We can remove this if @_nonSendable stops creating extensions.
@@ -49,7 +50,7 @@ swift::getTopLevelDeclsForDisplay(ModuleDecl *M,
   // Remove what we fetched and fetch again, possibly now with additional
   // extensions.
   Results.resize(startingSize);
-  M->getDisplayDecls(Results);
+  M->getDisplayDecls(Results, Recursive);
 }
 
 static bool shouldPrintAsFavorable(const Decl *D, const PrintOptions &Options) {

lib/Serialization/ModuleFile.cpp

@@ -973,9 +973,9 @@ ModuleFile::getOpaqueReturnTypeDecls(SmallVectorImpl<OpaqueTypeDecl *> &results)
   }
 }
 
-void ModuleFile::getDisplayDecls(SmallVectorImpl<Decl *> &results) {
+void ModuleFile::getDisplayDecls(SmallVectorImpl<Decl *> &results, bool recursive) {
   if (UnderlyingModule)
-    UnderlyingModule->getDisplayDecls(results);
+    UnderlyingModule->getDisplayDecls(results, recursive);
 
   PrettyStackTraceModuleFile stackEntry(*this);
   getImportDecls(results);

lib/Serialization/ModuleFile.h

@@ -672,7 +672,7 @@ class ModuleFile
   /// This includes all decls that should be displayed to clients of the module.
   /// This can differ from \c getTopLevelDecls, e.g. it returns decls from a
   /// shadowed clang module.
-  void getDisplayDecls(SmallVectorImpl<Decl*> &results);
+  void getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recursive = false);
 
   StringRef getModuleFilename() const {
     if (!Core->ModuleInterfacePath.empty())

lib/Serialization/SerializedModuleLoader.cpp

@@ -1526,8 +1526,8 @@ SerializedASTFile::getOpaqueReturnTypeDecls(
 }
 
 void
-SerializedASTFile::getDisplayDecls(SmallVectorImpl<Decl*> &results) const {
-  File.getDisplayDecls(results);
+SerializedASTFile::getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recursive) const {
+  File.getDisplayDecls(results, recursive);
 }
 
 StringRef SerializedASTFile::getFilename() const {

lib/SymbolGraphGen/SymbolGraphGen.cpp

@@ -57,7 +57,7 @@ symbolgraphgen::emitSymbolGraphForModule(ModuleDecl *M,
                                          const SymbolGraphOptions &Options) {
   SymbolGraphASTWalker Walker(*M, Options);
   SmallVector<Decl *, 64> ModuleDecls;
-  swift::getTopLevelDeclsForDisplay(M, ModuleDecls);
+  swift::getTopLevelDeclsForDisplay(M, ModuleDecls, /*recursive*/true);
 
   if (Options.PrintMessages)
     llvm::errs() << ModuleDecls.size()

test/ModuleInterface/exported-import.swift

@@ -0,0 +1,12 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -swift-version 5 -enable-library-evolution -emit-module-path %t/Other.swiftmodule -module-name Other %S/Inputs/other.swift
+// RUN: %target-swift-frontend -swift-version 5 -enable-library-evolution -emit-module-path /dev/null -emit-module-interface-path %t/ExportedImport.swiftmodule -module-name ExportedImport %s -I %t
+
+// RUN: %FileCheck --input-file %t/ExportedImport.swiftmodule %s
+
+// CHECK-NOT: otherFileFunction
+
+@_exported import Other
+
+// CHECK: public struct SomeStruct
+public struct SomeStruct {}

test/SymbolGraph/ClangImporter/EmitWhileBuilding.swift

@@ -1,15 +1,19 @@
 // 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 -emit-symbol-graph -emit-symbol-graph-dir %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
 // RUN: %{python} -m json.tool %t/EmitWhileBuilding.symbols.json %t/EmitWhileBuilding.formatted.symbols.json
 // RUN: %FileCheck %s --input-file %t/EmitWhileBuilding.formatted.symbols.json
+// RUN: %FileCheck %s --input-file %t/EmitWhileBuilding.formatted.symbols.json --check-prefix HEADER
 
 // REQUIRES: objc_interop
 
 import Foundation
 
 public enum SwiftEnum {}
 
+// HEADER:       "precise": "c:@testVariable"
+
+// CHECK:        "precise": "s:17EmitWhileBuilding9SwiftEnumO",
 // CHECK:        "declarationFragments": [
 // CHECK-NEXT:       {
 // CHECK-NEXT:           "kind": "keyword",

test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/Extra.swift

@@ -0,0 +1 @@
+public struct SomeStruct {}

test/SymbolGraph/ClangImporter/Inputs/Submodules/Mixed.h

@@ -0,0 +1 @@
+double outerVar = 1.0;

test/SymbolGraph/ClangImporter/Inputs/Submodules/Submodule.h

@@ -0,0 +1 @@
+double innerVar = 2.0;

test/SymbolGraph/ClangImporter/Inputs/Submodules/module.modulemap

@@ -0,0 +1,9 @@
+module Mixed {
+  header "Mixed.h"
+  export *
+
+  explicit module Submodule {
+    header "Submodule.h"
+    export *
+  }
+}

test/SymbolGraph/ClangImporter/Submodules.swift

@@ -0,0 +1,15 @@
+// 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/EmitWhileBuilding.swift
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -I %S/Inputs/Submodules -emit-module-path %t/Submodules.swiftmodule -enable-objc-interop -module-name Submodules -F %t %s -emit-symbol-graph -emit-symbol-graph-dir %t
+
+// REQUIRES: objc_interop
+
+// Don't crash when a module declared an `@_exported import` for a Clang non-top-level module.
+
+@_exported import Mixed
+@_exported import Mixed.Submodule
+
+@_exported import EmitWhileBuilding
+
+public func someFunc() {}