Skip to content

Commit 34b7afe

Browse files
authored
Merge pull request #26833 from nkcsgexi/sugared-generic-sig
ABI checker: prefer sugared version of generic signature while emitting diagnostics
2 parents d086fcf + 03bd23e commit 34b7afe

File tree

9 files changed

+73
-22
lines changed

9 files changed

+73
-22
lines changed

include/swift/IDE/DigesterEnums.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ KEY_STRING(ModuleName, moduleName)
143143
KEY_STRING(SuperclassUsr, superclassUsr)
144144
KEY_STRING(EnumRawTypeName, enumRawTypeName)
145145
KEY_STRING(GenericSig, genericSig)
146+
KEY_STRING(SugaredGenericSig, sugared_genericSig)
146147
KEY_STRING(FuncSelfKind, funcSelfKind)
147148
KEY_STRING(ParamValueOwnership, paramValueOwnership)
148149
KEY_STRING(IntromacOS, intro_Macosx)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11

22
/* Generic Signature Changes */
3-
cake: Func P1.P1Constraint() has generic signature change from <τ_0_0 where τ_0_0 : P1, τ_0_0 : P2> to <τ_0_0 where τ_0_0 : P1>
4-
cake: Protocol P3 has generic signature change from <τ_0_0 : cake.P1, τ_0_0 : cake.P2> to <τ_0_0 : cake.P1, τ_0_0 : cake.P4>
3+
cake: Func P1.P1Constraint() has generic signature change from <Self where Self : P1, Self : P2> to <Self where Self : P1>
4+
cake: Protocol P3 has generic signature change from <Self : cake.P1, Self : cake.P2> to <Self : cake.P1, Self : cake.P4>
55

