Skip to content

Commit 94dc36d

Browse files
nkcsgexinkcsgexi
committed
ABI checker: prefer sugared version of generic signature while emitting diagnostics.
1 parent 76dd2db commit 94dc36d

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",
@@ -924,6 +929,7 @@
924929
"usr": "s:4cake21ProWithAssociatedTypePAAE10NonReqFuncyyF",
925930
"moduleName": "cake",
926931
"genericSig": "<τ_0_0 where τ_0_0 : ProWithAssociatedType>",
932+
"sugared_genericSig": "<Self where Self : ProWithAssociatedType>",
927933
"funcSelfKind": "NonMutating"
928934
},
929935
{
@@ -958,6 +964,7 @@
958964
"usr": "s:4cake21ProWithAssociatedTypePAAE9NonReqVarSivg",
959965
"moduleName": "cake",
960966
"genericSig": "<τ_0_0 where τ_0_0 : ProWithAssociatedType>",
967+
"sugared_genericSig": "<Self where Self : ProWithAssociatedType>",
961968
"accessorKind": "get"
962969
}
963970
]
@@ -994,6 +1001,7 @@
9941001
"usr": "s:4cake13SubsContainerP6getterS2i_tcip",
9951002
"moduleName": "cake",
9961003
"genericSig": "<τ_0_0 where τ_0_0 : SubsContainer>",
1004+
"sugared_genericSig": "<Self where Self : SubsContainer>",
9971005
"protocolReq": true,
9981006
"accessors": [
9991007
{
@@ -1018,6 +1026,7 @@
10181026
"usr": "s:4cake13SubsContainerP6getterS2i_tcig",
10191027
"moduleName": "cake",
10201028
"genericSig": "<τ_0_0 where τ_0_0 : SubsContainer>",
1029+
"sugared_genericSig": "<Self where Self : SubsContainer>",
10211030
"protocolReq": true,
10221031
"reqNewWitnessTableEntry": true,
10231032
"accessorKind": "get"
@@ -1046,6 +1055,7 @@
10461055
"usr": "s:4cake13SubsContainerP6setterS2i_tcip",
10471056
"moduleName": "cake",
10481057
"genericSig": "<τ_0_0 where τ_0_0 : SubsContainer>",
1058+
"sugared_genericSig": "<Self where Self : SubsContainer>",
10491059
"protocolReq": true,
10501060
"accessors": [
10511061
{
@@ -1070,6 +1080,7 @@
10701080
"usr": "s:4cake13SubsContainerP6setterS2i_tcig",
10711081
"moduleName": "cake",
10721082
"genericSig": "<τ_0_0 where τ_0_0 : SubsContainer>",
1083+
"sugared_genericSig": "<Self where Self : SubsContainer>",
10731084
"protocolReq": true,
10741085
"reqNewWitnessTableEntry": true,
10751086
"accessorKind": "get"
@@ -1101,6 +1112,7 @@
11011112
"usr": "s:4cake13SubsContainerP6setterS2i_tcis",
11021113
"moduleName": "cake",
11031114
"genericSig": "<τ_0_0 where τ_0_0 : SubsContainer>",
1115+
"sugared_genericSig": "<Self where Self : SubsContainer>",
11041116
"protocolReq": true,
11051117
"reqNewWitnessTableEntry": true,
11061118
"accessorKind": "set"
@@ -1126,6 +1138,7 @@
11261138
"usr": "s:4cake13SubsContainerP6setterS2i_tciM",
11271139
"moduleName": "cake",
11281140
"genericSig": "<τ_0_0 where τ_0_0 : SubsContainer>",
1141+
"sugared_genericSig": "<Self where Self : SubsContainer>",
11291142
"protocolReq": true,
11301143
"implicit": true,
11311144
"reqNewWitnessTableEntry": true,
@@ -1167,6 +1180,7 @@
11671180
"usr": "s:4cake6PSuperP3fooyyF",
11681181
"moduleName": "cake",
11691182
"genericSig": "<τ_0_0 where τ_0_0 : PSuper>",
1183+
"sugared_genericSig": "<Self where Self : PSuper>",
11701184
"protocolReq": true,
11711185
"reqNewWitnessTableEntry": true,
11721186
"funcSelfKind": "NonMutating"
@@ -1186,6 +1200,7 @@
11861200
"usr": "s:4cake6PSuperPAAE9futureFooyyF",
11871201
"moduleName": "cake",
11881202
"genericSig": "<τ_0_0 where τ_0_0 : PSuper>",
1203+
"sugared_genericSig": "<Self where Self : PSuper>",
11891204
"intro_Macosx": "10.15",
11901205
"intro_iOS": "13",
11911206
"intro_tvOS": "13",
@@ -1227,6 +1242,7 @@
12271242
"usr": "s:4cake4PSubP3fooyyF",
12281243
"moduleName": "cake",
12291244
"genericSig": "<τ_0_0 where τ_0_0 : PSub>",
1245+
"sugared_genericSig": "<Self where Self : PSub>",
12301246
"protocolReq": true,
12311247
"overriding": true,
12321248
"declAttributes": [
@@ -1239,6 +1255,7 @@
12391255
"usr": "s:4cake4PSubP",
12401256
"moduleName": "cake",
12411257
"genericSig": "<τ_0_0 : cake.PSuper>",
1258+
"sugared_genericSig": "<Self : cake.PSuper>",
12421259
"conformances": [
12431260
{
12441261
"kind": "Conformance",
@@ -1757,5 +1774,5 @@
17571774
]
17581775
}
17591776
],
1760-
"json_format_version": 1
1777+
"json_format_version": 2
17611778
}

‎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
@@ -92,7 +92,9 @@ SDKNodeDecl::SDKNodeDecl(SDKNodeInitInfo Info, SDKNodeKind Kind)
9292
IsOpen(Info.IsOpen),
9393
IsInternal(Info.IsInternal), IsABIPlaceholder(Info.IsABIPlaceholder),
9494
ReferenceOwnership(uint8_t(Info.ReferenceOwnership)),
95-
GenericSig(Info.GenericSig), FixedBinaryOrder(Info.FixedBinaryOrder),
95+
GenericSig(Info.GenericSig),
96+
SugaredGenericSig(Info.SugaredGenericSig),
97+
FixedBinaryOrder(Info.FixedBinaryOrder),
9698
introVersions({Info.IntromacOS, Info.IntroiOS, Info.IntrotvOS,
9799
Info.IntrowatchOS, Info.Introswift}){}
98100

@@ -212,13 +214,24 @@ SDKNode* SDKNode::getOnlyChild() const {
212214
}
213215

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

231+
uint8_t SDKNode::getJsonFormatVersion() const {
232+
return getRootNode()->getJsonFormatVersion();
233+
}
234+
222235
void SDKNode::addChild(SDKNode *Child) {
223236
Child->Parent = this;
224237
Children.push_back(Child);
@@ -1085,7 +1098,8 @@ Requirement getCanonicalRequirement(Requirement &Req) {
10851098
}
10861099

10871100
static
1088-
StringRef printGenericSignature(SDKContext &Ctx, ArrayRef<Requirement> AllReqs) {
1101+
StringRef printGenericSignature(SDKContext &Ctx, ArrayRef<Requirement> AllReqs,
1102+
bool Canonical) {
10891103
llvm::SmallString<32> Result;
10901104
llvm::raw_svector_ostream OS(Result);
10911105
if (AllReqs.empty())
@@ -1101,7 +1115,7 @@ StringRef printGenericSignature(SDKContext &Ctx, ArrayRef<Requirement> AllReqs)
11011115
} else {
11021116
First = false;
11031117
}
1104-
if (Ctx.checkingABI())
1118+
if (Canonical)
11051119
getCanonicalRequirement(Req).print(OS, Opts);
11061120
else
11071121
Req.print(OS, Opts);
@@ -1110,16 +1124,16 @@ StringRef printGenericSignature(SDKContext &Ctx, ArrayRef<Requirement> AllReqs)
11101124
return Ctx.buffer(OS.str());
11111125
}
11121126

1113-
static StringRef printGenericSignature(SDKContext &Ctx, Decl *D) {
1127+
static StringRef printGenericSignature(SDKContext &Ctx, Decl *D, bool Canonical) {
11141128
llvm::SmallString<32> Result;
11151129
llvm::raw_svector_ostream OS(Result);
11161130
if (auto *PD = dyn_cast<ProtocolDecl>(D)) {
1117-
return printGenericSignature(Ctx, PD->getRequirementSignature());
1131+
return printGenericSignature(Ctx, PD->getRequirementSignature(), Canonical);
11181132
}
11191133

11201134
if (auto *GC = D->getAsGenericContext()) {
11211135
if (auto *Sig = GC->getGenericSignature()) {
1122-
if (Ctx.checkingABI())
1136+
if (Canonical)
11231137
Sig->getCanonicalSignature()->print(OS);
11241138
else
11251139
Sig->print(OS);
@@ -1130,8 +1144,8 @@ static StringRef printGenericSignature(SDKContext &Ctx, Decl *D) {
11301144
}
11311145

11321146
static
1133-
StringRef printGenericSignature(SDKContext &Ctx, ProtocolConformance *Conf) {
1134-
return printGenericSignature(Ctx, Conf->getConditionalRequirements());
1147+
StringRef printGenericSignature(SDKContext &Ctx, ProtocolConformance *Conf, bool Canonical) {
1148+
return printGenericSignature(Ctx, Conf->getConditionalRequirements(), Canonical);
11351149
}
11361150

11371151
static Optional<uint8_t> getSimilarMemberCount(NominalTypeDecl *NTD,
@@ -1243,7 +1257,10 @@ SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, Decl *D):
12431257
Ctx(Ctx), DKind(D->getKind()),
12441258
Location(calculateLocation(Ctx, D)),
12451259
ModuleName(D->getModuleContext()->getName().str()),
1246-
GenericSig(printGenericSignature(Ctx, D)),
1260+
GenericSig(printGenericSignature(Ctx, D, /*Canonical*/Ctx.checkingABI())),
1261+
SugaredGenericSig(Ctx.checkingABI()?
1262+
printGenericSignature(Ctx, D, /*Canonical*/false):
1263+
StringRef()),
12471264
IntromacOS(Ctx.getPlatformIntroVersion(D, PlatformKind::OSX)),
12481265
IntroiOS(Ctx.getPlatformIntroVersion(D, PlatformKind::iOS)),
12491266
IntrotvOS(Ctx.getPlatformIntroVersion(D, PlatformKind::tvOS)),
@@ -1279,7 +1296,9 @@ SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, ProtocolConformance *Conform):
12791296
SDKNodeInitInfo(Ctx, Conform->getProtocol()) {
12801297
// The conformance can be conditional. The generic signature keeps track of
12811298
// the requirements.
1282-
GenericSig = printGenericSignature(Ctx, Conform);
1299+
GenericSig = printGenericSignature(Ctx, Conform, Ctx.checkingABI());
1300+
SugaredGenericSig = Ctx.checkingABI() ?
1301+
printGenericSignature(Ctx, Conform, false): StringRef();
12831302
// Whether this conformance is ABI placeholder depends on the decl context
12841303
// of this conformance.
12851304
IsABIPlaceholder = isABIPlaceholderRecursive(Conform->getDeclContext()->
@@ -1853,6 +1872,7 @@ void SDKNodeDecl::jsonize(json::Output &out) {
18531872
output(out, KeyKind::KK_location, Location);
18541873
output(out, KeyKind::KK_moduleName, ModuleName);
18551874
output(out, KeyKind::KK_genericSig, GenericSig);
1875+
output(out, KeyKind::KK_sugared_genericSig, SugaredGenericSig);
18561876
output(out, KeyKind::KK_static, IsStatic);
18571877
output(out, KeyKind::KK_deprecated,IsDeprecated);
18581878
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)