Merge pull request #59258 from nkcsgexi/abi-checker-fix-extension

nkcsgexi · web-flow · commit 399d5f32ec94 · 2022-06-03T15:01:38.000-07:00
diff --git a/include/swift/APIDigester/ModuleAnalyzerNodes.h b/include/swift/APIDigester/ModuleAnalyzerNodes.h
@@ -63,7 +63,7 @@ namespace api {
 ///
 /// When the json format changes in a way that requires version-specific handling, this number should be incremented.
 /// This ensures we could have backward compatibility so that version changes in the format won't stop the checker from working.
-const uint8_t DIGESTER_JSON_VERSION = 7; // push SDKNodeRoot to lower-level
+const uint8_t DIGESTER_JSON_VERSION = 8; // add isFromExtension
 const uint8_t DIGESTER_JSON_DEFAULT_VERSION = 0; // Use this version number for files before we have a version number in json.
 const StringRef ABIRootKey = "ABIRoot";
 const StringRef ConstValuesKey = "ConstValues";
@@ -354,6 +354,7 @@ class SDKNodeDecl: public SDKNode {
   bool IsOpen;
   bool IsInternal;
   bool IsABIPlaceholder;
+  bool IsFromExtension;
   uint8_t ReferenceOwnership;
   StringRef GenericSig;
   // In ABI mode, this field is populated as a user-friendly version of GenericSig.
@@ -392,6 +393,7 @@ class SDKNodeDecl: public SDKNode {
   bool isOpen() const { return IsOpen; }
   bool isInternal() const { return IsInternal; }
   bool isABIPlaceholder() const { return IsABIPlaceholder; }
+  bool isFromExtension() const { return IsFromExtension; }
   StringRef getGenericSignature() const { return GenericSig; }
   StringRef getSugaredGenericSignature() const { return SugaredGenericSig; }
   StringRef getScreenInfo() const;
@@ -710,6 +712,7 @@ class SDKNodeDeclConstructor: public SDKNodeDeclAbstractFunc {
   void jsonize(json::Output &Out) override;
 };
 
+// Note: Accessor doesn't have Parent pointer.
 class SDKNodeDeclAccessor: public SDKNodeDeclAbstractFunc {
   SDKNodeDecl *Owner;
   AccessorKind AccKind;
@@ -828,6 +831,8 @@ int findDeclUsr(StringRef dumpPath, CheckerOptions Opts);
 
 void nodeSetDifference(ArrayRef<SDKNode*> Left, ArrayRef<SDKNode*> Right,
   NodeVector &LeftMinusRight, NodeVector &RightMinusLeft);
+
+bool hasValidParentPtr(SDKNodeKind kind);
 } // end of abi namespace
 } // end of ide namespace
 } // end of Swift namespace
diff --git a/include/swift/AST/DiagnosticsModuleDiffer.def b/include/swift/AST/DiagnosticsModuleDiffer.def
@@ -76,6 +76,8 @@ ERROR(type_witness_change,APIDigesterBreakage,"%0 has type witness type for %1 c
 
 ERROR(decl_new_witness_table_entry,APIDigesterBreakage,"%0 now requires%select{| no}1 new witness table entry", (StringRef, bool))
 
+ERROR(class_member_moved_to_extension,APIDigesterBreakage,"Non-final class member %0 is moved to extension", (StringRef))
+
 WARNING(new_decl_without_intro,APIDigesterBreakage,"%0 is a new API without @available attribute", (StringRef))
 
 ERROR(objc_name_change,APIDigesterBreakage,"%0 has ObjC name change from %1 to %2", (StringRef, StringRef, StringRef))
diff --git a/include/swift/IDE/DigesterEnums.def b/include/swift/IDE/DigesterEnums.def
@@ -136,6 +136,7 @@ KEY_BOOL(IsExternal, isExternal)
 KEY_BOOL(IsEnumExhaustive, isEnumExhaustive)
 KEY_BOOL(HasMissingDesignatedInitializers, hasMissingDesignatedInitializers)
 KEY_BOOL(InheritsConvenienceInitializers, inheritsConvenienceInitializers)
+KEY_BOOL(IsFromExtension, isFromExtension)
 
 KEY(kind)
 
diff --git a/lib/APIDigester/ModuleAnalyzerNodes.cpp b/lib/APIDigester/ModuleAnalyzerNodes.cpp
@@ -54,6 +54,16 @@ struct swift::ide::api::SDKNodeInitInfo {
   SDKNode* createSDKNode(SDKNodeKind Kind);
 };
 
+bool swift::ide::api::hasValidParentPtr(SDKNodeKind kind) {
+  switch(kind) {
+  case SDKNodeKind::Conformance:
+  case SDKNodeKind::DeclAccessor:
+    return false;
+  default:
+    return true;
+  }
+}
+
 SDKContext::SDKContext(CheckerOptions Opts): Diags(SourceMgr), Opts(Opts) {}
 
 DiagnosticEngine &SDKContext::getDiags(SourceLoc Loc) {
@@ -109,6 +119,7 @@ SDKNodeDecl::SDKNodeDecl(SDKNodeInitInfo Info, SDKNodeKind Kind)
         IsOverriding(Info.IsOverriding),
         IsOpen(Info.IsOpen),
         IsInternal(Info.IsInternal), IsABIPlaceholder(Info.IsABIPlaceholder),
+        IsFromExtension(Info.IsFromExtension),
         ReferenceOwnership(uint8_t(Info.ReferenceOwnership)),
         GenericSig(Info.GenericSig),
         SugaredGenericSig(Info.SugaredGenericSig),
@@ -826,6 +837,25 @@ static bool hasSameParameterFlags(const SDKNodeType *Left, const SDKNodeType *Ri
   return true;
 }
 
+// Return whether a decl has been moved in/out to an extension
+static Optional<bool> isFromExtensionChanged(const SDKNode &L, const SDKNode &R) {
+  assert(L.getKind() == R.getKind());
+  // Version 8 starts to include whether a decl is from an extension.
+  if (L.getJsonFormatVersion() + R.getJsonFormatVersion() < 2 * 8) {
+    return llvm::None;
+  }
+  auto *Left = dyn_cast<SDKNodeDecl>(&L);
+  auto *Right = dyn_cast<SDKNodeDecl>(&R);
+  if (!Left) {
+    return llvm::None;
+  }
+  if (Left->isFromExtension() == Right->isFromExtension()) {
+    return llvm::None;
+  } else {
+    return Right->isFromExtension();
+  }
+}
+
 static bool isSDKNodeEqual(SDKContext &Ctx, const SDKNode &L, const SDKNode &R) {
   auto *LeftAlias = dyn_cast<SDKNodeTypeAlias>(&L);
   auto *RightAlias = dyn_cast<SDKNodeTypeAlias>(&R);
@@ -965,6 +995,8 @@ static bool isSDKNodeEqual(SDKContext &Ctx, const SDKNode &L, const SDKNode &R)
     case SDKNodeKind::TypeWitness:
     case SDKNodeKind::DeclImport:
     case SDKNodeKind::Root: {
+      if (isFromExtensionChanged(L, R))
+        return false;
       return L.getPrintedName() == R.getPrintedName() &&
         L.hasSameChildren(R);
     }
@@ -1376,6 +1408,13 @@ StringRef SDKContext::getInitKind(Decl *D) {
   return StringRef();
 }
 
+static bool isDeclaredInExtension(Decl *D) {
+  if (auto *DC = D->getDeclContext()->getAsDecl()) {
+    return isa<ExtensionDecl>(DC);
+  }
+  return false;
+}
+
 SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, Decl *D):
       Ctx(Ctx), DKind(D->getKind()), Loc(D->getLoc()),
       Location(calculateLocation(Ctx, D)),
@@ -1394,6 +1433,7 @@ SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, Decl *D):
       IsImplicit(D->isImplicit()),
       IsDeprecated(D->getAttrs().getDeprecated(D->getASTContext())),
       IsABIPlaceholder(isABIPlaceholderRecursive(D)),
+      IsFromExtension(isDeclaredInExtension(D)),
       DeclAttrs(collectDeclAttributes(D)) {}
 
 SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, OperatorDecl *OD):
@@ -2040,6 +2080,7 @@ void SDKNodeDecl::jsonize(json::Output &out) {
     uint8_t Raw = uint8_t(getReferenceOwnership());
     out.mapRequired(getKeyContent(Ctx, KeyKind::KK_ownership).data(), Raw);
   }
+  output(out, KeyKind::KK_isFromExtension, IsFromExtension);
 }
 
 void SDKNodeDeclAbstractFunc::jsonize(json::Output &out) {
@@ -2611,6 +2652,23 @@ void swift::ide::api::SDKNodeDeclAbstractFunc::diagnose(SDKNode *Right) {
     if (reqNewWitnessTableEntry() != R->reqNewWitnessTableEntry()) {
       emitDiag(Loc, diag::decl_new_witness_table_entry, reqNewWitnessTableEntry());
     }
+
+    // Diagnose moving a non-final class member to an extension.
+    if (hasValidParentPtr(getKind())) {
+      while(auto *parent = dyn_cast<SDKNodeDecl>(getParent())) {
+        if (parent->getDeclKind() != DeclKind::Class) {
+          break;
+        }
+        if (hasDeclAttribute(DeclAttrKind::DAK_Final)) {
+          break;
+        }
+        auto result = isFromExtensionChanged(*this, *Right);
+        if (result.hasValue() && *result) {
+          emitDiag(Loc, diag::class_member_moved_to_extension);
+        }
+        break;
+      }
+    }
   }
 }
 
