Skip to content

Commit 7744433

Browse files
franklinschfranklinsch
authored
Merge pull request #40866 from apple/QuietMisdreavus/5.6/display-decl-recur
[5.6][AST] scan `@_exported` imports of source files for display decls
2 parents 45fa618 + 79f8b48 commit 7744433

File tree

21 files changed

+173
-18
lines changed

21 files changed

+173
-18
lines changed

‎include/swift/AST/FileUnit.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "swift/AST/Module.h"
1717
#include "swift/AST/RawComment.h"
1818
#include "swift/Basic/BasicSourceInfo.h"
19+
#include "swift/Basic/Debug.h"
1920

2021
#include "llvm/ADT/PointerIntPair.h"
2122

@@ -188,6 +189,8 @@ class FileUnit : public DeclContext, public ASTAllocated<FileUnit> {
188189
virtual Identifier
189190
getDiscriminatorForPrivateValue(const ValueDecl *D) const = 0;
190191

192+
virtual bool shouldCollectDisplayDecls() const { return true; }
193+
191194
/// Finds all top-level decls in this file.
192195
///
193196
/// This does a simple local lookup, not recursively looking through imports.
@@ -243,7 +246,7 @@ class FileUnit : public DeclContext, public ASTAllocated<FileUnit> {
243246
///
244247
/// This can differ from \c getTopLevelDecls, e.g. it returns decls from a
245248
/// shadowed clang module.
246-
virtual void getDisplayDecls(SmallVectorImpl<Decl*> &results) const {
249+
virtual void getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recursive = false) const {
247250
getTopLevelDecls(results);
248251
}
249252

@@ -308,6 +311,9 @@ class FileUnit : public DeclContext, public ASTAllocated<FileUnit> {
308311
return getParentModule()->getRealName().str();
309312
}
310313

314+
SWIFT_DEBUG_DUMPER(dumpDisplayDecls());
315+
SWIFT_DEBUG_DUMPER(dumpTopLevelDecls());
316+
311317
/// Traverse the decls within this file.
312318
///
313319
/// \returns true if traversal was aborted, false if it completed

‎include/swift/AST/Module.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "swift/AST/Type.h"
2727
#include "swift/Basic/BasicSourceInfo.h"
2828
#include "swift/Basic/Compiler.h"
29+
#include "swift/Basic/Debug.h"
2930
#include "swift/Basic/OptionSet.h"
3031
#include "swift/Basic/STLExtras.h"
3132
#include "swift/Basic/SourceLoc.h"
@@ -759,6 +760,15 @@ class ModuleDecl
759760
/// The order of the results is not guaranteed to be meaningful.
760761
void getPrecedenceGroups(SmallVectorImpl<PrecedenceGroupDecl*> &Results) const;
761762

763+
/// Determines whether this module should be recursed into when calling
764+
/// \c getDisplayDecls.
765+
///
766+
/// Some modules should not call \c getDisplayDecls, due to assertions
767+
/// in their implementation. These are usually implicit imports that would be
768+
/// recursed into for parsed modules. This function provides a guard against
769+
/// recusing into modules that should not have decls collected.
770+
bool shouldCollectDisplayDecls() const;
771+
762772
/// Finds all top-level decls that should be displayed to a client of this
763773
/// module.
764774
///
@@ -770,7 +780,7 @@ class ModuleDecl
770780
/// shadowed clang module. It does not force synthesized top-level decls that
771781
/// should be printed to be added; use \c swift::getTopLevelDeclsForDisplay()
772782
/// for that.
773-
void getDisplayDecls(SmallVectorImpl<Decl*> &results) const;
783+
void getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recursive = false) const;
774784

775785
using LinkLibraryCallback = llvm::function_ref<void(LinkLibrary)>;
776786

@@ -856,6 +866,9 @@ class ModuleDecl
856866
/// transferred from module files to the dSYMs, remove this.
857867
bool isExternallyConsumed() const;
858868

869+
SWIFT_DEBUG_DUMPER(dumpDisplayDecls());
870+
SWIFT_DEBUG_DUMPER(dumpTopLevelDecls());
871+
859872
SourceRange getSourceRange() const { return SourceRange(); }
860873

861874
static bool classof(const DeclContext *DC) {

‎include/swift/AST/SourceFile.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,10 @@ class SourceFile final : public FileUnit {
288288

289289
~SourceFile();
290290

291+
bool hasImports() const {
292+
return Imports.hasValue();
293+
}
294+
291295
/// Retrieve an immutable view of the source file's imports.
292296
ArrayRef<AttributedImport<ImportedModule>> getImports() const {
293297
return *Imports;

‎include/swift/ClangImporter/ClangModule.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,11 @@ class ClangModuleUnit final : public LoadedFile {
8787
ObjCSelector selector,
8888
SmallVectorImpl<AbstractFunctionDecl *> &results) const override;
8989

90+
virtual bool shouldCollectDisplayDecls() const override;
91+
9092
virtual void getTopLevelDecls(SmallVectorImpl<Decl*> &results) const override;
9193

92-
virtual void getDisplayDecls(SmallVectorImpl<Decl*> &results) const override;
94+
virtual void getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recursive = false) const override;
9395

9496
virtual void
9597
getImportedModules(SmallVectorImpl<ImportedModule> &imports,

‎include/swift/Sema/IDETypeChecking.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ namespace swift {
145145
/// \c ModuleDecl::getDisplayDecls() would only return if previous
146146
/// work happened to have synthesized them.
147147
void
148-
getTopLevelDeclsForDisplay(ModuleDecl *M, SmallVectorImpl<Decl*> &Results);
148+
getTopLevelDeclsForDisplay(ModuleDecl *M, SmallVectorImpl<Decl*> &Results, bool Recursive = false);
149149

150150
struct ExtensionInfo {
151151
// The extension with the declarations to apply.

‎include/swift/Serialization/SerializedModuleLoader.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ class SerializedASTFile final : public LoadedFile {
433433
virtual void
434434
getOpaqueReturnTypeDecls(SmallVectorImpl<OpaqueTypeDecl*> &results) const override;
435435

436-
virtual void getDisplayDecls(SmallVectorImpl<Decl*> &results) const override;
436+
virtual void getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recursive = false) const override;
437437

438438
virtual void
439439
getImportedModules(SmallVectorImpl<ImportedModule> &imports,

‎lib/AST/Module.cpp

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,30 @@ void SourceFile::lookupObjCMethods(
780780
results.append(known->second.begin(), known->second.end());
781781
}
782782

783+
bool ModuleDecl::shouldCollectDisplayDecls() const {
784+
for (const FileUnit *file : Files) {
785+
if (!file->shouldCollectDisplayDecls())
786+
return false;
787+
}
788+
return true;
789+
}
790+
791+
static void collectParsedExportedImports(const ModuleDecl *M, SmallPtrSetImpl<ModuleDecl *> &Imports) {
792+
for (const FileUnit *file : M->getFiles()) {
793+
if (const SourceFile *source = dyn_cast<SourceFile>(file)) {
794+
if (source->hasImports()) {
795+
for (auto import : source->getImports()) {
796+
if (import.options.contains(ImportFlags::Exported) &&
797+
!Imports.contains(import.module.importedModule) &&
798+
import.module.importedModule->shouldCollectDisplayDecls()) {
799+
Imports.insert(import.module.importedModule);
800+
}
801+
}
802+
}
803+
}
804+
}
805+
}
806+
783807
void ModuleDecl::getLocalTypeDecls(SmallVectorImpl<TypeDecl*> &Results) const {
784808
FORWARD(getLocalTypeDecls, (Results));
785809
}
@@ -788,6 +812,24 @@ void ModuleDecl::getTopLevelDecls(SmallVectorImpl<Decl*> &Results) const {
788812
FORWARD(getTopLevelDecls, (Results));
789813
}
790814

815+
void ModuleDecl::dumpDisplayDecls() const {
816+
SmallVector<Decl *, 32> Decls;
817+
getDisplayDecls(Decls);
818+
for (auto *D : Decls) {
819+
D->dump(llvm::errs());
820+
llvm::errs() << "\n";
821+
}
822+
}
823+
824+
void ModuleDecl::dumpTopLevelDecls() const {
825+
SmallVector<Decl *, 32> Decls;
826+
getTopLevelDecls(Decls);
827+
for (auto *D : Decls) {
828+
D->dump(llvm::errs());
829+
llvm::errs() << "\n";
830+
}
831+
}
832+
791833
void ModuleDecl::getExportedPrespecializations(
792834
SmallVectorImpl<Decl *> &Results) const {
793835
FORWARD(getExportedPrespecializations, (Results));
@@ -907,9 +949,34 @@ SourceFile::getExternalRawLocsForDecl(const Decl *D) const {
907949
return Result;
908950
}
909951

910-
void ModuleDecl::getDisplayDecls(SmallVectorImpl<Decl*> &Results) const {
952+
void ModuleDecl::getDisplayDecls(SmallVectorImpl<Decl*> &Results, bool Recursive) const {
953+
if (Recursive && isParsedModule(this)) {
954+
SmallPtrSet<ModuleDecl *, 4> Modules;
955+
collectParsedExportedImports(this, Modules);
956+
for (const ModuleDecl *import : Modules) {
957+
import->getDisplayDecls(Results, Recursive);
958+
}
959+
}
911960
// FIXME: Should this do extra access control filtering?
912961
FORWARD(getDisplayDecls, (Results));
962+
963+
#ifndef NDEBUG
964+
if (Recursive) {
965+
llvm::DenseSet<Decl *> visited;
966+
for (auto *D : Results) {
967+
// decls synthesized from implicit clang decls may appear multiple times;
968+
// e.g. if multiple modules with underlying clang modules are re-exported.
969+
// including duplicates of these is harmless, so skip them when counting
970+
// this assertion
971+
if (const auto *CD = D->getClangDecl()) {
972+
if (CD->isImplicit()) continue;
973+
}
974+
975+
auto inserted = visited.insert(D).second;
976+
assert(inserted && "there should be no duplicate decls");
977+
}
978+
}
979+
#endif
913980
}
914981

915982
ProtocolConformanceRef
@@ -3065,6 +3132,22 @@ void FileUnit::getTopLevelDeclsWhereAttributesMatch(
30653132
Results.erase(newEnd, Results.end());
30663133
}
30673134

3135+
void FileUnit::dumpDisplayDecls() const {
3136+
SmallVector<Decl *, 32> Decls;
3137+
getDisplayDecls(Decls);
3138+
for (auto *D : Decls) {
3139+
D->dump(llvm::errs());
3140+
}
3141+
}
3142+
3143+
void FileUnit::dumpTopLevelDecls() const {
3144+
SmallVector<Decl *, 32> Decls;
3145+
getTopLevelDecls(Decls);
3146+
for (auto *D : Decls) {
3147+
D->dump(llvm::errs());
3148+
}
3149+
}
3150+
30683151
void swift::simple_display(llvm::raw_ostream &out, const FileUnit *file) {
30693152
if (!file) {
30703153
out << "(null)";

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2998,6 +2998,9 @@ class VectorDeclPtrConsumer : public swift::VisibleDeclConsumer {
29982998
};
29992999
} // unnamed namespace
30003000

3001+
// FIXME: should submodules still be crawled for the symbol graph? (SR-15753)
3002+
bool ClangModuleUnit::shouldCollectDisplayDecls() const { return isTopLevel(); }
3003+
30013004
void ClangModuleUnit::getTopLevelDecls(SmallVectorImpl<Decl*> &results) const {
30023005
VectorDeclPtrConsumer consumer(results);
30033006
FilteringDeclaredDeclConsumer filterConsumer(consumer, this);
@@ -3118,7 +3121,7 @@ static void getImportDecls(ClangModuleUnit *ClangUnit, const clang::Module *M,
31183121
}
31193122
}
31203123

3121-
void ClangModuleUnit::getDisplayDecls(SmallVectorImpl<Decl*> &results) const {
3124+
void ClangModuleUnit::getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recursive) const {
31223125
if (clangModule)
31233126
getImportDecls(const_cast<ClangModuleUnit *>(this), clangModule, results);
31243127
getTopLevelDecls(results);

‎lib/ClangImporter/DWARFImporter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ class DWARFModuleUnit final : public LoadedFile {
6464
getTopLevelDecls(SmallVectorImpl<Decl *> &results) const override {}
6565

6666
virtual void
67-
getDisplayDecls(SmallVectorImpl<Decl *> &results) const override {}
67+
getDisplayDecls(SmallVectorImpl<Decl *> &results, bool recursive = false) const override {}
6868

6969
virtual void
7070
getImportedModules(SmallVectorImpl<ImportedModule> &imports,

‎lib/IDE/IDETypeChecking.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,10 @@ using namespace swift;
3535

3636
void
3737
swift::getTopLevelDeclsForDisplay(ModuleDecl *M,
38-
SmallVectorImpl<Decl*> &Results) {
38+
SmallVectorImpl<Decl*> &Results,
39+
bool Recursive) {
3940
auto startingSize = Results.size();
40-
M->getDisplayDecls(Results);
41+
M->getDisplayDecls(Results, Recursive);
4142

4243
// Force Sendable on all types, which might synthesize some extensions.
4344
// FIXME: We can remove this if @_nonSendable stops creating extensions.
@@ -49,7 +50,7 @@ swift::getTopLevelDeclsForDisplay(ModuleDecl *M,
4950
// Remove what we fetched and fetch again, possibly now with additional
5051
// extensions.
5152
Results.resize(startingSize);
52-
M->getDisplayDecls(Results);
53+
M->getDisplayDecls(Results, Recursive);
5354
}
5455

5556
static bool shouldPrintAsFavorable(const Decl *D, const PrintOptions &Options) {

‎lib/Serialization/ModuleFile.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -973,9 +973,9 @@ ModuleFile::getOpaqueReturnTypeDecls(SmallVectorImpl<OpaqueTypeDecl *> &results)
973973
}
974974
}
975975

976-
void ModuleFile::getDisplayDecls(SmallVectorImpl<Decl *> &results) {
976+
void ModuleFile::getDisplayDecls(SmallVectorImpl<Decl *> &results, bool recursive) {
977977
if (UnderlyingModule)
978-
UnderlyingModule->getDisplayDecls(results);
978+
UnderlyingModule->getDisplayDecls(results, recursive);
979979

980980
PrettyStackTraceModuleFile stackEntry(*this);
981981
getImportDecls(results);

‎lib/Serialization/ModuleFile.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,7 @@ class ModuleFile
672672
/// This includes all decls that should be displayed to clients of the module.
673673
/// This can differ from \c getTopLevelDecls, e.g. it returns decls from a
674674
/// shadowed clang module.
675-
void getDisplayDecls(SmallVectorImpl<Decl*> &results);
675+
void getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recursive = false);
676676

677677
StringRef getModuleFilename() const {
678678
if (!Core->ModuleInterfacePath.empty())

‎lib/Serialization/SerializedModuleLoader.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1526,8 +1526,8 @@ SerializedASTFile::getOpaqueReturnTypeDecls(
15261526
}
15271527

15281528
void
1529-
SerializedASTFile::getDisplayDecls(SmallVectorImpl<Decl*> &results) const {
1530-
File.getDisplayDecls(results);
1529+
SerializedASTFile::getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recursive) const {
1530+
File.getDisplayDecls(results, recursive);
15311531
}
15321532

15331533
StringRef SerializedASTFile::getFilename() const {

‎lib/SymbolGraphGen/SymbolGraphGen.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ symbolgraphgen::emitSymbolGraphForModule(ModuleDecl *M,
5757
const SymbolGraphOptions &Options) {
5858
SymbolGraphASTWalker Walker(*M, Options);
5959
SmallVector<Decl *, 64> ModuleDecls;
60-
swift::getTopLevelDeclsForDisplay(M, ModuleDecls);
60+
swift::getTopLevelDeclsForDisplay(M, ModuleDecls, /*recursive*/true);
6161

6262
if (Options.PrintMessages)
6363
llvm::errs() << ModuleDecls.size()

‎test/ModuleInterface/exported-import.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -swift-version 5 -enable-library-evolution -emit-module-path %t/Other.swiftmodule -module-name Other %S/Inputs/other.swift
3+
// 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
4+
5+
// RUN: %FileCheck --input-file %t/ExportedImport.swiftmodule %s
6+
7+
// CHECK-NOT: otherFileFunction
8+
9+
@_exported import Other
10+
11+
// CHECK: public struct SomeStruct
12+
public struct SomeStruct {}

‎test/SymbolGraph/ClangImporter/EmitWhileBuilding.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
11
// RUN: %empty-directory(%t)
22
// RUN: cp -r %S/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework %t
3-
// 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
3+
// 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
44
// RUN: %{python} -m json.tool %t/EmitWhileBuilding.symbols.json %t/EmitWhileBuilding.formatted.symbols.json
55
// RUN: %FileCheck %s --input-file %t/EmitWhileBuilding.formatted.symbols.json
6+
// RUN: %FileCheck %s --input-file %t/EmitWhileBuilding.formatted.symbols.json --check-prefix HEADER
67

78
// REQUIRES: objc_interop
89

910
import Foundation
1011

1112
public enum SwiftEnum {}
1213

14+
// HEADER: "precise": "c:@testVariable"
15+
16+
// CHECK: "precise": "s:17EmitWhileBuilding9SwiftEnumO",
1317
// CHECK: "declarationFragments": [
1418
// CHECK-NEXT: {
1519
// CHECK-NEXT: "kind": "keyword",

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
public struct SomeStruct {}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
double outerVar = 1.0;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
double innerVar = 2.0;

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
module Mixed {
2+
header "Mixed.h"
3+
export *
4+
5+
explicit module Submodule {
6+
header "Submodule.h"
7+
export *
8+
}
9+
}

‎test/SymbolGraph/ClangImporter/Submodules.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: cp -r %S/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework %t
3+
// 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
4+
// 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
5+
6+
// REQUIRES: objc_interop
7+
8+
// Don't crash when a module declared an `@_exported import` for a Clang non-top-level module.
9+
10+
@_exported import Mixed
11+
@_exported import Mixed.Submodule
12+
13+
@_exported import EmitWhileBuilding
14+
15+
public func someFunc() {}

0 commit comments

Comments
 (0)