66
/* RawRepresentable Changes */
77

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
"usr": "s:4cake2P1PAAE1poiyAaB_pAaB_p_AaB_ptFZ",
3737
"moduleName": "cake",
3838
"genericSig": "<τ_0_0 where τ_0_0 : P1>",
39+
"sugared_genericSig": "<Self where Self : P1>",
3940
"static": true,
4041
"funcSelfKind": "NonMutating"
4142
}
@@ -60,6 +61,7 @@
6061
"usr": "s:4cake2P3P",
6162
"moduleName": "cake",
6263
"genericSig": "<τ_0_0 : cake.P1, τ_0_0 : cake.P2>",
64+
"sugared_genericSig": "<Self : cake.P1, Self : cake.P2>",
6365
"conformances": [
6466
{
6567
"kind": "Conformance",
@@ -171,6 +173,7 @@
171173
"usr": "s:4cake2C0CA2A2S1VRszAERs_AERs0_rlE17conditionalFooExtyyF",
172174
"moduleName": "cake",
173175
"genericSig": "<τ_0_0, τ_0_1, τ_0_2 where τ_0_0 == S1, τ_0_1 == S1, τ_0_2 == S1>",
176+
"sugared_genericSig": "<T1, T2, T3 where T1 == S1, T2 == S1, T3 == S1>",
174177
"funcSelfKind": "NonMutating"
175178
},
176179
{
@@ -188,13 +191,15 @@
188191
"usr": "s:4cake2C0C19unconditionalFooExtyyF",
189192
"moduleName": "cake",
190193
"genericSig": "<τ_0_0, τ_0_1, τ_0_2>",
194+
"sugared_genericSig": "<T1, T2, T3>",
191195
"funcSelfKind": "NonMutating"
192196
}
193197
],
194198
"declKind": "Class",
195199
"usr": "s:4cake2C0C",
196200
"moduleName": "cake",
197-
"genericSig": "<τ_0_0, τ_0_1, τ_0_2>"
201+
"genericSig": "<τ_0_0, τ_0_1, τ_0_2>",
202+
"sugared_genericSig": "<T1, T2, T3>"
198203
},
199204
{
200205
"kind": "TypeDecl",
@@ -921,6 +926,7 @@
921926
"usr": "s:4cake21ProWithAssociatedTypePAAE10NonReqFuncyyF",
922927
"moduleName": "cake",
923928
"genericSig": "<τ_0_0 where τ_0_0 : ProWithAssociatedType>",
929+
"sugared_genericSig": "<Self where Self : ProWithAssociatedType>",
924930
"funcSelfKind": "NonMutating"
925931
},
926932
{
@@ -955,6 +961,7 @@
955961
"usr": "s:4cake21ProWithAssociatedTypePAAE9NonReqVarSivg",
956962
"moduleName": "cake",
957963
"genericSig": "<τ_0_0 where τ_0_0 : ProWithAssociatedType>",
964+
"sugared_genericSig": "<Self where Self : ProWithAssociatedType>",
958965
"accessorKind": "get"
959966
}
960967
]
@@ -991,6 +998,7 @@
991998
"usr": "s:4cake13SubsContainerP6getterS2i_tcip",
992999
"moduleName": "cake",
9931000
"genericSig": "<τ_0_0 where τ_0_0 : SubsContainer>",
1001+
"sugared_genericSig": "<Self where Self : SubsContainer>",
9941002
"protocolReq": true,
9951003
"accessors": [
9961004
{
@@ -1015,6 +1023,7 @@
10151023
"usr": "s:4cake13SubsContainerP6getterS2i_tcig",
10161024
"moduleName": "cake",
10171025
"genericSig": "<τ_0_0 where τ_0_0 : SubsContainer>",
1026+
"sugared_genericSig": "<Self where Self : SubsContainer>",
10181027
"protocolReq": true,
10191028
"reqNewWitnessTableEntry": true,
10201029
"accessorKind": "get"
@@ -1043,6 +1052,7 @@
10431052
"usr": "s:4cake13SubsContainerP6setterS2i_tcip",
10441053
"moduleName": "cake",
10451054
"genericSig": "<τ_0_0 where τ_0_0 : SubsContainer>",
1055+
"sugared_genericSig": "<Self where Self : SubsContainer>",
10461056
"protocolReq": true,
10471057
"accessors": [
10481058
{
@@ -1067,6 +1077,7 @@
10671077
"usr": "s:4cake13SubsContainerP6setterS2i_tcig",
10681078
"moduleName": "cake",
10691079
"genericSig": "<τ_0_0 where τ_0_0 : SubsContainer>",
1080+
"sugared_genericSig": "<Self where Self : SubsContainer>",
10701081
"protocolReq": true,
10711082
"reqNewWitnessTableEntry": true,
10721083
"accessorKind": "get"
@@ -1098,6 +1109,7 @@
10981109
"usr": "s:4cake13SubsContainerP6setterS2i_tcis",
10991110
"moduleName": "cake",
11001111
"genericSig": "<τ_0_0 where τ_0_0 : SubsContainer>",
1112+
"sugared_genericSig": "<Self where Self : SubsContainer>",
11011113
"protocolReq": true,
11021114
"reqNewWitnessTableEntry": true,
11031115
"accessorKind": "set"
@@ -1123,6 +1135,7 @@
11231135
"usr": "s:4cake13SubsContainerP6setterS2i_tciM",
11241136
"moduleName": "cake",
11251137
"genericSig": "<τ_0_0 where τ_0_0 : SubsContainer>",
1138+
"sugared_genericSig": "<Self where Self : SubsContainer>",
11261139
"protocolReq": true,
11271140
"implicit": true,
11281141
"reqNewWitnessTableEntry": true,
@@ -1164,6 +1177,7 @@
11641177
"usr": "s:4cake6PSuperP3fooyyF",
11651178
"moduleName": "cake",
11661179
"genericSig": "<τ_0_0 where τ_0_0 : PSuper>",
1180+
"sugared_genericSig": "<Self where Self : PSuper>",
11671181
"protocolReq": true,
11681182
"reqNewWitnessTableEntry": true,
11691183
"funcSelfKind": "NonMutating"
@@ -1183,6 +1197,7 @@
11831197
"usr": "s:4cake6PSuperPAAE9futureFooyyF",
11841198
"moduleName": "cake",
11851199
"genericSig": "<τ_0_0 where τ_0_0 : PSuper>",
1200+
"sugared_genericSig": "<Self where Self : PSuper>",
11861201
"intro_Macosx": "10.15",
11871202
"intro_iOS": "13",
11881203
"intro_tvOS": "13",
@@ -1224,6 +1239,7 @@
12241239
"usr": "s:4cake4PSubP3fooyyF",
12251240
"moduleName": "cake",
12261241
"genericSig": "<τ_0_0 where τ_0_0 : PSub>",
1242+
"sugared_genericSig": "<Self where Self : PSub>",
12271243
"protocolReq": true,
12281244
"overriding": true,
12291245
"declAttributes": [
@@ -1236,6 +1252,7 @@
12361252
"usr": "s:4cake4PSubP",
12371253
"moduleName": "cake",
12381254
"genericSig": "<τ_0_0 : cake.PSuper>",
1255+
"sugared_genericSig": "<Self : cake.PSuper>",
12391256
"conformances": [
12401257
{
12411258
"kind": "Conformance",
@@ -1754,5 +1771,5 @@
17541771
]
17551772
}
17561773
],
1757-
"json_format_version": 1
1774+
"json_format_version": 2
17581775
}

test/api-digester/Outputs/cake.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1622,5 +1622,5 @@
16221622
]
16231623
}
16241624
],
1625-
"json_format_version": 1
1625+
"json_format_version": 2
16261626
}