diff --git a/test/api-digester/Outputs/cake-abi.json b/test/api-digester/Outputs/cake-abi.json
@@ -64,6 +64,7 @@
             "genericSig": "<τ_0_0 where τ_0_0 : cake.P1>",
             "sugared_genericSig": "<Self where Self : cake.P1>",
             "static": true,
+            "isFromExtension": true,
             "funcSelfKind": "NonMutating"
           }
         ],
@@ -219,6 +220,7 @@
             "moduleName": "cake",
             "genericSig": "<τ_0_0, τ_0_1, τ_0_2 where τ_0_0 == cake.S1, τ_0_1 == cake.S1, τ_0_2 == cake.S1>",
             "sugared_genericSig": "<T1, T2, T3 where T1 == cake.S1, T2 == cake.S1, T3 == cake.S1>",
+            "isFromExtension": true,
             "funcSelfKind": "NonMutating"
           },
           {
@@ -238,6 +240,7 @@
             "moduleName": "cake",
             "genericSig": "<τ_0_0, τ_0_1, τ_0_2>",
             "sugared_genericSig": "<T1, T2, T3>",
+            "isFromExtension": true,
             "funcSelfKind": "NonMutating"
           }
         ],
@@ -1021,6 +1024,7 @@
             "moduleName": "cake",
             "genericSig": "<τ_0_0 where τ_0_0 : cake.ProWithAssociatedType>",
             "sugared_genericSig": "<Self where Self : cake.ProWithAssociatedType>",
