Merge pull request #18739 from nkcsgexi/abi-flag

swift-api-digester: diagnose the addition and removal of @objc, @_fixed_layout and @_frozen under ABI mode.

Author: nkcsgexi

include/swift/IDE/DigesterEnums.def

@@ -59,6 +59,9 @@ NODE_ANNOTATION(NowThrowing)
 NODE_ANNOTATION(NowMutating)
 NODE_ANNOTATION(StaticChange)
 NODE_ANNOTATION(OwnershipChange)
+NODE_ANNOTATION(ChangeObjC)
+NODE_ANNOTATION(ChangeFixedLayout)
+NODE_ANNOTATION(ChangeFrozen)
 NODE_ANNOTATION(RawTypeLeft)
 NODE_ANNOTATION(RawTypeRight)
 

test/api-digester/Inputs/cake1.swift

@@ -27,4 +27,12 @@ public struct Somestruct2 {
 
 public class C4: OldType {
   public func foo() {}
-}
+}
+
+@objc
+public class C5 {}
+
+public struct C6 {}
+
+@_frozen
+public enum IceKind {}

test/api-digester/Inputs/cake2.swift

@@ -28,4 +28,11 @@ public struct NSSomestruct2 {
   public static func foo1(_ a : C3) {}
 }
 
-public class C4: NewType {}
+public class C4: NewType {}
+
+public class C5 {}
+
+@_fixed_layout
+public struct C6 {}
+
+public enum IceKind {}

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

@@ -0,0 +1,27 @@
+
+/* RawRepresentable Changes */
+
+/* Removed Decls */
+cake1: Constructor Somestruct2.init(_:) has been removed
+cake1: Func C4.foo() has been removed
+
+/* Moved Decls */
+
+/* Renamed Decls */
+cake1: Struct Somestruct2 has been renamed to Struct NSSomestruct2
+cake1: Func S1.foo5(x:y:) has been renamed to Func S1.foo5(x:y:z:)
+
+/* Type Changes */
+cake1: Constructor S1.init(_:) has parameter 0 type change from Int to Double
+cake1: Func C1.foo2(_:) has parameter 0 type change from Int to () -> ()
+cake1: Func Somestruct2.foo1(_:) has parameter 0 type change from C3 to C1
+
+/* Decl Attribute changes */
+cake1: Enum IceKind is now without @_frozen
+cake1: Struct C6 is now with @_fixed_layout
+cake1: Class C5 is now without @objc
+cake1: Var C1.CIIns1 changes from weak to strong
+cake1: Var C1.CIIns2 changes from strong to weak
+cake1: Func C1.foo1() is now not static
+cake1: Func S1.foo1() is now mutating
+cake1: Func S1.foo3() is now static

test/api-digester/Outputs/Cake.txt

@@ -19,5 +19,5 @@ cake1: Func C1.foo2(_:) has parameter 0 type change from Int to () -> ()
 cake1: Var C1.CIIns1 changes from weak to strong
 cake1: Var C1.CIIns2 changes from strong to weak
 cake1: Func C1.foo1() is now not static
-cake1: Func S1.foo3() is now static
 cake1: Func S1.foo1() is now mutating
+cake1: Func S1.foo3() is now static

test/api-digester/compare-dump.swift

@@ -7,3 +7,8 @@
 // RUN: %api-digester -dump-sdk -module cake2 -o %t.dump2.json -module-cache-path %t.module-cache %clang-importer-sdk-nosource -I %t.mod -I %S/Inputs/APINotesRight
 // RUN: %api-digester -diagnose-sdk -print-module --input-paths %t.dump1.json -input-paths %t.dump2.json > %t.result
 // RUN: diff -u %S/Outputs/Cake.txt %t.result
+
+// RUN: %api-digester -dump-sdk -module cake1 -o %t.dump1.json -module-cache-path %t.module-cache %clang-importer-sdk-nosource -I %t.mod -I %S/Inputs/APINotesLeft -abi
+// RUN: %api-digester -dump-sdk -module cake2 -o %t.dump2.json -module-cache-path %t.module-cache %clang-importer-sdk-nosource -I %t.mod -I %S/Inputs/APINotesRight -abi
+// RUN: %api-digester -diagnose-sdk -print-module --input-paths %t.dump1.json -input-paths %t.dump2.json -abi > %t.result
+// RUN: diff -u %S/Outputs/Cake-abi.txt %t.result

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

