Merge pull request #19427 from nkcsgexi/diagnose-conformance

swift-module-digester: diagnose adding/removing protocol conformances as API breakages.

Author: nkcsgexi

include/swift/AST/DiagnosticsModuleDiffer.def

@@ -58,6 +58,10 @@ ERROR(decl_added,none,"%0 is added to a non-resilient type", (StringRef))
 
 ERROR(default_arg_removed,none,"%0 has removed default argument from %1", (StringRef, StringRef))
 
+ERROR(conformance_removed,none,"%0 has removed %select{conformance to|inherited protocol}2 %1", (StringRef, StringRef, bool))
+
+ERROR(conformance_added,none,"%0 has added inherited protocol %1", (StringRef, StringRef))
+
 #ifndef DIAG_NO_UNDEF
 # if defined(DIAG)
 #  undef DIAG

test/api-digester/Inputs/cake.swift

@@ -48,3 +48,5 @@ public struct fixedLayoutStruct {
   private var b = 2
   var c = 3
 }
+
+extension Int: P1 { public func bar() {} }

test/api-digester/Inputs/cake1.swift

@@ -64,4 +64,8 @@ public enum FrozenKind {
 
 public class C7 {
   public func foo(_ a: Int = 1, _ b: Int = 2) {}
-}
+}
+
+public protocol P3: P2, P1 {}
+
+extension fixedLayoutStruct: P1 {}

test/api-digester/Inputs/cake2.swift

@@ -67,6 +67,12 @@ public enum FrozenKind {
   case AddedCase
 }
 
-public class C7 {
+public class C7: P1 {
   public func foo(_ a: Int, _ b: Int) {}
-}
+}
+
+public protocol P3: P1, P4 {}
+
+public protocol P4 {}
+
+extension fixedLayoutStruct: P2 {}

test/api-digester/Outputs/Cake-abi.txt

@@ -1,6 +1,7 @@
 
 /* Generic Signature Changes */
 cake1: Func P1.P1Constraint() has generic signature change from <τ_0_0 where τ_0_0 : P1, τ_0_0 : P2> to <τ_0_0 where τ_0_0 : P1>
+cake1: Protocol P3 has generic signature change from <τ_0_0 : P1, τ_0_0 : P2> to <τ_0_0 : P1, τ_0_0 : P4>
 
 /* RawRepresentable Changes */
 
@@ -40,4 +41,9 @@ cake1: Var fixedLayoutStruct.a in a non-resilient type changes position from 1 t
 cake1: Var fixedLayoutStruct.b in a non-resilient type changes position from 0 to 1
 cake2: EnumElement FrozenKind.AddedCase is added to a non-resilient type
 cake2: Var fixedLayoutStruct.c is added to a non-resilient type
-cake2: Var fixedLayoutStruct.lazy_d.storage is added to a non-resilient type
+cake2: Var fixedLayoutStruct.lazy_d.storage is added to a non-resilient type
+
+/* Conformance changes */
+cake1: Protocol P3 has added inherited protocol P4
+cake1: Protocol P3 has removed inherited protocol P2
+cake1: Struct fixedLayoutStruct has removed conformance to P1

test/api-digester/Outputs/Cake.txt

@@ -1,6 +1,7 @@
 
 /* Generic Signature Changes */
 cake1: Func P1.P1Constraint() has generic signature change from <Self where Self : P1, Self : P2> to <Self where Self : P1>
+cake1: Protocol P3 has generic signature change from <Self : P1, Self : P2> to <Self : P1, Self : P4>
 
 /* RawRepresentable Changes */
 
@@ -27,3 +28,8 @@ cake1: Func S1.foo1() is now mutating
 cake1: Func S1.foo3() is now static
 cake1: Var C1.CIIns1 changes from weak to strong
 cake1: Var C1.CIIns2 changes from strong to weak
+
+/* Conformance changes */
+cake1: Protocol P3 has added inherited protocol P4
+cake1: Protocol P3 has removed inherited protocol P2
+cake1: Struct fixedLayoutStruct has removed conformance to P1

test/api-digester/Outputs/cake-abi.json

@@ -983,6 +983,7 @@
         "Strideable",
         "ExpressibleByIntegerLiteral",
         "FixedWidthInteger",
+        "P1",
         "Encodable",
         "Decodable",
         "Hashable",
@@ -1012,6 +1013,22 @@
               "printedName": "()"
             }
           ]
+        },
+        {
+          "kind": "Function",
+          "name": "bar",
+          "printedName": "bar()",
+          "declKind": "Func",
+          "usr": "s:Si4cakeE3baryyF",
+          "location": "",
+          "moduleName": "cake",
+          "children": [
+            {
+              "kind": "TypeNominal",
+              "name": "Void",
+              "printedName": "()"
+            }
+          ]
         }
       ]
     }