+            "isFromExtension": true,
             "funcSelfKind": "NonMutating"
           },
           {
@@ -1039,6 +1043,7 @@
             "usr": "s:4cake21ProWithAssociatedTypePAAE9NonReqVarSivp",
             "mangledName": "$s4cake21ProWithAssociatedTypePAAE9NonReqVarSivp",
             "moduleName": "cake",
+            "isFromExtension": true,
             "accessors": [
               {
                 "kind": "Accessor",
@@ -1058,6 +1063,7 @@
                 "moduleName": "cake",
                 "genericSig": "<τ_0_0 where τ_0_0 : cake.ProWithAssociatedType>",
                 "sugared_genericSig": "<Self where Self : cake.ProWithAssociatedType>",
+                "isFromExtension": true,
                 "accessorKind": "get"
               }
             ]
@@ -1305,6 +1311,7 @@
             "moduleName": "cake",
             "genericSig": "<τ_0_0 where τ_0_0 : cake.PSuper>",
             "sugared_genericSig": "<Self where Self : cake.PSuper>",
+            "isFromExtension": true,
             "funcSelfKind": "NonMutating"
           }
         ],
@@ -1648,6 +1655,7 @@
             "usr": "s:Si4cakeE3fooyyF",
             "mangledName": "$sSi4cakeE3fooyyF",
             "moduleName": "cake",
