From d565c86a5cdaa97d97a5ec0b6f49255e266c4d8f Mon Sep 17 00:00:00 2001 From: Victoria Mitchell Date: Wed, 22 Dec 2021 16:34:49 -0700 Subject: [PATCH 1/4] [SymbolGraphGen] don't emit access control attributes in declarations rdar://85280786 --- lib/SymbolGraphGen/SymbolGraph.cpp | 4 +++ .../CursorInfo/cursor_symbol_graph.swift | 9 ------- .../cursor_symbol_graph_referenced.swift | 16 ------------ .../ClangImporter/EmitWhileBuilding.swift | 26 +++++++++++++++++++ .../Headers/EmitWhileBuilding.h | 1 + .../EmitWhileBuilding.swiftmodule/.keep | 0 .../EmitWhileBuilding.framework/ObjcProperty | 0 .../EmitWhileBuilding.framework/module.map | 4 +++ 8 files changed, 35 insertions(+), 25 deletions(-) create mode 100644 test/SymbolGraph/ClangImporter/EmitWhileBuilding.swift create mode 100644 test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework/Headers/EmitWhileBuilding.h create mode 100644 test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework/Modules/EmitWhileBuilding.swiftmodule/.keep create mode 100644 test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework/ObjcProperty create mode 100644 test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework/module.map diff --git a/lib/SymbolGraphGen/SymbolGraph.cpp b/lib/SymbolGraphGen/SymbolGraph.cpp index e115a0928bcdc..66fc5900489b0 100644 --- a/lib/SymbolGraphGen/SymbolGraph.cpp +++ b/lib/SymbolGraphGen/SymbolGraph.cpp @@ -99,6 +99,10 @@ PrintOptions SymbolGraph::getDeclarationFragmentsPrintOptions() const { ExcludeAttrs.insert(std::make_pair("DAK_Postfix", DAK_Postfix)); ExcludeAttrs.insert(std::make_pair("DAK_Infix", DAK_Infix)); + // In "emit modules separately" jobs, access modifiers show up as attributes, + // but we don't want them to be printed in declarations + ExcludeAttrs.insert(std::make_pair("DAK_AccessControl", DAK_AccessControl)); + for (const auto &Entry : ExcludeAttrs) { Opts.ExcludeAttrList.push_back(Entry.getValue()); } diff --git a/test/SourceKit/CursorInfo/cursor_symbol_graph.swift b/test/SourceKit/CursorInfo/cursor_symbol_graph.swift index fca7007c94949..80794a94c4a1f 100644 --- a/test/SourceKit/CursorInfo/cursor_symbol_graph.swift +++ b/test/SourceKit/CursorInfo/cursor_symbol_graph.swift @@ -182,15 +182,6 @@ enum MyEnum { // CHECKY: "symbols": [ // CHECKY: { // CHECKY: "accessLevel": "private", -// CHECKY: "declarationFragments": [ -// CHECKY: { -// CHECKY: "kind": "keyword", -// CHECKY: "spelling": "private" -// CHECKY: }, -// CHECKY: { -// CHECKY: "kind": "text", -// CHECKY: "spelling": " " -// CHECKY: }, // CHECKY: { // CHECKY: "kind": "keyword", // CHECKY: "spelling": "var" diff --git a/test/SourceKit/CursorInfo/cursor_symbol_graph_referenced.swift b/test/SourceKit/CursorInfo/cursor_symbol_graph_referenced.swift index 5cfe3805389b3..1754032227536 100644 --- a/test/SourceKit/CursorInfo/cursor_symbol_graph_referenced.swift +++ b/test/SourceKit/CursorInfo/cursor_symbol_graph_referenced.swift @@ -237,14 +237,6 @@ extension Parent { // PRIVATE: "declarationFragments": [ // PRIVATE-NEXT: { // PRIVATE-NEXT: "kind": "keyword", -// PRIVATE-NEXT: "spelling": "private" -// PRIVATE-NEXT: }, -// PRIVATE-NEXT: { -// PRIVATE-NEXT: "kind": "text", -// PRIVATE-NEXT: "spelling": " " -// PRIVATE-NEXT: }, -// PRIVATE-NEXT: { -// PRIVATE-NEXT: "kind": "keyword", // PRIVATE-NEXT: "spelling": "func" // PRIVATE-NEXT: }, // PRIVATE-NEXT: { @@ -302,14 +294,6 @@ extension Parent { // SPI: "declarationFragments": [ // SPI-NEXT: { // SPI-NEXT: "kind": "keyword", -// SPI-NEXT: "spelling": "internal" -// SPI-NEXT: }, -// SPI-NEXT: { -// SPI-NEXT: "kind": "text", -// SPI-NEXT: "spelling": " " -// SPI-NEXT: }, -// SPI-NEXT: { -// SPI-NEXT: "kind": "keyword", // SPI-NEXT: "spelling": "func" // SPI-NEXT: }, // SPI-NEXT: { diff --git a/test/SymbolGraph/ClangImporter/EmitWhileBuilding.swift b/test/SymbolGraph/ClangImporter/EmitWhileBuilding.swift new file mode 100644 index 0000000000000..f44a3bdcf9b62 --- /dev/null +++ b/test/SymbolGraph/ClangImporter/EmitWhileBuilding.swift @@ -0,0 +1,26 @@ +// 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: %{python} -m json.tool %t/EmitWhileBuilding.symbols.json %t/EmitWhileBuilding.formatted.symbols.json +// RUN: %FileCheck %s --input-file %t/EmitWhileBuilding.formatted.symbols.json + +// REQUIRES: objc_interop + +import Foundation + +public enum SwiftEnum {} + +// CHECK: "declarationFragments": [ +// CHECK-NEXT: { +// CHECK-NEXT: "kind": "keyword", +// CHECK-NEXT: "spelling": "enum" +// CHECK-NEXT: }, +// CHECK-NEXT: { +// CHECK-NEXT: "kind": "text", +// CHECK-NEXT: "spelling": " " +// CHECK-NEXT: }, +// CHECK-NEXT: { +// CHECK-NEXT: "kind": "identifier", +// CHECK-NEXT: "spelling": "SwiftEnum" +// CHECK-NEXT: } +// CHECK-NEXT: ], diff --git a/test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework/Headers/EmitWhileBuilding.h b/test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework/Headers/EmitWhileBuilding.h new file mode 100644 index 0000000000000..97e9757f4261b --- /dev/null +++ b/test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework/Headers/EmitWhileBuilding.h @@ -0,0 +1 @@ +double testVariable = 1.0; diff --git a/test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework/Modules/EmitWhileBuilding.swiftmodule/.keep b/test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework/Modules/EmitWhileBuilding.swiftmodule/.keep new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework/ObjcProperty b/test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework/ObjcProperty new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework/module.map b/test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework/module.map new file mode 100644 index 0000000000000..11f3b12d04e5a --- /dev/null +++ b/test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/EmitWhileBuilding.framework/module.map @@ -0,0 +1,4 @@ +framework module EmitWhileBuilding { + header "EmitWhileBuilding.h" + export * +} From 22ac4d16ad9f93e34932585bc14c74abf014f57c Mon Sep 17 00:00:00 2001 From: Victoria Mitchell Date: Tue, 11 Jan 2022 18:17:48 -0700 Subject: [PATCH 2/4] add module/file debug dumpers --- include/swift/AST/FileUnit.h | 4 ++++ include/swift/AST/Module.h | 4 ++++ lib/AST/Module.cpp | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/include/swift/AST/FileUnit.h b/include/swift/AST/FileUnit.h index 0417ea846368c..51cc26bd8e4f4 100644 --- a/include/swift/AST/FileUnit.h +++ b/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" @@ -308,6 +309,9 @@ class FileUnit : public DeclContext, public ASTAllocated { 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 diff --git a/include/swift/AST/Module.h b/include/swift/AST/Module.h index 31bb84ad0480b..d6e5a8ec6b69f 100644 --- a/include/swift/AST/Module.h +++ b/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" @@ -856,6 +857,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) { diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index 5df1407dd9ba2..e3a574c173279 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -788,6 +788,24 @@ void ModuleDecl::getTopLevelDecls(SmallVectorImpl &Results) const { FORWARD(getTopLevelDecls, (Results)); } +void ModuleDecl::dumpDisplayDecls() const { + SmallVector Decls; + getDisplayDecls(Decls); + for (auto *D : Decls) { + D->dump(llvm::errs()); + llvm::errs() << "\n"; + } +} + +void ModuleDecl::dumpTopLevelDecls() const { + SmallVector Decls; + getTopLevelDecls(Decls); + for (auto *D : Decls) { + D->dump(llvm::errs()); + llvm::errs() << "\n"; + } +} + void ModuleDecl::getExportedPrespecializations( SmallVectorImpl &Results) const { FORWARD(getExportedPrespecializations, (Results)); @@ -3060,6 +3078,22 @@ void FileUnit::getTopLevelDeclsWhereAttributesMatch( Results.erase(newEnd, Results.end()); } +void FileUnit::dumpDisplayDecls() const { + SmallVector Decls; + getDisplayDecls(Decls); + for (auto *D : Decls) { + D->dump(llvm::errs()); + } +} + +void FileUnit::dumpTopLevelDecls() const { + SmallVector 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)"; From 7ead1f6ab0091c5fee1fad7f3e23cc8525aa9221 Mon Sep 17 00:00:00 2001 From: Victoria Mitchell Date: Tue, 11 Jan 2022 18:17:56 -0700 Subject: [PATCH 3/4] SourceFile::getDisplayDecls also walks @_exported imports --- include/swift/AST/SourceFile.h | 2 ++ lib/AST/Module.cpp | 11 +++++++++++ .../SymbolGraph/ClangImporter/EmitWhileBuilding.swift | 4 ++++ 3 files changed, 17 insertions(+) diff --git a/include/swift/AST/SourceFile.h b/include/swift/AST/SourceFile.h index 99136c7051dba..03d918b0daf70 100644 --- a/include/swift/AST/SourceFile.h +++ b/include/swift/AST/SourceFile.h @@ -393,6 +393,8 @@ class SourceFile final : public FileUnit { public: virtual void getTopLevelDecls(SmallVectorImpl &results) const override; + virtual void getDisplayDecls(SmallVectorImpl &results) const override; + virtual void getOperatorDecls(SmallVectorImpl &results) const override; diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index e3a574c173279..46a17b241ec0d 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -822,6 +822,17 @@ void SourceFile::getTopLevelDecls(SmallVectorImpl &Results) const { Results.append(decls.begin(), decls.end()); } +void SourceFile::getDisplayDecls(SmallVectorImpl &Results) const { + if (Imports.hasValue()) { + for (auto import : *Imports) { + if (import.options.contains(ImportFlags::Exported)) { + import.module.importedModule->getDisplayDecls(Results); + } + } + } + getTopLevelDecls(Results); +} + void ModuleDecl::getOperatorDecls( SmallVectorImpl &results) const { // For a parsed module, we can check the source cache on the module rather diff --git a/test/SymbolGraph/ClangImporter/EmitWhileBuilding.swift b/test/SymbolGraph/ClangImporter/EmitWhileBuilding.swift index f44a3bdcf9b62..2f3c7e541d227 100644 --- a/test/SymbolGraph/ClangImporter/EmitWhileBuilding.swift +++ b/test/SymbolGraph/ClangImporter/EmitWhileBuilding.swift @@ -3,6 +3,7 @@ // 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: %{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 @@ -10,6 +11,9 @@ import Foundation public enum SwiftEnum {} +// HEADER: "precise": "c:@testVariable" + +// CHECK: "precise": "s:17EmitWhileBuilding9SwiftEnumO", // CHECK: "declarationFragments": [ // CHECK-NEXT: { // CHECK-NEXT: "kind": "keyword", From f2142a4073c518167938a4d78cbac1e17abfd9a1 Mon Sep 17 00:00:00 2001 From: Victoria Mitchell Date: Wed, 12 Jan 2022 16:24:11 -0700 Subject: [PATCH 4/4] don't crawl exported imports more than once --- include/swift/AST/SourceFile.h | 6 ++- lib/AST/Module.cpp | 42 ++++++++++++++----- .../ClangImporter/EmitWhileBuilding.swift | 2 +- .../Inputs/EmitWhileBuilding/Extra.swift | 1 + 4 files changed, 37 insertions(+), 14 deletions(-) create mode 100644 test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/Extra.swift diff --git a/include/swift/AST/SourceFile.h b/include/swift/AST/SourceFile.h index 03d918b0daf70..9b7d221a60c45 100644 --- a/include/swift/AST/SourceFile.h +++ b/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> getImports() const { return *Imports; @@ -393,8 +397,6 @@ class SourceFile final : public FileUnit { public: virtual void getTopLevelDecls(SmallVectorImpl &results) const override; - virtual void getDisplayDecls(SmallVectorImpl &results) const override; - virtual void getOperatorDecls(SmallVectorImpl &results) const override; diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index 46a17b241ec0d..50a693dcc6662 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -780,6 +780,22 @@ void SourceFile::lookupObjCMethods( results.append(known->second.begin(), known->second.end()); } +static void collectParsedExportedImports(const ModuleDecl *M, SmallPtrSetImpl &Imports) { + for (const FileUnit *file : M->getFiles()) { + if (const SourceFile *source = dyn_cast(file)) { + if (source->hasImports()) { + for (auto import : source->getImports()) { + if (import.options.contains(ImportFlags::Exported)) { + if (!Imports.contains(import.module.importedModule)) { + Imports.insert(import.module.importedModule); + } + } + } + } + } + } +} + void ModuleDecl::getLocalTypeDecls(SmallVectorImpl &Results) const { FORWARD(getLocalTypeDecls, (Results)); } @@ -822,17 +838,6 @@ void SourceFile::getTopLevelDecls(SmallVectorImpl &Results) const { Results.append(decls.begin(), decls.end()); } -void SourceFile::getDisplayDecls(SmallVectorImpl &Results) const { - if (Imports.hasValue()) { - for (auto import : *Imports) { - if (import.options.contains(ImportFlags::Exported)) { - import.module.importedModule->getDisplayDecls(Results); - } - } - } - getTopLevelDecls(Results); -} - void ModuleDecl::getOperatorDecls( SmallVectorImpl &results) const { // For a parsed module, we can check the source cache on the module rather @@ -937,8 +942,23 @@ SourceFile::getExternalRawLocsForDecl(const Decl *D) const { } void ModuleDecl::getDisplayDecls(SmallVectorImpl &Results) const { + if (isParsedModule(this)) { + SmallPtrSet Modules; + collectParsedExportedImports(this, Modules); + for (const ModuleDecl *import : Modules) { + import->getDisplayDecls(Results); + } + } // FIXME: Should this do extra access control filtering? FORWARD(getDisplayDecls, (Results)); + +#ifndef NDEBUG + llvm::DenseSet visited; + for (auto *D : Results) { + auto inserted = visited.insert(D).second; + assert(inserted && "there should be no duplicate decls"); + } +#endif } ProtocolConformanceRef diff --git a/test/SymbolGraph/ClangImporter/EmitWhileBuilding.swift b/test/SymbolGraph/ClangImporter/EmitWhileBuilding.swift index 2f3c7e541d227..7c39f50551537 100644 --- a/test/SymbolGraph/ClangImporter/EmitWhileBuilding.swift +++ b/test/SymbolGraph/ClangImporter/EmitWhileBuilding.swift @@ -1,6 +1,6 @@ // 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 diff --git a/test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/Extra.swift b/test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/Extra.swift new file mode 100644 index 0000000000000..7a0e59c907c0b --- /dev/null +++ b/test/SymbolGraph/ClangImporter/Inputs/EmitWhileBuilding/Extra.swift @@ -0,0 +1 @@ +public struct SomeStruct {}