test/api-digester/Outputs/cake.json

@@ -859,6 +859,7 @@
         "Strideable",
         "ExpressibleByIntegerLiteral",
         "FixedWidthInteger",
+        "P1",
         "Encodable",
         "Decodable",
         "Hashable",
@@ -888,6 +889,22 @@
               "printedName": "()"
             }
           ]
+        },
+        {
+          "kind": "Function",
+          "name": "bar",
+          "printedName": "bar()",
+          "declKind": "Func",
+          "usr": "s:Si4cakeE3baryyF",
+          "location": "",
+          "moduleName": "cake",
+          "children": [
+            {
+              "kind": "TypeNominal",
+              "name": "Void",
+              "printedName": "()"
+            }
+          ]
         }
       ]
     }

tools/swift-api-digester/ModuleAnalyzerNodes.cpp

@@ -1,4 +1,5 @@
 #include <ModuleAnalyzerNodes.h>
+#include <algorithm>
 
 using namespace swift;
 using namespace ide;
@@ -642,6 +643,25 @@ bool SDKNode::hasSameChildren(const SDKNode &Other) const {
   return true;
 }
 
+void swift::ide::api::stringSetDifference(ArrayRef<StringRef> Left,
+                                          ArrayRef<StringRef> Right,
+                                          std::vector<StringRef> &LeftMinusRight,
+                                          std::vector<StringRef> &RightMinusLeft) {
+  std::set<StringRef> LS(Left.begin(), Left.end());
+  std::set<StringRef> RS(Right.begin(), Right.end());
+  std::set_difference(LS.begin(), LS.end(), RS.begin(), RS.end(),
+                      std::back_inserter(LeftMinusRight));
+  std::set_difference(RS.begin(), RS.end(), LS.begin(), LS.end(),
+                      std::back_inserter(RightMinusLeft));
+}
+
+static bool hasSameContents(ArrayRef<StringRef> Left,
+                            ArrayRef<StringRef> Right) {
+  std::vector<StringRef> LeftMinusRight, RightMinusLeft;
+  stringSetDifference(Left, Right, LeftMinusRight, RightMinusLeft);
+  return LeftMinusRight.empty() && RightMinusLeft.empty();
+}
+
 bool SDKNode::operator==(const SDKNode &Other) const {
   auto *LeftAlias = dyn_cast<SDKNodeTypeAlias>(this);
   auto *RightAlias = dyn_cast<SDKNodeTypeAlias>(&Other);
@@ -701,7 +721,16 @@ bool SDKNode::operator==(const SDKNode &Other) const {
       }
       LLVM_FALLTHROUGH;
     }