+            "isFromExtension": true,
             "funcSelfKind": "NonMutating"
           },
           {
@@ -1665,6 +1673,7 @@
             "usr": "s:Si4cakeE3baryyF",
             "mangledName": "$sSi4cakeE3baryyF",
             "moduleName": "cake",
+            "isFromExtension": true,
             "funcSelfKind": "NonMutating"
           }
         ],
@@ -2008,6 +2017,6 @@
         ]
       }
     ],
-    "json_format_version": 7
+    "json_format_version": 8
   }
 }
diff --git a/test/api-digester/Outputs/cake.json b/test/api-digester/Outputs/cake.json
@@ -63,6 +63,7 @@
             "moduleName": "cake",
             "genericSig": "<Self where Self : cake.P1>",
             "static": true,
+            "isFromExtension": true,
             "funcSelfKind": "NonMutating"
           }
         ],
@@ -223,6 +224,7 @@
             "mangledName": "$s4cake2C0CA2A2S1VRszAERs_AERs0_rlE17conditionalFooExtyyF",
             "moduleName": "cake",
             "genericSig": "<T1, T2, T3 where T1 == cake.S1, T2 == cake.S1, T3 == cake.S1>",
+            "isFromExtension": true,
             "funcSelfKind": "NonMutating"
           },
           {
@@ -241,6 +243,7 @@
             "mangledName": "$s4cake2C0C19unconditionalFooExtyyF",
             "moduleName": "cake",
             "genericSig": "<T1, T2, T3>",
+            "isFromExtension": true,
             "funcSelfKind": "NonMutating"
           }
         ],
@@ -921,6 +924,7 @@
             "mangledName": "$s4cake21ProWithAssociatedTypePAAE10NonReqFuncyyF",
             "moduleName": "cake",
             "genericSig": "<Self where Self : cake.ProWithAssociatedType>",
+            "isFromExtension": true,
             "funcSelfKind": "NonMutating"
           },
           {
@@ -939,6 +943,7 @@
             "usr": "s:4cake21ProWithAssociatedTypePAAE9NonReqVarSivp",
             "mangledName": "$s4cake21ProWithAssociatedTypePAAE9NonReqVarSivp",
             "moduleName": "cake",
+            "isFromExtension": true,
             "accessors": [
               {
                 "kind": "Accessor",
@@ -957,6 +962,7 @@
                 "mangledName": "$s4cake21ProWithAssociatedTypePAAE9NonReqVarSivg",
                 "moduleName": "cake",
                 "genericSig": "<Self where Self : cake.ProWithAssociatedType>",
+                "isFromExtension": true,
                 "accessorKind": "get"
               }
             ]
@@ -977,7 +983,8 @@
             "usr": "s:4cake21ProWithAssociatedTypePAAE11NonReqAliasa",
             "mangledName": "$s4cake21ProWithAssociatedTypePAAE11NonReqAliasa",
             "moduleName": "cake",
-            "genericSig": "<Self where Self : cake.ProWithAssociatedType>"
+            "genericSig": "<Self where Self : cake.ProWithAssociatedType>",
+            "isFromExtension": true
           }
         ],
         "declKind": "Protocol",