test/api-digester/Outputs/clang-module-dump.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,5 +169,5 @@
169169
]
170170
}
171171
],
172-
"json_format_version": 1
172+
"json_format_version": 2
173173
}

test/api-digester/Outputs/empty-baseline.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22
"kind": "Root",
33
"name": "TopLevel",
44
"printedName": "TopLevel",
5-
"json_format_version": 1
5+
"json_format_version": 2
66
}

tools/swift-api-digester/ModuleAnalyzerNodes.cpp

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,9 @@ SDKNodeDecl::SDKNodeDecl(SDKNodeInitInfo Info, SDKNodeKind Kind)
9393
IsOpen(Info.IsOpen),
9494
IsInternal(Info.IsInternal), IsABIPlaceholder(Info.IsABIPlaceholder),
9595
ReferenceOwnership(uint8_t(Info.ReferenceOwnership)),
96-
GenericSig(Info.GenericSig), FixedBinaryOrder(Info.FixedBinaryOrder),
96+
GenericSig(Info.GenericSig),
97+
SugaredGenericSig(Info.SugaredGenericSig),
98+
FixedBinaryOrder(Info.FixedBinaryOrder),
9799
introVersions({Info.IntromacOS, Info.IntroiOS, Info.IntrotvOS,
98100
Info.IntrowatchOS, Info.Introswift}){}
99101

@@ -213,13 +215,24 @@ SDKNode* SDKNode::getOnlyChild() const {
213215
}
214216