@@ -252,16 +252,43 @@ class UpdatedNodesMap : public MatchedNodeListener {
   }
 };
 
+// Describing some attributes with ABI impact. The addition or removal of these
+// attributes is considerred ABI-breaking.
+struct ABIAttributeInfo {
+  const DeclAttrKind Kind;
+  const NodeAnnotation Annotation;
+  const StringRef Content;
+};
+
 class SDKContext {
-  bool ABI;
   llvm::StringSet<> TextData;
   llvm::BumpPtrAllocator Allocator;
   UpdatedNodesMap UpdateMap;
   NodeMap TypeAliasUpdateMap;
   NodeMap RevertTypeAliasUpdateMap;
   TypeMemberDiffVector TypeMemberDiffs;
+
+  bool ABI;
+  std::vector<ABIAttributeInfo> ABIAttrs;
+
+  static StringRef getAttrName(DeclAttrKind Kind) {
+    switch (Kind) {
+#define DECL_ATTR(NAME, CLASS, ...) case DAK_##CLASS: return "@"#NAME;
+#include "swift/AST/Attr.def"
+    case DAK_Count:
+      llvm_unreachable("unrecognized attribute kind.");
+    }
+  }
+
 public:
-  SDKContext(bool ABI): ABI(ABI) {}
+  SDKContext(bool ABI): ABI(ABI) {
+#define ADD(NAME) ABIAttrs.push_back({DeclAttrKind::DAK_##NAME, \
+      NodeAnnotation::Change##NAME, getAttrName(DeclAttrKind::DAK_##NAME)});
+    ADD(ObjC)
+    ADD(FixedLayout)
+    ADD(Frozen)
+#undef ADD
+  }
   llvm::BumpPtrAllocator &allocator() {
     return Allocator;
   }
@@ -281,6 +308,7 @@ class SDKContext {
     return TypeMemberDiffs;
   }
   bool checkingABI() const { return ABI; }
+  ArrayRef<ABIAttributeInfo> getABIAttributeInfo() const { return ABIAttrs; }
 };
 
 // A node matcher will traverse two trees of SDKNode and find matched nodes
