Skip to content

Commit 18ca90e

Browse files
nkcsgexinkcsgexi
authored
Merge pull request #59275 from nkcsgexi/abi-checker-fixes-cherry-pick
[5.7] Cherry-pick multiple ABI checker fixes
2 parents 350e87e + ab45b36 commit 18ca90e

16 files changed

+280
-12
lines changed

‎include/swift/APIDigester/ModuleAnalyzerNodes.h

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ namespace api {
6363
///
6464
/// When the json format changes in a way that requires version-specific handling, this number should be incremented.
6565
/// This ensures we could have backward compatibility so that version changes in the format won't stop the checker from working.
66-
const uint8_t DIGESTER_JSON_VERSION = 7; // push SDKNodeRoot to lower-level
66+
const uint8_t DIGESTER_JSON_VERSION = 8; // add isFromExtension
6767
const uint8_t DIGESTER_JSON_DEFAULT_VERSION = 0; // Use this version number for files before we have a version number in json.
6868
const StringRef ABIRootKey = "ABIRoot";
6969
const StringRef ConstValuesKey = "ConstValues";
@@ -161,6 +161,7 @@ struct CheckerOptions {
161161
bool Migrator;
162162
StringRef LocationFilter;
163163
std::vector<std::string> ToolArgs;
164+
llvm::StringSet<> SPIGroupNamesToIgnore;
164165
};
165166

166167
class SDKContext {
@@ -346,6 +347,7 @@ class SDKNodeDecl: public SDKNode {
346347
StringRef Location;
347348
StringRef ModuleName;
348349
std::vector<DeclAttrKind> DeclAttributes;
350+
std::vector<StringRef> SPIGroups;
349351
bool IsImplicit;
350352
bool IsStatic;
351353
bool IsDeprecated;
@@ -354,6 +356,7 @@ class SDKNodeDecl: public SDKNode {
354356
bool IsOpen;
355357
bool IsInternal;
356358
bool IsABIPlaceholder;
359+
bool IsFromExtension;
357360
uint8_t ReferenceOwnership;
358361
StringRef GenericSig;
359362
// In ABI mode, this field is populated as a user-friendly version of GenericSig.
@@ -372,6 +375,7 @@ class SDKNodeDecl: public SDKNode {
372375
StringRef getModuleName() const {return ModuleName;}
373376
StringRef getHeaderName() const;
374377
ArrayRef<DeclAttrKind> getDeclAttributes() const;
378+
ArrayRef<StringRef> getSPIGroups() const { return SPIGroups; }
375379
bool hasAttributeChange(const SDKNodeDecl &Another) const;
376380
swift::ReferenceOwnership getReferenceOwnership() const {
377381
return swift::ReferenceOwnership(ReferenceOwnership);
@@ -392,6 +396,7 @@ class SDKNodeDecl: public SDKNode {
392396
bool isOpen() const { return IsOpen; }
393397
bool isInternal() const { return IsInternal; }
394398
bool isABIPlaceholder() const { return IsABIPlaceholder; }
399+
bool isFromExtension() const { return IsFromExtension; }
395400
StringRef getGenericSignature() const { return GenericSig; }
396401
StringRef getSugaredGenericSignature() const { return SugaredGenericSig; }
397402
StringRef getScreenInfo() const;
@@ -413,6 +418,12 @@ class SDKNodeDecl: public SDKNode {
413418
if (isObjc())
414419
return;
415420
}
421+
// Don't emit SPIs if the group name is out-out.
422+
for (auto spi: getSPIGroups()) {
423+
if (Ctx.getOpts().SPIGroupNamesToIgnore.contains(spi)) {
424+
return;
425+
}
426+
}
416427
Ctx.getDiags(Loc).diagnose(Loc, ID, getScreenInfo(), std::move(Args)...);
417428
}
418429
};
@@ -710,6 +721,7 @@ class SDKNodeDeclConstructor: public SDKNodeDeclAbstractFunc {
710721
void jsonize(json::Output &Out) override;
711722
};
712723

724+
// Note: Accessor doesn't have Parent pointer.
713725
class SDKNodeDeclAccessor: public SDKNodeDeclAbstractFunc {
714726
SDKNodeDecl *Owner;
715727
AccessorKind AccKind;
@@ -828,6 +840,8 @@ int findDeclUsr(StringRef dumpPath, CheckerOptions Opts);
828840

829841
void nodeSetDifference(ArrayRef<SDKNode*> Left, ArrayRef<SDKNode*> Right,
830842
NodeVector &LeftMinusRight, NodeVector &RightMinusLeft);
843+
844+
bool hasValidParentPtr(SDKNodeKind kind);
831845
} // end of abi namespace
832846
} // end of ide namespace
833847
} // end of Swift namespace

‎include/swift/AST/ASTMangler.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,8 @@ class ASTMangler : public Mangler {
297297

298298
std::string mangleTypeAsContextUSR(const NominalTypeDecl *type);
299299

300-
std::string mangleAnyDecl(const ValueDecl *Decl, bool prefix);
300+
std::string mangleAnyDecl(const ValueDecl *Decl, bool prefix,
301+
bool respectOriginallyDefinedIn = false);
301302
std::string mangleDeclAsUSR(const ValueDecl *Decl, StringRef USRPrefix);
302303

303304
std::string mangleAccessorEntityAsUSR(AccessorKind kind,

‎include/swift/AST/DiagnosticsModuleDiffer.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ ERROR(type_witness_change,APIDigesterBreakage,"%0 has type witness type for %1 c
7676

7777
ERROR(decl_new_witness_table_entry,APIDigesterBreakage,"%0 now requires%select{| no}1 new witness table entry", (StringRef, bool))
7878

79+
ERROR(class_member_moved_to_extension,APIDigesterBreakage,"Non-final class member %0 is moved to extension", (StringRef))
80+
7981
WARNING(new_decl_without_intro,APIDigesterBreakage,"%0 is a new API without @available attribute", (StringRef))
8082

8183
ERROR(objc_name_change,APIDigesterBreakage,"%0 has ObjC name change from %1 to %2", (StringRef, StringRef, StringRef))

‎include/swift/AST/PrintOptions.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,9 @@ struct PrintOptions {
209209
/// \see FileUnit::getExportedModuleName
210210
bool UseExportedModuleNames = false;
211211

212+
/// Use the original module name to qualify a symbol.
213+
bool UseOriginallyDefinedInModuleNames = false;
214+
212215
/// Print Swift.Array and Swift.Optional with sugared syntax
213216
/// ([] and ?), even if there are no sugar type nodes.
214217
bool SynthesizeSugarOnTypes = false;

‎include/swift/IDE/DigesterEnums.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ KEY_BOOL(IsExternal, isExternal)
136136
KEY_BOOL(IsEnumExhaustive, isEnumExhaustive)
137137
KEY_BOOL(HasMissingDesignatedInitializers, hasMissingDesignatedInitializers)
138138
KEY_BOOL(InheritsConvenienceInitializers, inheritsConvenienceInitializers)
139+
KEY_BOOL(IsFromExtension, isFromExtension)
139140

140141
KEY(kind)
141142

@@ -161,6 +162,7 @@ KEY_STRING(InitKind, init_kind)
161162

162163
KEY_STRING_ARR(SuperclassNames, superclassNames)
163164
KEY_STRING_ARR(ToolArgs, tool_arguments)
165+
KEY_STRING_ARR(SPIGroups, spi_group_names)
164166

165167
KEY_UINT(SelfIndex, selfIndex)
166168
KEY_UINT(FixedBinaryOrder, fixedbinaryorder)

‎include/swift/Option/Options.td

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1402,6 +1402,10 @@ def ignored_usrs: Separate<["-", "--"], "ignored-usrs">,
14021402
HelpText<"the file containing USRs of removed decls that the digester should ignore">,
14031403
MetaVarName<"<path>">;
14041404

1405+
def ignore_spi_groups : Separate<["-", "--"], "ignore-spi-group">,
1406+
Flags<[NoDriverOption, SwiftAPIDigesterOption]>,
1407+
HelpText<"SPI group name to not diagnose about">;
1408+
14051409
def protocol_requirement_allow_list: Separate<["-", "--"], "protocol-requirement-allow-list">,
14061410
Flags<[NoDriverOption, SwiftAPIDigesterOption, ArgumentIsPath]>,
14071411
HelpText<"File containing a new-line separated list of protocol names">,

‎lib/APIDigester/ModuleAnalyzerNodes.cpp

Lines changed: 71 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ namespace {
1717
static PrintOptions getTypePrintOpts(CheckerOptions CheckerOpts) {
1818
PrintOptions Opts;
1919
Opts.SynthesizeSugarOnTypes = true;
20+
Opts.UseOriginallyDefinedInModuleNames = true;
2021
if (!CheckerOpts.Migrator) {
2122
// We should always print fully qualified type names for checking either
2223
// API or ABI stability.
@@ -54,6 +55,16 @@ struct swift::ide::api::SDKNodeInitInfo {
5455
SDKNode* createSDKNode(SDKNodeKind Kind);
5556
};
5657

58+
bool swift::ide::api::hasValidParentPtr(SDKNodeKind kind) {
59+
switch(kind) {
60+
case SDKNodeKind::Conformance:
61+
case SDKNodeKind::DeclAccessor:
62+
return false;
63+
default:
64+
return true;
65+
}
66+
}
67+
5768
SDKContext::SDKContext(CheckerOptions Opts): Diags(SourceMgr), Opts(Opts) {}
5869

5970
DiagnosticEngine &SDKContext::getDiags(SourceLoc Loc) {
@@ -103,12 +114,15 @@ SDKNodeDecl::SDKNodeDecl(SDKNodeInitInfo Info, SDKNodeKind Kind)
103114
: SDKNode(Info, Kind), DKind(Info.DKind), Usr(Info.Usr),
104115
MangledName(Info.MangledName), Loc(Info.Loc),
105116
Location(Info.Location), ModuleName(Info.ModuleName),
106-
DeclAttributes(Info.DeclAttrs), IsImplicit(Info.IsImplicit),
117+
DeclAttributes(Info.DeclAttrs),
118+
SPIGroups(Info.SPIGroups),
119+
IsImplicit(Info.IsImplicit),
107120
IsStatic(Info.IsStatic), IsDeprecated(Info.IsDeprecated),
108121
IsProtocolReq(Info.IsProtocolReq),
109122
IsOverriding(Info.IsOverriding),
110123
IsOpen(Info.IsOpen),
111124
IsInternal(Info.IsInternal), IsABIPlaceholder(Info.IsABIPlaceholder),
125+
IsFromExtension(Info.IsFromExtension),
112126
ReferenceOwnership(uint8_t(Info.ReferenceOwnership)),
113127
GenericSig(Info.GenericSig),
114128
SugaredGenericSig(Info.SugaredGenericSig),
@@ -826,6 +840,25 @@ static bool hasSameParameterFlags(const SDKNodeType *Left, const SDKNodeType *Ri
826840
return true;
827841
}
828842

843+
// Return whether a decl has been moved in/out to an extension
844+
static Optional<bool> isFromExtensionChanged(const SDKNode &L, const SDKNode &R) {
845+
assert(L.getKind() == R.getKind());
846+
// Version 8 starts to include whether a decl is from an extension.
847+
if (L.getJsonFormatVersion() + R.getJsonFormatVersion() < 2 * 8) {
848+
return llvm::None;
849+
}
850+
auto *Left = dyn_cast<SDKNodeDecl>(&L);
851+
auto *Right = dyn_cast<SDKNodeDecl>(&R);
852+
if (!Left) {
853+
return llvm::None;
854+
}
855+
if (Left->isFromExtension() == Right->isFromExtension()) {
856+
return llvm::None;
857+
} else {
858+
return Right->isFromExtension();
859+
}
860+
}
861+
829862
static bool isSDKNodeEqual(SDKContext &Ctx, const SDKNode &L, const SDKNode &R) {
830863
auto *LeftAlias = dyn_cast<SDKNodeTypeAlias>(&L);
831864
auto *RightAlias = dyn_cast<SDKNodeTypeAlias>(&R);
@@ -965,6 +998,8 @@ static bool isSDKNodeEqual(SDKContext &Ctx, const SDKNode &L, const SDKNode &R)
965998
case SDKNodeKind::TypeWitness:
966999
case SDKNodeKind::DeclImport:
9671000
case SDKNodeKind::Root: {
1001+
if (isFromExtensionChanged(L, R))
1002+
return false;
9681003
return L.getPrintedName() == R.getPrintedName() &&
9691004
L.hasSameChildren(R);
9701005
}
@@ -1064,7 +1099,8 @@ static StringRef calculateMangledName(SDKContext &Ctx, ValueDecl *VD) {
10641099
return Ctx.buffer(attr->Name);
10651100
}
10661101
Mangle::ASTMangler NewMangler;
1067-
return Ctx.buffer(NewMangler.mangleAnyDecl(VD, true));
1102+
return Ctx.buffer(NewMangler.mangleAnyDecl(VD, true,
1103+
/*bool respectOriginallyDefinedIn*/true));
10681104
}
10691105

10701106
static StringRef calculateLocation(SDKContext &SDKCtx, Decl *D) {
@@ -1376,6 +1412,13 @@ StringRef SDKContext::getInitKind(Decl *D) {
13761412
return StringRef();
13771413
}
13781414

1415+
static bool isDeclaredInExtension(Decl *D) {
1416+
if (auto *DC = D->getDeclContext()->getAsDecl()) {
1417+
return isa<ExtensionDecl>(DC);
1418+
}
1419+
return false;
1420+
}
1421+
13791422
SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, Decl *D):
13801423
Ctx(Ctx), DKind(D->getKind()), Loc(D->getLoc()),
13811424
Location(calculateLocation(Ctx, D)),
@@ -1394,7 +1437,13 @@ SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, Decl *D):
13941437
IsImplicit(D->isImplicit()),
13951438
IsDeprecated(D->getAttrs().getDeprecated(D->getASTContext())),
13961439
IsABIPlaceholder(isABIPlaceholderRecursive(D)),
1397-
DeclAttrs(collectDeclAttributes(D)) {}
1440+
IsFromExtension(isDeclaredInExtension(D)),
1441+
DeclAttrs(collectDeclAttributes(D)) {
1442+
// Keep track of SPI group names
1443+
for (auto id: D->getSPIGroups()) {
1444+
SPIGroups.push_back(id.str());
1445+
}
1446+
}
13981447

13991448
SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, OperatorDecl *OD):
14001449
SDKNodeInitInfo(Ctx, cast<Decl>(OD)) {
@@ -2038,6 +2087,8 @@ void SDKNodeDecl::jsonize(json::Output &out) {
20382087
uint8_t Raw = uint8_t(getReferenceOwnership());
20392088
out.mapRequired(getKeyContent(Ctx, KeyKind::KK_ownership).data(), Raw);
20402089
}
2090+
output(out, KeyKind::KK_isFromExtension, IsFromExtension);
2091+
out.mapOptional(getKeyContent(Ctx, KeyKind::KK_spi_group_names).data(), SPIGroups);
20412092
}
20422093

20432094
void SDKNodeDeclAbstractFunc::jsonize(json::Output &out) {
@@ -2609,6 +2660,23 @@ void swift::ide::api::SDKNodeDeclAbstractFunc::diagnose(SDKNode *Right) {
26092660
if (reqNewWitnessTableEntry() != R->reqNewWitnessTableEntry()) {
26102661
emitDiag(Loc, diag::decl_new_witness_table_entry, reqNewWitnessTableEntry());
26112662
}
2663+
2664+
// Diagnose moving a non-final class member to an extension.
2665+
if (hasValidParentPtr(getKind())) {
2666+
while(auto *parent = dyn_cast<SDKNodeDecl>(getParent())) {
2667+
if (parent->getDeclKind() != DeclKind::Class) {
2668+
break;
2669+
}
2670+
if (hasDeclAttribute(DeclAttrKind::DAK_Final)) {
2671+
break;
2672+
}
2673+
auto result = isFromExtensionChanged(*this, *Right);
2674+
if (result.hasValue() && *result) {
2675+
emitDiag(Loc, diag::class_member_moved_to_extension);
2676+
}
2677+
break;
2678+
}
2679+
}
26122680
}
26132681
}
26142682

‎lib/AST/ASTMangler.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -759,9 +759,12 @@ std::string ASTMangler::mangleTypeAsUSR(Type Ty) {
759759
return finalize();
760760
}
761761

762-
std::string ASTMangler::mangleAnyDecl(const ValueDecl *Decl, bool prefix) {
762+
std::string
763+
ASTMangler::mangleAnyDecl(const ValueDecl *Decl,
764+
bool prefix,
765+
bool respectOriginallyDefinedIn) {
763766
DWARFMangling = true;
764-
RespectOriginallyDefinedIn = false;
767+
RespectOriginallyDefinedIn = respectOriginallyDefinedIn;
765768
if (prefix) {
766769
beginMangling();
767770
} else {

‎lib/AST/ASTPrinter.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5252,6 +5252,16 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
52525252
Name = Mod->getASTContext().getIdentifier(ExportedModuleName);
52535253
}
52545254

5255+
if (Options.UseOriginallyDefinedInModuleNames) {
5256+
Decl *D = Ty->getDecl();
5257+
for (auto attr: D->getAttrs().getAttributes<OriginallyDefinedInAttr>()) {
5258+
Name = Mod->getASTContext()
5259+
.getIdentifier(const_cast<OriginallyDefinedInAttr*>(attr)
5260+
->OriginalModuleName);
5261+
break;
5262+
}
5263+
}
5264+
52555265
Printer.printModuleRef(Mod, Name);
52565266
Printer << ".";
52575267
}

‎lib/DriverTool/swift_api_digester_main.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2338,7 +2338,8 @@ class SwiftAPIDigesterInvocation {
23382338
CompilerStyleDiags || !SerializedDiagPath.empty();
23392339
for (auto Arg : Args)
23402340
CheckerOpts.ToolArgs.push_back(Arg);
2341-
2341+
for(auto spi: ParsedArgs.getAllArgValues(OPT_ignore_spi_groups))
2342+
CheckerOpts.SPIGroupNamesToIgnore.insert(spi);
23422343
if (!SDK.empty()) {
23432344
auto Ver = getSDKBuildVersion(SDK);
23442345
if (!Ver.empty()) {

‎test/api-digester/Inputs/cake.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,3 +145,11 @@ public func emitIntoClientFunc() {}
145145

146146
@_silgen_name("silgenName")
147147
public func silgenNamedFunc() {}
148+
149+
@available(OSX 10.7, *)
150+
@_originallyDefinedIn(module: "Bread", OSX 10.9)
151+
@_spi(top_secret_1)
152+
@_spi(top_secret_2)
153+
public class SinkingClass {
154+
public init() {}
155+
}

0 commit comments

Comments
 (0)