215217
SDKNodeRoot *SDKNode::getRootNode() const {
216-
for (auto *Root = const_cast<SDKNode*>(this); ; Root = Root->getParent()) {
218+
for (auto *Root = const_cast<SDKNode*>(this); Root;) {
217219
if (auto Result = dyn_cast<SDKNodeRoot>(Root))
218220
return Result;
221+
if (auto *Conf = dyn_cast<SDKNodeConformance>(Root)) {
222+
Root = Conf->getNominalTypeDecl();
223+
} else if (auto *Acc = dyn_cast<SDKNodeDeclAccessor>(Root)) {
224+
Root = Acc->getStorage();
225+
} else {
226+
Root = Root->getParent();
227+
}
219228
}
220229
llvm_unreachable("Unhandled SDKNodeKind in switch.");
221230
}
222231

232+
uint8_t SDKNode::getJsonFormatVersion() const {
233+
return getRootNode()->getJsonFormatVersion();
234+
}
235+
223236
void SDKNode::addChild(SDKNode *Child) {
224237
Child->Parent = this;
225238
Children.push_back(Child);
@@ -1087,7 +1100,8 @@ Requirement getCanonicalRequirement(Requirement &Req) {
10871100
}
10881101

10891102
static
1090-
StringRef printGenericSignature(SDKContext &Ctx, ArrayRef<Requirement> AllReqs) {
1103+
StringRef printGenericSignature(SDKContext &Ctx, ArrayRef<Requirement> AllReqs,
1104+
bool Canonical) {
10911105
llvm::SmallString<32> Result;
10921106
llvm::raw_svector_ostream OS(Result);
10931107
if (AllReqs.empty())
@@ -1103,7 +1117,7 @@ StringRef printGenericSignature(SDKContext &Ctx, ArrayRef<Requirement> AllReqs)
11031117
} else {
11041118
First = false;
11051119
}
1106-
if (Ctx.checkingABI())
1120+
if (Canonical)
11071121
getCanonicalRequirement(Req).print(OS, Opts);
11081122
else
11091123
Req.print(OS, Opts);
@@ -1112,16 +1126,16 @@ StringRef printGenericSignature(SDKContext &Ctx, ArrayRef<Requirement> AllReqs)
11121126
return Ctx.buffer(OS.str());
11131127
}
11141128

1115-
static StringRef printGenericSignature(SDKContext &Ctx, Decl *D) {
1129+
static StringRef printGenericSignature(SDKContext &Ctx, Decl *D, bool Canonical) {
11161130
llvm::SmallString<32> Result;
11171131
llvm::raw_svector_ostream OS(Result);
11181132
if (auto *PD = dyn_cast<ProtocolDecl>(D)) {
1119-
return printGenericSignature(Ctx, PD->getRequirementSignature());
1133+
return printGenericSignature(Ctx, PD->getRequirementSignature(), Canonical);
11201134
}
11211135

11221136
if (auto *GC = D->getAsGenericContext()) {
11231137
if (auto *Sig = GC->getGenericSignature()) {
1124-
if (Ctx.checkingABI())
1138+
if (Canonical)
11251139
Sig->getCanonicalSignature()->print(OS);
11261140
else
11271141
Sig->print(OS);
@@ -1132,8 +1146,8 @@ static StringRef printGenericSignature(SDKContext &Ctx, Decl *D) {
11321146
}
11331147

11341148
static
1135-
StringRef printGenericSignature(SDKContext &Ctx, ProtocolConformance *Conf) {
1136-
return printGenericSignature(Ctx, Conf->getConditionalRequirements());
1149+
StringRef printGenericSignature(SDKContext &Ctx, ProtocolConformance *Conf, bool Canonical) {
1150+
return printGenericSignature(Ctx, Conf->getConditionalRequirements(), Canonical);
11371151
}
11381152

11391153
static Optional<uint8_t> getSimilarMemberCount(NominalTypeDecl *NTD,
@@ -1245,7 +1259,10 @@ SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, Decl *D):
12451259
Ctx(Ctx), DKind(D->getKind()),
12461260
Location(calculateLocation(Ctx, D)),
12471261
ModuleName(D->getModuleContext()->getName().str()),
1248-
GenericSig(printGenericSignature(Ctx, D)),
1262+
GenericSig(printGenericSignature(Ctx, D, /*Canonical*/Ctx.checkingABI())),
1263+
SugaredGenericSig(Ctx.checkingABI()?
1264+
printGenericSignature(Ctx, D, /*Canonical*/false):
1265+
StringRef()),
12491266
IntromacOS(Ctx.getPlatformIntroVersion(D, PlatformKind::OSX)),
12501267
IntroiOS(Ctx.getPlatformIntroVersion(D, PlatformKind::iOS)),
12511268
IntrotvOS(Ctx.getPlatformIntroVersion(D, PlatformKind::tvOS)),
@@ -1281,7 +1298,9 @@ SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, ProtocolConformance *Conform):
12811298
SDKNodeInitInfo(Ctx, Conform->getProtocol()) {
12821299
// The conformance can be conditional. The generic signature keeps track of
12831300
// the requirements.
1284-
GenericSig = printGenericSignature(Ctx, Conform);
1301+
GenericSig = printGenericSignature(Ctx, Conform, Ctx.checkingABI());
1302+
SugaredGenericSig = Ctx.checkingABI() ?
1303+
printGenericSignature(Ctx, Conform, false): StringRef();
12851304
// Whether this conformance is ABI placeholder depends on the decl context
12861305
// of this conformance.
12871306
IsABIPlaceholder = isABIPlaceholderRecursive(Conform->getDeclContext()->
@@ -1858,6 +1877,7 @@ void SDKNodeDecl::jsonize(json::Output &out) {
18581877
output(out, KeyKind::KK_location, Location);
18591878
output(out, KeyKind::KK_moduleName, ModuleName);
18601879
output(out, KeyKind::KK_genericSig, GenericSig);
1880+
output(out, KeyKind::KK_sugared_genericSig, SugaredGenericSig);
18611881
output(out, KeyKind::KK_static, IsStatic);
18621882
output(out, KeyKind::KK_deprecated,IsDeprecated);
18631883
output(out, KeyKind::KK_protocolReq, IsProtocolReq);

tools/swift-api-digester/ModuleAnalyzerNodes.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ namespace api {
6262
///
6363
/// When the json format changes in a way that requires version-specific handling, this number should be incremented.
6464
/// This ensures we could have backward compatibility so that version changes in the format won't stop the checker from working.
65-
const uint8_t DIGESTER_JSON_VERSION = 1; // Adding digester_version to the format
65+
const uint8_t DIGESTER_JSON_VERSION = 2; // Adding sugared generic signature to the format
6666
const uint8_t DIGESTER_JSON_DEFAULT_VERSION = 0; // Use this version number for files before we have a version number in json.
6767

6868
class SDKNode;
@@ -294,6 +294,8 @@ class SDKNode {
294294
SDKNode* getOnlyChild() const;
295295
SDKContext &getSDKContext() const { return Ctx; }
296296
SDKNodeRoot *getRootNode() const;
297+
uint8_t getJsonFormatVersion() const;
298+
bool versionAtLeast(uint8_t Ver) const { return getJsonFormatVersion() >= Ver; }
297299
virtual void jsonize(json::Output &Out);
298300
virtual void diagnose(SDKNode *Right) {};
299301
template <typename T> const T *getAs() const {
@@ -335,6 +337,9 @@ class SDKNodeDecl: public SDKNode {
335337
bool IsABIPlaceholder;
336338
uint8_t ReferenceOwnership;
337339
StringRef GenericSig;
340+
// In ABI mode, this field is populated as a user-friendly version of GenericSig.
341+
// Dignostic preferes the sugared versions if they differ as well.
342+
StringRef SugaredGenericSig;
338343
Optional<uint8_t> FixedBinaryOrder;
339344
PlatformIntroVersion introVersions;
340345

@@ -368,6 +373,7 @@ class SDKNodeDecl: public SDKNode {
368373
bool isInternal() const { return IsInternal; }
369374
bool isABIPlaceholder() const { return IsABIPlaceholder; }
370375
StringRef getGenericSignature() const { return GenericSig; }
376+
StringRef getSugaredGenericSignature() const { return SugaredGenericSig; }
371377
StringRef getScreenInfo() const;
372378
bool hasFixedBinaryOrder() const { return FixedBinaryOrder.hasValue(); }
373379
uint8_t getFixedBinaryOrder() const { return *FixedBinaryOrder; }
@@ -401,7 +407,7 @@ class SDKNodeRoot: public SDKNode {
401407
static bool classof(const SDKNode *N);
402408
void registerDescendant(SDKNode *D);
403409
virtual void jsonize(json::Output &Out) override;
404-
Optional<uint8_t> getJsonFormatVersion() const { return JsonFormatVer; }
410+
uint8_t getJsonFormatVersion() const { return JsonFormatVer; }
405411
ArrayRef<SDKNodeDecl*> getDescendantsByUsr(StringRef Usr) {
406412
return DescendantDeclTable[Usr].getArrayRef();
407413
}

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -834,8 +834,15 @@ void swift::ide::api::SDKNodeDecl::diagnose(SDKNode *Right) {
834834
}
835835
// Diagnose generic signature change
836836
if (getGenericSignature() != RD->getGenericSignature()) {
837-
emitDiag(diag::generic_sig_change,
838-
getGenericSignature(), RD->getGenericSignature());
837+
// Prefer sugared signature in diagnostics to be more user-friendly.
838+
if (versionAtLeast(2) && RD->versionAtLeast(2) &&
839+
getSugaredGenericSignature() != RD->getSugaredGenericSignature()) {
840+
emitDiag(diag::generic_sig_change,
841+
getSugaredGenericSignature(), RD->getSugaredGenericSignature());
842+
} else {
843+
emitDiag(diag::generic_sig_change,
844+
getGenericSignature(), RD->getGenericSignature());
845+
}
839846
}
840847
if (isOptional() != RD->isOptional()) {
841848
if (Ctx.checkingABI()) {

0 commit comments

Comments
 (0)