@@ -446,7 +474,6 @@ class SDKNodeDecl : public SDKNode {
   bool IsStatic;
   bool IsDeprecated;
   uint8_t ReferenceOwnership;
-  bool hasDeclAttribute(DeclAttrKind DAKind) const;
   StringRef GenericSig;
 
 protected:
@@ -464,6 +491,7 @@ class SDKNodeDecl : public SDKNode {
   StringRef getModuleName() const {return ModuleName;}
   StringRef getHeaderName() const;
   ArrayRef<DeclAttrKind> getDeclAttributes() const;
+  bool hasAttributeChange(const SDKNodeDecl &Another) const;
   swift::ReferenceOwnership getReferenceOwnership() const {
     return swift::ReferenceOwnership(ReferenceOwnership);
   }
@@ -474,7 +502,7 @@ class SDKNodeDecl : public SDKNode {
   StringRef getFullyQualifiedName() const;
   bool isSDKPrivate() const;
   bool isDeprecated() const { return IsDeprecated; };
-  bool hasFixedLayout() const;
+  bool hasDeclAttribute(DeclAttrKind DAKind) const;
   bool isStatic() const { return IsStatic; };
   StringRef getGenericSignature() const { return GenericSig; }
 };
@@ -761,10 +789,6 @@ SDKNode *SDKNodeRoot::getInstance(SDKContext &Ctx) {
   return Info.createSDKNode(SDKNodeKind::Root);
 }
 
-bool SDKNodeDecl::hasFixedLayout() const {
-  return hasDeclAttribute(DeclAttrKind::DAK_FixedLayout);
-}
-
 bool SDKNodeDecl::isSDKPrivate() const {
   if (getName().startswith("__"))
     return true;
@@ -818,6 +842,16 @@ ArrayRef<DeclAttrKind> SDKNodeDecl::getDeclAttributes() const {
   return llvm::makeArrayRef(DeclAttributes.data(), DeclAttributes.size());
 }
 
+bool SDKNodeDecl::hasAttributeChange(const SDKNodeDecl &Another) const {
+  if (getDeclAttributes().size() != Another.getDeclAttributes().size())
+    return true;
+  for (auto K: getDeclAttributes()) {
+    if (!Another.hasDeclAttribute(K))
+      return true;
+  }
+  return false;
+}
+
 SDKNodeDecl *SDKNodeType::getClosestParentDecl() const {
   auto *Result = getParent();
   for (; !isa<SDKNodeDecl>(Result); Result = Result->getParent());
@@ -1175,6 +1209,10 @@ bool SDKNode::operator==(const SDKNode &Other) const {
         return false;
       if (Left->getReferenceOwnership() != Right->getReferenceOwnership())
         return false;
+      if (Left->hasAttributeChange(*Right))
+        return false;
+      if (Left->getGenericSignature() != Right->getGenericSignature())
+        return false;
       LLVM_FALLTHROUGH;
     }
     case SDKNodeKind::Root: {
@@ -2454,13 +2492,19 @@ static bool isOwnershipEquivalent(ReferenceOwnership Left,
 
 static void detectDeclChange(NodePtr L, NodePtr R) {
   assert(L->getKind() == R->getKind());
+  auto &Ctx = L->getSDKContext();
   if (auto LD = dyn_cast<SDKNodeDecl>(L)) {
     auto *RD = R->getAs<SDKNodeDecl>();
     if (LD->isStatic() ^ RD->isStatic())
       L->annotate(NodeAnnotation::StaticChange);
     if (!isOwnershipEquivalent(LD->getReferenceOwnership(),
                                RD->getReferenceOwnership()))
       L->annotate(NodeAnnotation::OwnershipChange);
+    // Check if some attributes with ABI-impact have been added/removed.
+    for (auto &Info: Ctx.getABIAttributeInfo()) {
+      if (LD->hasDeclAttribute(Info.Kind) != RD->hasDeclAttribute(Info.Kind))
+        L->annotate(Info.Annotation);
+    }
     detectRename(L, R);
   }
 }
@@ -3266,6 +3310,8 @@ class DiagnosisEmitter : public SDKNodeVisitor {
       llvm::outs() << " */\n";
       removeRedundantAndSort(Diags);
       std::for_each(Diags.begin(), Diags.end(), [](T &Diag) {
+        if (Diag.isABISpecific() && !options::Abi)
+          return;
         Diag.outputModule();
         Diag.output();
       });
@@ -3275,12 +3321,15 @@ class DiagnosisEmitter : public SDKNodeVisitor {
   struct MetaInfo {
     StringRef ModuleName;
     StringRef HeaderName;
-    MetaInfo(const SDKNodeDecl *Node):
-      ModuleName(Node->getModuleName()), HeaderName(Node->getHeaderName()) {}
+    bool IsABISpecific;
+    MetaInfo(const SDKNodeDecl *Node, bool IsABISpecific = false):
+      ModuleName(Node->getModuleName()), HeaderName(Node->getHeaderName()),
+      IsABISpecific(IsABISpecific) {}
   };
 
-  struct DiagBase {
+  class DiagBase {
     MetaInfo Info;
+  public:
     DiagBase(MetaInfo Info): Info(Info) {}
     virtual ~DiagBase() = default;
     void outputModule() const {
@@ -3292,6 +3341,7 @@ class DiagnosisEmitter : public SDKNodeVisitor {
       }
     }
     virtual void output() const = 0;
+    bool isABISpecific() const { return Info.IsABISpecific; }
   };
 
   struct RemovedDeclDiag: public DiagBase  {
@@ -3513,7 +3563,9 @@ void DiagnosisEmitter::DeclTypeChangeDiag::output() const {
 bool DiagnosisEmitter::DeclAttrDiag::operator<(DeclAttrDiag Other) const {
   if (Kind != Other.Kind)
     return Kind < Other.Kind;
-  return DeclName.compare_lower(Other.DeclName);
+  if (DeclName != Other.DeclName)
+    return DeclName.compare(Other.DeclName) < 0;
+  return AttrAfter.compare(Other.AttrAfter) < 0;
 }
 
 void DiagnosisEmitter::DeclAttrDiag::output() const {
@@ -3535,9 +3587,9 @@ void DiagnosisEmitter::diagnosis(NodePtr LeftRoot, NodePtr RightRoot,
 void DiagnosisEmitter::handle(const SDKNodeDecl *Node, NodeAnnotation Anno) {
   assert(Node->isAnnotatedAs(Anno));
   auto &Ctx = Node->getSDKContext();
-  MetaInfo ScreenInfo(Node);
   switch(Anno) {
   case NodeAnnotation::Removed: {
+    MetaInfo ScreenInfo(Node, false);
     // If we can find a type alias decl with the same name of this type, we
     // consider the type is not removed.
     if (findTypeAliasDecl(Node))
@@ -3599,6 +3651,7 @@ void DiagnosisEmitter::handle(const SDKNodeDecl *Node, NodeAnnotation Anno) {
     return;
   }
   case NodeAnnotation::Rename: {
+    MetaInfo ScreenInfo(Node, false);
     auto *Count = UpdateMap.findUpdateCounterpart(Node)->getAs<SDKNodeDecl>();
     RenamedDecls.Diags.emplace_back(ScreenInfo,
                                     Node->getDeclKind(), Count->getDeclKind(),
@@ -3607,27 +3660,31 @@ void DiagnosisEmitter::handle(const SDKNodeDecl *Node, NodeAnnotation Anno) {
     return;
   }
   case NodeAnnotation::NowMutating: {
+    MetaInfo ScreenInfo(Node, false);
     AttrChangedDecls.Diags.emplace_back(ScreenInfo,
                                         Node->getDeclKind(),
                                         Node->getFullyQualifiedName(),
                                         Ctx.buffer("mutating"));
     return;
   }
   case NodeAnnotation::NowThrowing: {
+    MetaInfo ScreenInfo(Node, false);
     AttrChangedDecls.Diags.emplace_back(ScreenInfo,
                                         Node->getDeclKind(),
                                         Node->getFullyQualifiedName(),
                                         Ctx.buffer("throwing"));
     return;
   }
   case NodeAnnotation::StaticChange: {
+    MetaInfo ScreenInfo(Node, false);
     AttrChangedDecls.Diags.emplace_back(ScreenInfo,
                                         Node->getDeclKind(),
                                         Node->getFullyQualifiedName(),
                         Ctx.buffer(Node->isStatic() ? "not static" : "static"));
     return;
   }
   case NodeAnnotation::OwnershipChange: {
+    MetaInfo ScreenInfo(Node, false);
     auto getOwnershipDescription = [&](swift::ReferenceOwnership O) {
       if (O == ReferenceOwnership::Strong)
         return Ctx.buffer("strong");
@@ -3640,9 +3697,22 @@ void DiagnosisEmitter::handle(const SDKNodeDecl *Node, NodeAnnotation Anno) {
         getOwnershipDescription(Count->getReferenceOwnership()));
     return;
   }
-  default:
+  default: {
+    // Diagnose the addition/removal of attributes with ABI impact.
+    auto Infos = Ctx.getABIAttributeInfo();
+    auto It = std::find_if(Infos.begin(), Infos.end(),
+      [&](const ABIAttributeInfo &I) { return I.Annotation == Anno; });
+    if (It == Infos.end())
+      return;
+    MetaInfo ScreenInfo(Node, true);
+    auto Desc = Node->hasDeclAttribute(It->Kind) ?
+      Ctx.buffer((llvm::Twine("without ") + It->Content).str()):
+      Ctx.buffer((llvm::Twine("with ") + It->Content).str());
+    AttrChangedDecls.Diags.emplace_back(ScreenInfo, Node->getDeclKind(),
+                                        Node->getFullyQualifiedName(), Desc);
     return;
   }
+  }
 }
 
 void DiagnosisEmitter::visitDecl(SDKNodeDecl *Node) {
@@ -3692,7 +3762,7 @@ void DiagnosisEmitter::visit(NodePtr Node) {
   }
 }
 
-  typedef std::vector<NoEscapeFuncParam> NoEscapeFuncParamVector;
+typedef std::vector<NoEscapeFuncParam> NoEscapeFuncParamVector;
 
 class NoEscapingFuncEmitter : public SDKNodeVisitor {
   NoEscapeFuncParamVector &AllItems;