From 7980273d5345091eca6e1738a62fbaf56c6d06db Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Sat, 17 Sep 2022 16:35:32 -0400 Subject: [PATCH] [SymbolGraphGen] Refactor export-import logic (#61049) (#61112) rdar://99939379 --- lib/SymbolGraphGen/SymbolGraph.cpp | 8 ++- lib/SymbolGraphGen/SymbolGraphASTWalker.cpp | 60 ++++++++++--------- lib/SymbolGraphGen/SymbolGraphASTWalker.h | 6 +- .../ClangImporter/ExportedImport.swift | 12 ++++ .../Inputs/ExportedImport/A.swift | 15 +++++ .../Headers/ObjcProperty.apinotes | 5 ++ .../Headers/ObjcProperty.h | 3 + .../Modules/ObjcProperty.swiftmodule/.keep | 0 .../ObjcProperty.framework/ObjcProperty | 0 .../ObjcProperty.framework/module.map | 4 ++ .../Module/Inputs/ThirdOrder/B.swift | 4 ++ test/SymbolGraph/Module/ThirdOrder.swift | 5 +- 12 files changed, 89 insertions(+), 33 deletions(-) create mode 100644 test/SymbolGraph/ClangImporter/ExportedImport.swift create mode 100644 test/SymbolGraph/ClangImporter/Inputs/ExportedImport/A.swift create mode 100644 test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/Headers/ObjcProperty.apinotes create mode 100644 test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/Headers/ObjcProperty.h create mode 100644 test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/Modules/ObjcProperty.swiftmodule/.keep create mode 100644 test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/ObjcProperty create mode 100644 test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/module.map diff --git a/lib/SymbolGraphGen/SymbolGraph.cpp b/lib/SymbolGraphGen/SymbolGraph.cpp index bed736aac1953..70e156d4cc3ef 100644 --- a/lib/SymbolGraphGen/SymbolGraph.cpp +++ b/lib/SymbolGraphGen/SymbolGraph.cpp @@ -321,8 +321,10 @@ void SymbolGraph::recordConformanceSynthesizedMemberRelationships(Symbol S) { // We are only interested in synthesized members that come from an // extension that we defined in our module. - if (Info.EnablingExt && Info.EnablingExt->getModuleContext() != &M) { - continue; + if (Info.EnablingExt) { + const auto *ExtM = Info.EnablingExt->getModuleContext(); + if (!Walker.isOurModule(ExtM)) + continue; } for (const auto ExtensionMember : Info.Ext->getMembers()) { @@ -398,7 +400,7 @@ void SymbolGraph::recordDefaultImplementationRelationships(Symbol S) { // If P is from a different module, and it's being added to a type // from the current module, add a `memberOf` relation to the extended // protocol. - if (MemberVD->getModuleContext()->getNameStr() != M.getNameStr() && VD->getDeclContext()) { + if (!Walker.isOurModule(MemberVD->getModuleContext()) && VD->getDeclContext()) { if (auto *ExP = VD->getDeclContext()->getSelfNominalTypeDecl()) { recordEdge(Symbol(this, VD, nullptr), Symbol(this, ExP, nullptr), diff --git a/lib/SymbolGraphGen/SymbolGraphASTWalker.cpp b/lib/SymbolGraphGen/SymbolGraphASTWalker.cpp index c19ece0a58215..e79fe4f9bff4a 100644 --- a/lib/SymbolGraphGen/SymbolGraphASTWalker.cpp +++ b/lib/SymbolGraphGen/SymbolGraphASTWalker.cpp @@ -25,12 +25,10 @@ namespace { /// Compare the two \c ModuleDecl instances to see whether they are the same. /// -/// Pass \c true to the \c ignoreUnderlying argument to consider two modules the same even if -/// one is a Swift module and the other a non-Swift module. This allows a Swift module and its -/// underlying Clang module to compare as equal. -bool areModulesEqual(const ModuleDecl *lhs, const ModuleDecl *rhs, bool ignoreUnderlying = false) { - return lhs->getNameStr() == rhs->getNameStr() - && (ignoreUnderlying || lhs->isNonSwiftModule() == rhs->isNonSwiftModule()); +/// 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(); } } // anonymous namespace @@ -50,11 +48,13 @@ SymbolGraphASTWalker::SymbolGraphASTWalker(ModuleDecl &M, SymbolGraph *SymbolGraphASTWalker::getModuleSymbolGraph(const Decl *D) { auto *M = D->getModuleContext(); const auto *DC = D->getDeclContext(); + SmallVector ParentTypes = {}; const Decl *ExtendedNominal = nullptr; while (DC) { M = DC->getParentModule(); if (const auto *NTD = dyn_cast_or_null(DC->getAsDecl())) { DC = NTD->getDeclContext(); + ParentTypes.push_back(NTD); } else if (const auto *Ext = dyn_cast_or_null(DC->getAsDecl())) { DC = Ext->getExtendedNominal()->getDeclContext(); if (!ExtendedNominal) @@ -64,10 +64,10 @@ SymbolGraph *SymbolGraphASTWalker::getModuleSymbolGraph(const Decl *D) { } } - if (areModulesEqual(&this->M, M, true)) { + if (areModulesEqual(&this->M, M)) { return &MainGraph; } else if (MainGraph.DeclaringModule.hasValue() && - areModulesEqual(MainGraph.DeclaringModule.getValue(), M, true)) { + areModulesEqual(MainGraph.DeclaringModule.getValue(), M)) { // Cross-import overlay modules already appear as "extensions" of their declaring module; we // should put actual extensions of that module into the main graph return &MainGraph; @@ -79,9 +79,8 @@ SymbolGraph *SymbolGraphASTWalker::getModuleSymbolGraph(const Decl *D) { return &MainGraph; } - if (ExtendedNominal && isFromExportedImportedModule(ExtendedNominal)) { - return &MainGraph; - } else if (!ExtendedNominal && isConsideredExportedImported(D)) { + // If this type is the child of a type which was re-exported in a qualified export, use the main graph. + if (llvm::any_of(ParentTypes, [&](const NominalTypeDecl *NTD){ return isQualifiedExportedImport(NTD); })) { return &MainGraph; } @@ -230,7 +229,7 @@ bool SymbolGraphASTWalker::walkToDeclPre(Decl *D, CharSourceRange Range) { if (const auto *ExtendedNominal = Extension->getExtendedNominal()) { auto ExtendedModule = ExtendedNominal->getModuleContext(); auto ExtendedSG = getModuleSymbolGraph(ExtendedNominal); - if (ExtendedModule != &M) { + if (!isOurModule(ExtendedModule)) { ExtendedSG->recordNode(Symbol(ExtendedSG, VD, nullptr)); return true; } @@ -244,22 +243,10 @@ bool SymbolGraphASTWalker::walkToDeclPre(Decl *D, CharSourceRange Range) { } bool SymbolGraphASTWalker::isConsideredExportedImported(const Decl *D) const { - // First check the decl itself to see if it was directly re-exported. - if (isFromExportedImportedModule(D)) - return true; - - const auto *DC = D->getDeclContext(); - - // Next, see if the decl is a child symbol of another decl that was re-exported. - if (DC) { - if (const auto *VD = dyn_cast_or_null(DC->getAsDecl())) { - if (isFromExportedImportedModule(VD)) - return true; - } - } - - // Finally, check to see if this decl is an extension of something else that was re-exported. + // Check to see if this decl is an extension of something else that was re-exported. + // Do this first in case there's a chain of extensions that leads somewhere that's not a re-export. // FIXME: this considers synthesized members of extensions to be valid + const auto *DC = D->getDeclContext(); const Decl *ExtendedNominal = nullptr; while (DC && !ExtendedNominal) { if (const auto *ED = dyn_cast_or_null(DC->getAsDecl())) { @@ -269,10 +256,23 @@ bool SymbolGraphASTWalker::isConsideredExportedImported(const Decl *D) const { } } - if (ExtendedNominal && isFromExportedImportedModule(ExtendedNominal)) { + if (ExtendedNominal && isConsideredExportedImported(ExtendedNominal)) { return true; } + // Check to see if the decl is a child symbol of another decl that was re-exported. + DC = D->getDeclContext(); + if (DC) { + if (const auto *VD = dyn_cast_or_null(DC->getAsDecl())) { + if (isConsideredExportedImported(VD)) + return true; + } + } + + // Check the decl itself to see if it was directly re-exported. + if (isFromExportedImportedModule(D) || isQualifiedExportedImport(D)) + return true; + // If none of the other checks passed, this wasn't from a re-export. return false; } @@ -293,3 +293,7 @@ bool SymbolGraphASTWalker::isExportedImportedModule(const ModuleDecl *M) const { return areModulesEqual(M, MD->getModuleContext()); }); } + +bool SymbolGraphASTWalker::isOurModule(const ModuleDecl *M) const { + return areModulesEqual(M, &this->M) || isExportedImportedModule(M); +} diff --git a/lib/SymbolGraphGen/SymbolGraphASTWalker.h b/lib/SymbolGraphGen/SymbolGraphASTWalker.h index 2b57f0efb4ef0..28982abf9948a 100644 --- a/lib/SymbolGraphGen/SymbolGraphASTWalker.h +++ b/lib/SymbolGraphGen/SymbolGraphASTWalker.h @@ -46,7 +46,8 @@ struct SymbolGraphASTWalker : public SourceEntityWalker { /// The module that this symbol graph will represent. const ModuleDecl &M; - + + // FIXME: these should be tracked per-graph, rather than at the top level const SmallPtrSet ExportedImportedModules; const llvm::SmallDenseMap, 4> QualifiedExportedImports; @@ -109,6 +110,9 @@ struct SymbolGraphASTWalker : public SourceEntityWalker { /// Returns whether the given module is an `@_exported import` module. virtual bool isExportedImportedModule(const ModuleDecl *M) const; + + /// Returns whether the given module is the main module, or is an `@_exported import` module. + virtual bool isOurModule(const ModuleDecl *M) const; }; } // end namespace symbolgraphgen diff --git a/test/SymbolGraph/ClangImporter/ExportedImport.swift b/test/SymbolGraph/ClangImporter/ExportedImport.swift new file mode 100644 index 0000000000000..ad1a63d5c7ff7 --- /dev/null +++ b/test/SymbolGraph/ClangImporter/ExportedImport.swift @@ -0,0 +1,12 @@ +// RUN: %empty-directory(%t) +// RUN: cp -r %S/Inputs/ExportedImport/ObjcProperty.framework %t +// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -enable-objc-interop -emit-module -o %t/ObjcProperty.framework/Modules/ObjcProperty.swiftmodule/%target-swiftmodule-name -import-underlying-module -F %t -module-name ObjcProperty %S/Inputs/ExportedImport/A.swift +// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -enable-objc-interop -emit-module -o %t/ExportedImport.swiftmodule -F %t -module-name ExportedImport %s -emit-symbol-graph -emit-symbol-graph-dir %t +// RUN: %FileCheck %s --input-file %t/ExportedImport.symbols.json + +// REQUIRES: objc_interop + +// CHECK-DAG: "precise":"s:So11ClangStructa12ObjcPropertyE05InnerB0V" +// CHECK-DAG: "precise":"s:12ObjcProperty12SomeProtocolPAAE8someFuncyyF::SYNTHESIZED::s:So11ClangStructa12ObjcPropertyE05InnerB0V06NestedB0V", + +@_exported import ObjcProperty diff --git a/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/A.swift b/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/A.swift new file mode 100644 index 0000000000000..dbd51b0d11fd2 --- /dev/null +++ b/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/A.swift @@ -0,0 +1,15 @@ +extension SwiftStruct { + public struct InnerStruct {} +} + +extension SwiftStruct.InnerStruct { + public struct NestedStruct {} +} + +public protocol SomeProtocol {} + +extension SomeProtocol { + public func someFunc() {} +} + +extension SwiftStruct.InnerStruct.NestedStruct: SomeProtocol {} diff --git a/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/Headers/ObjcProperty.apinotes b/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/Headers/ObjcProperty.apinotes new file mode 100644 index 0000000000000..5f601c8e26e80 --- /dev/null +++ b/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/Headers/ObjcProperty.apinotes @@ -0,0 +1,5 @@ +--- +Name: ObjcProperty +Typedefs: +- Name: ClangStruct + SwiftName: SwiftStruct diff --git a/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/Headers/ObjcProperty.h b/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/Headers/ObjcProperty.h new file mode 100644 index 0000000000000..4ee6ad636b943 --- /dev/null +++ b/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/Headers/ObjcProperty.h @@ -0,0 +1,3 @@ +typedef struct { + unsigned filler; +} ClangStruct; diff --git a/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/Modules/ObjcProperty.swiftmodule/.keep b/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/Modules/ObjcProperty.swiftmodule/.keep new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/ObjcProperty b/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/ObjcProperty new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/module.map b/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/module.map new file mode 100644 index 0000000000000..4ef75dd43304a --- /dev/null +++ b/test/SymbolGraph/ClangImporter/Inputs/ExportedImport/ObjcProperty.framework/module.map @@ -0,0 +1,4 @@ +framework module ObjcProperty { + header "ObjcProperty.h" + export * +} diff --git a/test/SymbolGraph/Module/Inputs/ThirdOrder/B.swift b/test/SymbolGraph/Module/Inputs/ThirdOrder/B.swift index cba400328a902..7a09ff46b0a58 100644 --- a/test/SymbolGraph/Module/Inputs/ThirdOrder/B.swift +++ b/test/SymbolGraph/Module/Inputs/ThirdOrder/B.swift @@ -3,3 +3,7 @@ import A public extension SomeStruct { struct InnerStruct: Equatable {} } + +public extension SomeStruct.InnerStruct { + struct NestedStruct: Equatable {} +} diff --git a/test/SymbolGraph/Module/ThirdOrder.swift b/test/SymbolGraph/Module/ThirdOrder.swift index 05356f06bbc53..01c929378a8c3 100644 --- a/test/SymbolGraph/Module/ThirdOrder.swift +++ b/test/SymbolGraph/Module/ThirdOrder.swift @@ -12,4 +12,7 @@ @_exported import B // BASE-NOT: "s:SQsE2neoiySbx_xtFZ::SYNTHESIZED::s:1A10SomeStructV1BE05InnerB0V" -// EXT: "s:SQsE2neoiySbx_xtFZ::SYNTHESIZED::s:1A10SomeStructV1BE05InnerB0V" +// EXT-DAG: "s:SQsE2neoiySbx_xtFZ::SYNTHESIZED::s:1A10SomeStructV1BE05InnerB0V" + +// BASE-NOT: "s:1A10SomeStructV1BE05InnerB0V06NestedB0V" +// EXT-DAG: "s:1A10SomeStructV1BE05InnerB0V06NestedB0V"