SymbolGraph: Fixes for noncopyable generics and some small cleanups

slavapestov · slavapestov · commit 892dd4594da6 · 2024-03-01T12:55:08.000-05:00
diff --git a/lib/SymbolGraphGen/Edge.cpp b/lib/SymbolGraphGen/Edge.cpp
@@ -47,13 +47,11 @@ void Edge::serialize(llvm::json::OStream &OS) const {
       SmallVector<Requirement, 4> FilteredRequirements;
       filterGenericRequirements(
           ConformanceExtension->getGenericRequirements(),
-          ConformanceExtension->getExtendedNominal()
-              ->getDeclContext()->getSelfNominalTypeDecl(),
+          ConformanceExtension->getExtendedProtocolDecl(),
           FilteredRequirements);
       if (!FilteredRequirements.empty()) {
         OS.attributeArray("swiftConstraints", [&](){
-          for (const auto &Req :
-               ConformanceExtension->getGenericRequirements()) {
+          for (const auto &Req : FilteredRequirements) {
             ::serialize(Req, OS);
           }
         });
diff --git a/lib/SymbolGraphGen/JSON.cpp b/lib/SymbolGraphGen/JSON.cpp
@@ -216,6 +216,9 @@ swift::symbolgraphgen::filterGenericParams(
   }
 }
 
+// FIXME: This is wrong. We should instead compute the new requirements of a
+// member declaration by comparing against the generic signature of its
+// parent, with getRequirementsNotSatisfiedBy().
 static bool containsParams(swift::Type Ty, llvm::ArrayRef<const swift::GenericTypeParamType*> Others) {
   return Ty.findIf([&](swift::Type T) -> bool {
     if (auto AT = T->getAs<swift::ArchetypeType>()) {
@@ -232,20 +235,23 @@ static bool containsParams(swift::Type Ty, llvm::ArrayRef<const swift::GenericTy
 
 void swift::symbolgraphgen::filterGenericRequirements(
     ArrayRef<Requirement> Requirements,
-    const NominalTypeDecl *Self,
+    const ProtocolDecl *Self,
     SmallVectorImpl<Requirement> &FilteredRequirements,
     SubstitutionMap SubMap,
     ArrayRef<const GenericTypeParamType *> FilteredParams) {
 
   for (const auto &Req : Requirements) {
+    // FIXME: We're just dropping "Self: AnyObject", etc.
     if (Req.getKind() == RequirementKind::Layout) {
       continue;
     }
     // extension /* protocol */ Q {
     // func foo() {}
     // }
     // ignore Self : Q, obvious
-    if (Req.getSecondType()->getAnyNominal() == Self) {
+    if (Self &&
+        Req.getKind() == RequirementKind::Conformance &&
+        Req.getProtocolDecl() == Self) {
       continue;
     }
 
@@ -274,21 +280,31 @@ void swift::symbolgraphgen::filterGenericRequirements(
 void
 swift::symbolgraphgen::filterGenericRequirements(const ExtensionDecl *Extension,
     SmallVectorImpl<Requirement> &FilteredRequirements) {
-  for (const auto &Req : Extension->getGenericRequirements()) {
-    if (Req.getKind() == RequirementKind::Layout) {
-      continue;
-    }
+  auto Sig = Extension->getGenericSignature();
+  if (!Sig)
+    return;
 
-    if (!isa<ProtocolDecl>(Extension->getExtendedNominal()) &&
-        Req.getFirstType()->isEqual(Extension->getExtendedType())) {
+  SmallVector<Requirement, 2> Reqs;
+  SmallVector<InverseRequirement, 2> InverseReqs;
+  Sig->getRequirementsWithInverses(Reqs, InverseReqs);
+  // FIXME(noncopyable_generics): Do something with InverseReqs, or just use
+  // getRequirements() above and update the tests.
+
+  for (const auto &Req : Reqs) {
+    if (Req.getKind() == RequirementKind::Layout) {
       continue;
     }
 
     // extension /* protocol */ Q
     // ignore Self : Q, obvious
-    if (Req.getSecondType()->isEqual(Extension->getExtendedType())) {
-      continue;
+    if (auto *Proto = Extension->getExtendedProtocolDecl()) {
+      if (Req.getKind() == RequirementKind::Conformance &&
+          Req.getFirstType()->isEqual(Extension->getSelfInterfaceType()) &&
+          Req.getProtocolDecl() == Proto) {
+        continue;
+      }
     }
+
     FilteredRequirements.push_back(Req);
   }
 }
diff --git a/lib/SymbolGraphGen/JSON.h b/lib/SymbolGraphGen/JSON.h
@@ -53,7 +53,7 @@ void filterGenericParams(
 /// Filter generic requirements on an extension that aren't relevant
 /// for documentation.
 void filterGenericRequirements(
-    ArrayRef<Requirement> Requirements, const NominalTypeDecl *Self,
+    ArrayRef<Requirement> Requirements, const ProtocolDecl *Self,
     SmallVectorImpl<Requirement> &FilteredRequirements,
     SubstitutionMap SubMap = {},
     ArrayRef<const GenericTypeParamType *> FilteredParams = {});
diff --git a/lib/SymbolGraphGen/Symbol.cpp b/lib/SymbolGraphGen/Symbol.cpp
@@ -516,12 +516,18 @@ void Symbol::serializeSwiftGenericMixin(llvm::json::OStream &OS) const {
       filterGenericParams(Generics.getGenericParams(), FilteredParams,
                           SubMap);
 
-      const auto *Self = dyn_cast<NominalTypeDecl>(D);
+      const auto *Self = dyn_cast<ProtocolDecl>(D);
       if (!Self) {
-        Self = D->getDeclContext()->getSelfNominalTypeDecl();
+        Self = D->getDeclContext()->getSelfProtocolDecl();
       }
 
-      filterGenericRequirements(Generics.getRequirements(), Self,
+      SmallVector<Requirement, 2> Reqs;
+      SmallVector<InverseRequirement, 2> InverseReqs;
+      Generics->getRequirementsWithInverses(Reqs, InverseReqs);
+      // FIXME(noncopyable_generics): Do something with InverseReqs, or just use
+      // getRequirements() above and update the tests.
+
+      filterGenericRequirements(Reqs, Self,
                                 FilteredRequirements, SubMap, FilteredParams);
 
       if (FilteredParams.empty() && FilteredRequirements.empty()) {
diff --git a/lib/SymbolGraphGen/SymbolGraph.cpp b/lib/SymbolGraphGen/SymbolGraph.cpp
@@ -164,7 +164,7 @@ SymbolGraph::isRequirementOrDefaultImplementation(const ValueDecl *VD) const {
   // or a freestanding implementation from a protocol extension without
   // a corresponding requirement.
 
-  auto *Proto = dyn_cast_or_null<ProtocolDecl>(DC->getSelfNominalTypeDecl());
+  auto *Proto = DC->getSelfProtocolDecl();
   if (!Proto) {
     return false;
   }
@@ -185,7 +185,8 @@ SymbolGraph::isRequirementOrDefaultImplementation(const ValueDecl *VD) const {
   if (FoundRequirementMemberNamed(VD->getName(), Proto)) {
     return true;
   }
-  for (auto *Inherited : Proto->getInheritedProtocols()) {
+
+  for (auto *Inherited : Proto->getAllInheritedProtocols()) {
     if (FoundRequirementMemberNamed(VD->getName(), Inherited)) {
       return true;
     }
@@ -396,23 +397,18 @@ void SymbolGraph::recordConformanceSynthesizedMemberRelationships(Symbol S) {
 
 void
 SymbolGraph::recordInheritanceRelationships(Symbol S) {
-  const auto VD = S.getLocalSymbolDecl();
-  if (const auto *NTD = dyn_cast<NominalTypeDecl>(VD)) {
-    for (const auto &InheritanceLoc : NTD->getInherited().getEntries()) {
-      auto Ty = InheritanceLoc.getType();
-      if (!Ty) {
-        continue;
-      }
-      auto *InheritedTypeDecl =
-          dyn_cast_or_null<ClassDecl>(Ty->getAnyNominal());
-      if (!InheritedTypeDecl) {
-        continue;
-      }
+  const auto D = S.getLocalSymbolDecl();
 
-      recordEdge(Symbol(this, NTD, nullptr),
-                 Symbol(this, InheritedTypeDecl, nullptr),
-                 RelationshipKind::InheritsFrom());
-    }
+  ClassDecl *Super = nullptr;
+  if (auto *CD = dyn_cast<ClassDecl>(D))
+    Super = CD->getSuperclassDecl();
+  else if (auto *PD = dyn_cast<ProtocolDecl>(D))
+    Super = PD->getSuperclassDecl();
+
+  if (Super) {
+    recordEdge(Symbol(this, cast<ValueDecl>(D), nullptr),
+               Symbol(this, Super, nullptr),
+               RelationshipKind::InheritsFrom());
   }
 }
 
@@ -457,7 +453,7 @@ void SymbolGraph::recordDefaultImplementationRelationships(Symbol S) {
   if (const auto *Extension = dyn_cast<ExtensionDecl>(VD->getDeclContext())) {
     if (const auto *ExtendedProtocol = Extension->getExtendedProtocolDecl()) {
       HandleProtocol(ExtendedProtocol);
-      for (const auto *Inherited : ExtendedProtocol->getInheritedProtocols()) {
+      for (const auto *Inherited : ExtendedProtocol->getAllInheritedProtocols()) {
         HandleProtocol(Inherited);
       }
     }
@@ -493,11 +489,19 @@ void SymbolGraph::recordConformanceRelationships(Symbol S) {
   if (const auto *NTD = dyn_cast<NominalTypeDecl>(D)) {
     if (auto *PD = dyn_cast<ProtocolDecl>(NTD)) {
       for (auto *inherited : PD->getAllInheritedProtocols()) {
+        // FIXME(noncopyable_generics): Figure out what we want here.
+        if (inherited->getInvertibleProtocolKind())
+          continue;
+
         recordEdge(S, Symbol(this, inherited, nullptr),
                    RelationshipKind::ConformsTo(), nullptr);
       }
     } else {
       for (const auto *Conformance : NTD->getAllConformances()) {
+        // FIXME(noncopyable_generics): Figure out what we want here.
+        if (Conformance->getProtocol()->getInvertibleProtocolKind())
+          continue;
+
         // Check to make sure that this conformance wasn't declared via an
         // unconditionally-unavailable extension. If so, don't add that to the graph.
         if (const auto *ED = dyn_cast_or_null<ExtensionDecl>(Conformance->getDeclContext())) {
@@ -778,6 +782,7 @@ bool SymbolGraph::isImplicitlyPrivate(const Decl *D,
   return false;
 }
 
+/// FIXME: This should use AvailableAttr::isUnavailable() or similar.
 bool SymbolGraph::isUnconditionallyUnavailableOnAllPlatforms(const Decl *D) const {
   return llvm::any_of(D->getAttrs(), [](const auto *Attr) { 
     if (const auto *AvAttr = dyn_cast<AvailableAttr>(Attr)) {
diff --git a/test/SymbolGraph/Symbols/Mixins/Generics/Arguments.swift b/test/SymbolGraph/Symbols/Mixins/Generics/Arguments.swift
@@ -3,8 +3,6 @@
 // RUN: %target-swift-symbolgraph-extract -module-name Arguments -I %t -pretty-print -output-dir %t
 // RUN: %FileCheck %s --input-file %t/Arguments.symbols.json
 
-// XFAIL: noncopyable_generics
-
 public struct MyStruct<T> {
   public var x: T
   public init(x: T) {
diff --git a/test/SymbolGraph/Symbols/SkipProtocolImplementations.swift b/test/SymbolGraph/Symbols/SkipProtocolImplementations.swift
@@ -36,8 +36,6 @@
 // COUNT-COUNT-3: sourceOrigin
 // COUNT-NOT: sourceOrigin
 
-// XFAIL: noncopyable_generics
-
 public protocol SomeProtocol {
     /// Base docs
     func someFunc()
diff --git a/test/SymbolGraph/Symbols/SkipsPublicUnderscore.swift b/test/SymbolGraph/Symbols/SkipsPublicUnderscore.swift
@@ -9,8 +9,6 @@
 // RUN: %target-swift-symbolgraph-extract -module-name SkipsPublicUnderscore -I %t -pretty-print -output-dir %t -minimum-access-level private
 // RUN: %FileCheck %s --input-file %t/SkipsPublicUnderscore.symbols.json --check-prefix PRIVATE
 
-// XFAIL: noncopyable_generics
-
 public protocol PublicProtocol {}
 
 public class SomeClass {