-    case SDKNodeKind::DeclType:
+    case SDKNodeKind::DeclType: {
+      auto *Left = dyn_cast<SDKNodeDeclType>(this);
+      auto *Right = dyn_cast<SDKNodeDeclType>(&Other);
+      if (Left && Right) {
+        if (!hasSameContents(Left->getAllProtocols(), Right->getAllProtocols())) {
+          return false;
+        }
+      }
+      LLVM_FALLTHROUGH;
+    }
     case SDKNodeKind::DeclTypeAlias: {
       auto Left = this->getAs<SDKNodeDecl>();
       auto Right = (&Other)->getAs<SDKNodeDecl>();
@@ -1187,13 +1216,16 @@ static SDKNode *constructTypeDeclNode(SDKContext &Ctx, NominalTypeDecl *NTD,
 /// synthesize this type node to include those extension members, since these
 /// extension members are legit members of the module.
 static SDKNode *constructExternalExtensionNode(SDKContext &Ctx, SDKNode *Root,
-                                               ExtensionDecl *Ext,
+                                               NominalTypeDecl *NTD,
+                                               ArrayRef<ExtensionDecl*> AllExts,
                                         std::set<ExtensionDecl*> &HandledExts) {
-  auto *TypeNode = SDKNodeInitInfo(Ctx, Ext->getSelfNominalTypeDecl())
-      .createSDKNode(SDKNodeKind::DeclType);
+  auto *TypeNode = SDKNodeInitInfo(Ctx, NTD).createSDKNode(SDKNodeKind::DeclType);
 
-  // The members of the extension are the only members of this synthesized type.
-  addMembersToRoot(Ctx, TypeNode, Ext, HandledExts);
+  // The members of the extensions are the only members of this synthesized type.
+  for (auto *Ext: AllExts) {
+    HandledExts.insert(Ext);
+    addMembersToRoot(Ctx, TypeNode, Ext, HandledExts);
+  }
   return TypeNode;
 }
 
@@ -1273,16 +1305,20 @@ void SwiftDeclCollector::lookupVisibleDecls(ArrayRef<ModuleDecl *> Modules) {
   for (auto *VD : ClangMacros)
     processDecl(VD);
 
-  // For all known decls, collect those unhandled extensions and handle them
-  // separately.
+  // Collect extensions to types from other modules and synthesize type nodes
+  // for them.
+  llvm::MapVector<NominalTypeDecl*, llvm::SmallVector<ExtensionDecl*, 4>> ExtensionMap;
   for (auto *D: KnownDecls) {
     if (auto *Ext = dyn_cast<ExtensionDecl>(D)) {
       if (HandledExtensions.find(Ext) == HandledExtensions.end()) {
-        RootNode->addChild(constructExternalExtensionNode(Ctx, RootNode, Ext,
-                                                          HandledExtensions));
+        ExtensionMap[Ext->getExtendedNominal()].push_back(Ext);
       }
     }
   }
+  for (auto Pair: ExtensionMap) {
+    RootNode->addChild(constructExternalExtensionNode(Ctx, RootNode,
+      Pair.first, Pair.second, HandledExtensions));
+  }
 }
 
 void SwiftDeclCollector::processDecl(ValueDecl *VD) {

tools/swift-api-digester/ModuleAnalyzerNodes.h

@@ -495,7 +495,7 @@ class SwiftDeclCollector: public VisibleDeclConsumer {
   SDKContext &Ctx;
   std::vector<std::unique_ptr<llvm::MemoryBuffer>> OwnedBuffers;
   SDKNode *RootNode;
-  llvm::DenseSet<Decl*> KnownDecls;
+  llvm::SetVector<Decl*> KnownDecls;
   // Collected and sorted after we get all of them.
   std::vector<ValueDecl *> ClangMacros;
   std::set<ExtensionDecl*> HandledExtensions;
@@ -543,6 +543,9 @@ int deserializeSDKDump(StringRef dumpPath, StringRef OutputPath,
 
 int findDeclUsr(StringRef dumpPath, CheckerOptions Opts);
 
+void stringSetDifference(ArrayRef<StringRef> Left, ArrayRef<StringRef> Right,
+  std::vector<StringRef> &LeftMinusRight, std::vector<StringRef> &RightMinusLeft);
+
 } // end of abi namespace
 } // end of ide namespace
 } // end of Swift namespace

tools/swift-api-digester/ModuleDiagsConsumer.cpp

@@ -51,6 +51,9 @@ static StringRef getCategoryName(uint32_t ID) {
   case LocalDiagID::decl_added:
   case LocalDiagID::decl_reorder:
     return "/* Fixed-layout Type Changes */";
+  case LocalDiagID::conformance_added:
+  case LocalDiagID::conformance_removed:
+    return "/* Protocol Conformance Change */";
   default:
     return StringRef();
   }

tools/swift-api-digester/swift-api-digester.cpp

@@ -674,6 +674,28 @@ static bool isOwnershipEquivalent(ReferenceOwnership Left,
   return false;
 }
 
+static void diagnoseNominalTypeDeclChange(SDKNodeDeclType *L, SDKNodeDeclType *R) {
+  auto &Ctx = L->getSDKContext();
+  auto &Diags = Ctx.getDiags();
+  std::vector<StringRef> LeftMinusRight;
+  std::vector<StringRef> RightMinusLeft;
+  swift::ide::api::stringSetDifference(L->getAllProtocols(), R->getAllProtocols(),
+                                       LeftMinusRight, RightMinusLeft);
+  bool isProtocol = L->getDeclKind() == DeclKind::Protocol;
+  std::for_each(LeftMinusRight.begin(), LeftMinusRight.end(), [&](StringRef Name) {
+    Diags.diagnose(SourceLoc(), diag::conformance_removed, L->getScreenInfo(), Name,
+                   isProtocol);
+  });
+
+  // Adding inherited protocols can be API breaking.
+  if (isProtocol) {
+    std::for_each(RightMinusLeft.begin(), RightMinusLeft.end(), [&](StringRef Name) {
+      Diags.diagnose(SourceLoc(), diag::conformance_added, L->getScreenInfo(),
+                     Name);
+    });
+  }
+}
+
 static void detectDeclChange(NodePtr L, NodePtr R, SDKContext &Ctx) {
   assert(L->getKind() == R->getKind());
   auto &Diags = Ctx.getDiags();
@@ -727,6 +749,12 @@ static void detectDeclChange(NodePtr L, NodePtr R, SDKContext &Ctx) {
       Diags.diagnose(SourceLoc(), diag::generic_sig_change, LD->getScreenInfo(),
         LD->getGenericSignature(), RD->getGenericSignature());
     }
+
+    if (auto *LDT = dyn_cast<SDKNodeDeclType>(L)) {
+      if (auto *RDT = dyn_cast<SDKNodeDeclType>(R)) {
+        diagnoseNominalTypeDeclChange(LDT, RDT);
+      }
+    }
     detectRename(L, R);
   }
 }