@@ -1187,6 +1194,7 @@
             "mangledName": "$s4cake6PSuperPAAE9futureFooyyF",
             "moduleName": "cake",
             "genericSig": "<Self where Self : cake.PSuper>",
+            "isFromExtension": true,
             "funcSelfKind": "NonMutating"
           }
         ],
@@ -1508,6 +1516,7 @@
             "usr": "s:Si4cakeE3fooyyF",
             "mangledName": "$sSi4cakeE3fooyyF",
             "moduleName": "cake",
+            "isFromExtension": true,
             "funcSelfKind": "NonMutating"
           },
           {
@@ -1525,6 +1534,7 @@
             "usr": "s:Si4cakeE3baryyF",
             "mangledName": "$sSi4cakeE3baryyF",
             "moduleName": "cake",
+            "isFromExtension": true,
             "funcSelfKind": "NonMutating"
           }
         ],
@@ -1868,6 +1878,6 @@
         ]
       }
     ],
-    "json_format_version": 7
+    "json_format_version": 8
   }
 }
diff --git a/test/api-digester/Outputs/empty-baseline.json b/test/api-digester/Outputs/empty-baseline.json
@@ -3,6 +3,6 @@
     "kind": "Root",
     "name": "TopLevel",
     "printedName": "TopLevel",
-    "json_format_version": 7
+    "json_format_version": 8
   }
 }
diff --git a/test/api-digester/move-to-extension.swift b/test/api-digester/move-to-extension.swift
@@ -0,0 +1,23 @@
+// RUN: %empty-directory(%t)
+
+// RUN: %target-swift-frontend -emit-module -o %t/Foo.swiftmodule -emit-abi-descriptor-path %t/abi-before.json %s -enable-library-evolution -DBASELINE -emit-tbd-path %t/abi-before.tbd
+// RUN: %target-swift-frontend -emit-module -o %t/Foo.swiftmodule -emit-abi-descriptor-path %t/abi-after.json %s -enable-library-evolution -emit-tbd-path %t/abi-after.tbd
+// RUN: %api-digester -diagnose-sdk --input-paths %t/abi-before.json -input-paths %t/abi-after.json -abi -o %t/result.txt
+// RUN: %FileCheck %s < %t/result.txt
+
+#if BASELINE
+
+public class C {
+	public func foo() {}
+}
+
+#else
+
+public class C {}
+extension C {
+	public func foo() {}
+}
+
+#endif
+
+// CHECK: Non-final class member Func C.foo() is moved to extension

Original file line number	Diff line number	Diff line change
`@@ -64,6 +64,7 @@`
`64`	`64`	`"genericSig": "<τ_0_0 where τ_0_0 : cake.P1>",`
`65`	`65`	`"sugared_genericSig": "<Self where Self : cake.P1>",`
`66`	`66`	`"static": true,`
	`67`	`+ "isFromExtension": true,`
`67`	`68`	`"funcSelfKind": "NonMutating"`
`68`	`69`	`}`
`69`	`70`	`],`
`@@ -219,6 +220,7 @@`
`219`	`220`	`"moduleName": "cake",`
`220`	`221`	`"genericSig": "<τ_0_0, τ_0_1, τ_0_2 where τ_0_0 == cake.S1, τ_0_1 == cake.S1, τ_0_2 == cake.S1>",`
`221`	`222`	`"sugared_genericSig": "<T1, T2, T3 where T1 == cake.S1, T2 == cake.S1, T3 == cake.S1>",`
	`223`	`+ "isFromExtension": true,`
`222`	`224`	`"funcSelfKind": "NonMutating"`
`223`	`225`	`},`
`224`	`226`	`{`
`@@ -238,6 +240,7 @@`
`238`	`240`	`"moduleName": "cake",`
`239`	`241`	`"genericSig": "<τ_0_0, τ_0_1, τ_0_2>",`
`240`	`242`	`"sugared_genericSig": "<T1, T2, T3>",`
	`243`	`+ "isFromExtension": true,`
