Merge pull request #31303 from bitjammer/acgarland/rdar-60193198-sgf-filter-duplicate-synthesized

[SymbolGraph] Pick best synthesized member when possible

Author: bitjammer

lib/SymbolGraphGen/SymbolGraph.cpp

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/DeclObjC.h"
+#include "swift/AST/ASTContext.h"
 #include "swift/AST/Decl.h"
 #include "swift/AST/Module.h"
 #include "swift/AST/ProtocolConformance.h"
@@ -218,6 +219,28 @@ void SymbolGraph::recordSuperclassSynthesizedMemberRelationships(Symbol S) {
   }
 }
 
+bool SymbolGraph::synthesizedMemberIsBestCandidate(const ValueDecl *VD,
+    const NominalTypeDecl *Owner) const {
+  const auto *FD = dyn_cast<FuncDecl>(VD);
+  if (!FD) {
+    return true;
+  }
+  auto *DC = const_cast<DeclContext*>(Owner->getDeclContext());
+
+  ResolvedMemberResult Result =
+    resolveValueMember(*DC, Owner->getSelfTypeInContext(),
+                       FD->getEffectiveFullName());
+
+  const auto ViableCandidates =
+    Result.getMemberDecls(InterestedMemberKind::All);
+
+  if (ViableCandidates.size() < 2) {
+    return true;
+  }
+
+  return !(Result.hasBestOverload() && Result.getBestOverload() != VD);
+}
+
 void SymbolGraph::recordConformanceSynthesizedMemberRelationships(Symbol S) {
   if (!Walker.Options.EmitSynthesizedMembers) {
     return;
@@ -264,14 +287,22 @@ void SymbolGraph::recordConformanceSynthesizedMemberRelationships(Symbol S) {
             continue;
           }
 
+          const auto StdlibModule = OwningNominal->getASTContext()
+              .getStdlibModule(/*loadIfAbsent=*/true);
+
           // There can be synthesized members on effectively private protocols
           // or things that conform to them. We don't want to include those.
-          if (SynthMember->hasUnderscoredNaming()) {
+          if (isImplicitlyPrivate(SynthMember,
+              /*IgnoreContext =*/
+              SynthMember->getModuleContext() == StdlibModule)) {
+            continue;
+          }
+
+          if (!synthesizedMemberIsBestCandidate(SynthMember, OwningNominal)) {
             continue;
           }
 
-          auto ExtendedSG =
-              Walker.getModuleSymbolGraph(OwningNominal->getModuleContext());
+          auto ExtendedSG = Walker.getModuleSymbolGraph(OwningNominal);
 
           Symbol Source(this, SynthMember, OwningNominal);
           Symbol Target(this, OwningNominal, nullptr);
@@ -501,7 +532,8 @@ SymbolGraph::serializeDeclarationFragments(StringRef Key, Type T,
   T->print(Printer, getDeclarationFragmentsPrintOptions());
 }
 
-bool SymbolGraph::isImplicitlyPrivate(const ValueDecl *VD) const {
+bool SymbolGraph::isImplicitlyPrivate(const ValueDecl *VD,
+                                      bool IgnoreContext) const {
   // Don't record unconditionally private declarations
   if (VD->isPrivateStdlibDecl(/*treatNonBuiltinProtocolsAsPublic=*/false)) {
     return true;
@@ -550,15 +582,19 @@ bool SymbolGraph::isImplicitlyPrivate(const ValueDecl *VD) const {
     return true;
   }
 
+  if (IgnoreContext) {
+    return false;
+  }
+
   // Check up the parent chain. Anything inside a privately named
   // thing is also private. We could be looking at the `B` of `_A.B`.
   if (const auto *DC = VD->getDeclContext()) {
     if (const auto *Parent = DC->getAsDecl()) {
       if (const auto *ParentVD = dyn_cast<ValueDecl>(Parent)) {
-        return isImplicitlyPrivate(ParentVD);
+        return isImplicitlyPrivate(ParentVD, IgnoreContext);
       } else if (const auto *Extension = dyn_cast<ExtensionDecl>(Parent)) {
         if (const auto *Nominal = Extension->getExtendedNominal()) {
-          return isImplicitlyPrivate(Nominal);
+          return isImplicitlyPrivate(Nominal, IgnoreContext);
         }
       }
     }

lib/SymbolGraphGen/SymbolGraph.h

@@ -75,6 +75,9 @@ struct SymbolGraph {
   /// Get the base print options for declaration fragments.
   PrintOptions getDeclarationFragmentsPrintOptions() const;
 
+  bool synthesizedMemberIsBestCandidate(const ValueDecl *VD,
+                                        const NominalTypeDecl *Owner) const;
+
   // MARK: - Symbols (Nodes)
 
   /**
@@ -197,7 +200,11 @@ struct SymbolGraph {
   /// Returns `true` if the declaration has a name that makes it
   /// implicitly internal/private, such as underscore prefixes,
   /// and checking every named parent context as well.
-  bool isImplicitlyPrivate(const ValueDecl *VD) const;
+  ///
+  /// \param IgnoreContext If `true`, don't consider
+  /// the context of the symbol to determine whether it is implicitly private.
+  bool isImplicitlyPrivate(const ValueDecl *VD,
+                           bool IgnoreContext = false) const;
 
   /// Returns `true` if the declaration should be included as a node
   /// in the graph.

test/SymbolGraph/Relationships/Synthesized/PickBestCandidate.swift

@@ -0,0 +1,29 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift %s -module-name PickBestCandidate -emit-module -emit-module-path %t/
+// RUN: %target-swift-symbolgraph-extract -module-name PickBestCandidate -I %t -pretty-print -output-dir %t
+// RUN: %FileCheck %s --input-file %t/PickBestCandidate.symbols.json
+
+public protocol P {
+  func foo()
+  func bar()
+}
+
+public protocol Q : P {}
+extension Q {
+  public func foo() {}
+  public func bar() {}
+}
+
+public protocol R : Q {}
+extension R {
+  public func foo() {}
+  public func bar() {}
+}
+
+public struct MyStruct: R {
+  public func bar() {}
+}
+
+// MyStruct gets one and only one synthesized `foo`.
+// MyStruct gets no synthesized `bar`, because it has its own implementation.
+// CHECK-COUNT-1: ::SYNTHESIZED::

tools/driver/swift_symbolgraph_extract_main.cpp

@@ -239,6 +239,9 @@ int swift_symbolgraph_extract_main(ArrayRef<const char *> Args, const char *Argv
     return EXIT_FAILURE;
   }
 
+  const auto &MainFile = M->getMainFile(FileUnitKind::SerializedAST);
+  llvm::errs() << "Emitting symbol graph for module file: " << MainFile.getModuleDefiningPath() << '\n';
+
   return symbolgraphgen::emitSymbolGraphForModule(M,
     Options);
 }