`241`	`244`	`"funcSelfKind": "NonMutating"`
`242`	`245`	`}`
`243`	`246`	`],`
`@@ -1021,6 +1024,7 @@`
`1021`	`1024`	`"moduleName": "cake",`
`1022`	`1025`	`"genericSig": "<τ_0_0 where τ_0_0 : cake.ProWithAssociatedType>",`
`1023`	`1026`	`"sugared_genericSig": "<Self where Self : cake.ProWithAssociatedType>",`
	`1027`	`+ "isFromExtension": true,`
`1024`	`1028`	`"funcSelfKind": "NonMutating"`
`1025`	`1029`	`},`
`1026`	`1030`	`{`
`@@ -1039,6 +1043,7 @@`
`1039`	`1043`	`"usr": "s:4cake21ProWithAssociatedTypePAAE9NonReqVarSivp",`
`1040`	`1044`	`"mangledName": "$s4cake21ProWithAssociatedTypePAAE9NonReqVarSivp",`
`1041`	`1045`	`"moduleName": "cake",`
	`1046`	`+ "isFromExtension": true,`
`1042`	`1047`	`"accessors": [`
`1043`	`1048`	`{`
`1044`	`1049`	`"kind": "Accessor",`
`@@ -1058,6 +1063,7 @@`
`1058`	`1063`	`"moduleName": "cake",`
`1059`	`1064`	`"genericSig": "<τ_0_0 where τ_0_0 : cake.ProWithAssociatedType>",`
`1060`	`1065`	`"sugared_genericSig": "<Self where Self : cake.ProWithAssociatedType>",`
	`1066`	`+ "isFromExtension": true,`
`1061`	`1067`	`"accessorKind": "get"`
`1062`	`1068`	`}`
`1063`	`1069`	`]`
`@@ -1305,6 +1311,7 @@`
`1305`	`1311`	`"moduleName": "cake",`
`1306`	`1312`	`"genericSig": "<τ_0_0 where τ_0_0 : cake.PSuper>",`
`1307`	`1313`	`"sugared_genericSig": "<Self where Self : cake.PSuper>",`
	`1314`	`+ "isFromExtension": true,`
`1308`	`1315`	`"funcSelfKind": "NonMutating"`
`1309`	`1316`	`}`
`1310`	`1317`	`],`
`@@ -1648,6 +1655,7 @@`
`1648`	`1655`	`"usr": "s:Si4cakeE3fooyyF",`
`1649`	`1656`	`"mangledName": "$sSi4cakeE3fooyyF",`
`1650`	`1657`	`"moduleName": "cake",`
	`1658`	`+ "isFromExtension": true,`
`1651`	`1659`	`"funcSelfKind": "NonMutating"`
`1652`	`1660`	`},`
`1653`	`1661`	`{`
`@@ -1665,6 +1673,7 @@`
`1665`	`1673`	`"usr": "s:Si4cakeE3baryyF",`
`1666`	`1674`	`"mangledName": "$sSi4cakeE3baryyF",`
`1667`	`1675`	`"moduleName": "cake",`
	`1676`	`+ "isFromExtension": true,`
`1668`	`1677`	`"funcSelfKind": "NonMutating"`
`1669`	`1678`	`}`
`1670`	`1679`	`],`
`@@ -2008,6 +2017,6 @@`
`2008`	`2017`	`]`
`2009`	`2018`	`}`
`2010`	`2019`	`],`
`2011`		`- "json_format_version": 7`
	`2020`	`+ "json_format_version": 8`
`2012`	`2021`	`}`
`2013`	`2022`	`}`
Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,6 @@`
`3`	`3`	`"kind": "Root",`
`4`	`4`	`"name": "TopLevel",`
`5`	`5`	`"printedName": "TopLevel",`
`6`		`- "json_format_version": 7`
	`6`	`+ "json_format_version": 8`
`7`	`7`	`}`
`8`	`8`	`}`