Skip to content

Commit 4189b6c

Browse files
committed
AST: promote API/ABI impact bit of decl attributes to AST, NFC
ABI/API checker used to hard-code whether adding or removing of a decl attribute could break the existing ABI/API. This is not ideal because new attributes may be added to AST without updating the checker. After this change, new decl attribute could be specified whether it has ABI/API impact and the checker could pick up the knowledge instantly.
1 parent 2163698 commit 4189b6c

File tree

5 files changed

+76
-40
lines changed

5 files changed

+76
-40
lines changed

include/swift/AST/Attr.def

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,12 @@ DECL_ATTR(available, Available,
121121
1)
122122
CONTEXTUAL_SIMPLE_DECL_ATTR(final, Final,
123123
OnClass | OnFunc | OnAccessor | OnVar | OnSubscript |
124-
DeclModifier,
124+
DeclModifier | ABIBreakingToAdd | ABIBreakingToRemove |
125+
APIBreakingToAdd,
125126
2)
126127
DECL_ATTR(objc, ObjC,
127128
OnAbstractFunction | OnClass | OnProtocol | OnExtension | OnVar |
128-
OnSubscript | OnEnum | OnEnumElement,
129+
OnSubscript | OnEnum | OnEnumElement | ABIBreakingToAdd | ABIBreakingToRemove,
129130
3)
130131
CONTEXTUAL_SIMPLE_DECL_ATTR(required, Required,
131132
OnConstructor |
@@ -187,7 +188,7 @@ DECL_ATTR(_semantics, Semantics,
187188
21)
188189
CONTEXTUAL_SIMPLE_DECL_ATTR(dynamic, Dynamic,
189190
OnFunc | OnAccessor | OnVar | OnSubscript | OnConstructor |
190-
DeclModifier,
191+
DeclModifier | ABIBreakingToAdd | ABIBreakingToRemove,
191192
22)
192193
CONTEXTUAL_SIMPLE_DECL_ATTR(infix, Infix,
193194
OnFunc | OnOperator |
@@ -212,7 +213,7 @@ SIMPLE_DECL_ATTR(nonobjc, NonObjC,
212213
30)
213214
SIMPLE_DECL_ATTR(_fixed_layout, FixedLayout,
214215
OnVar | OnClass | OnStruct |
215-
UserInaccessible,
216+
UserInaccessible | ABIBreakingToAdd | ABIBreakingToRemove,
216217
31)
217218
SIMPLE_DECL_ATTR(inlinable, Inlinable,
218219
OnVar | OnSubscript | OnAbstractFunction,
@@ -362,7 +363,7 @@ SIMPLE_DECL_ATTR(_weakLinked, WeakLinked,
362363
OnSubscript | OnConstructor | OnEnumElement | OnExtension | UserInaccessible,
363364
75)
364365
SIMPLE_DECL_ATTR(frozen, Frozen,
365-
OnEnum | OnStruct,
366+
OnEnum | OnStruct | ABIBreakingToAdd | ABIBreakingToRemove | APIBreakingToRemove,
366367
76)
367368
DECL_ATTR_ALIAS(_frozen, Frozen)
368369
SIMPLE_DECL_ATTR(_forbidSerializingReference, ForbidSerializingReference,

include/swift/AST/Attr.h

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,18 @@ class DeclAttribute : public AttributeBase {
353353

354354
/// Whether client code cannot use the attribute.
355355
UserInaccessible = 1ull << (unsigned(DeclKindIndex::Last_Decl) + 7),
356+
357+
/// Whether adding this attribute can break API
358+
APIBreakingToAdd = 1ull << (unsigned(DeclKindIndex::Last_Decl) + 8),
359+
360+
/// Whether removing this attribute can break API
361+
APIBreakingToRemove = 1ull << (unsigned(DeclKindIndex::Last_Decl) + 9),
362+
363+
/// Whether adding this attribute can break ABI
364+
ABIBreakingToAdd = 1ull << (unsigned(DeclKindIndex::Last_Decl) + 10),
365+
366+
/// Whether removing this attribute can break ABI
367+
ABIBreakingToRemove = 1ull << (unsigned(DeclKindIndex::Last_Decl) + 11),
356368
};
357369

358370
LLVM_READNONE
@@ -435,6 +447,21 @@ class DeclAttribute : public AttributeBase {
435447
return getOptions(DK) & UserInaccessible;
436448
}
437449

450+
static bool isAddingBreakingABI(DeclAttrKind DK) {
451+
return getOptions(DK) & ABIBreakingToAdd;
452+
}
453+
454+
static bool isAddingBreakingAPI(DeclAttrKind DK) {
455+
return getOptions(DK) & APIBreakingToAdd;
456+
}
457+
458+
static bool isRemovingBreakingABI(DeclAttrKind DK) {
459+
return getOptions(DK) & ABIBreakingToRemove;
460+
}
461+
static bool isRemovingBreakingAPI(DeclAttrKind DK) {
462+
return getOptions(DK) & APIBreakingToRemove;
463+
}
464+
438465
bool isDeclModifier() const {
439466
return isDeclModifier(getKind());
440467
}

test/api-digester/Outputs/Cake.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ cake: Func ownershipChange(_:_:) has parameter 1 changing from Shared to Owned
3838
cake: TypeAlias TChangesFromIntToString.T has underlying type change from Swift.Int to Swift.String
3939

4040
/* Decl Attribute changes */
41+
cake: Enum IceKind is now without @frozen
4142
cake: Func C1.foo1() is now not static
4243
cake: Func FinalFuncContainer.NewFinalFunc() is now with final
43-
cake: Func FinalFuncContainer.NoLongerFinalFunc() is now without final
4444
cake: Func HasMutatingMethodClone.foo() has self access kind changing from Mutating to NonMutating
4545
cake: Func S1.foo1() has self access kind changing from NonMutating to Mutating
4646
cake: Func S1.foo3() is now static

tools/swift-api-digester/ModuleAnalyzerNodes.cpp

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,6 @@ namespace fs = llvm::sys::fs;
1111
namespace path = llvm::sys::path;
1212

1313
namespace {
14-
static StringRef getAttrName(DeclAttrKind Kind) {
15-
switch (Kind) {
16-
#define DECL_ATTR(NAME, CLASS, ...) \
17-
case DAK_##CLASS: \
18-
return DeclAttribute::isDeclModifier(DAK_##CLASS) ? #NAME : "@"#NAME;
19-
#include "swift/AST/Attr.def"
20-
case DAK_Count:
21-
llvm_unreachable("unrecognized attribute kind.");
22-
}
23-
llvm_unreachable("covered switch");
24-
}
25-
2614
static PrintOptions getTypePrintOpts(CheckerOptions CheckerOpts) {
2715
PrintOptions Opts;
2816
Opts.SynthesizeSugarOnTypes = true;
@@ -33,7 +21,6 @@ static PrintOptions getTypePrintOpts(CheckerOptions CheckerOpts) {
3321
}
3422
return Opts;
3523
}
36-
3724
} // End of anonymous namespace.
3825

3926
struct swift::ide::api::SDKNodeInitInfo {
@@ -63,20 +50,7 @@ struct swift::ide::api::SDKNodeInitInfo {
6350
SDKNode* createSDKNode(SDKNodeKind Kind);
6451
};
6552

66-
SDKContext::SDKContext(CheckerOptions Opts): Diags(SourceMgr), Opts(Opts) {
67-
#define ADD(NAME) BreakingAttrs.push_back({DeclAttrKind::DAK_##NAME, \
68-
getAttrName(DeclAttrKind::DAK_##NAME)});
69-
// Add attributes that both break ABI and API.
70-
ADD(Final)
71-
if (checkingABI()) {
72-
// Add ABI-breaking-specific attributes.
73-
ADD(ObjC)
74-
ADD(FixedLayout)
75-
ADD(Frozen)
76-
ADD(Dynamic)
77-
}
78-
#undef ADD
79-
}
53+
SDKContext::SDKContext(CheckerOptions Opts): Diags(SourceMgr), Opts(Opts) {}
8054

8155
void SDKNodeRoot::registerDescendant(SDKNode *D) {
8256
// Operator doesn't have usr

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

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -824,6 +824,26 @@ void swift::ide::api::SDKNodeDeclFunction::diagnose(SDKNode *Right) {
824824
}
825825
}
826826

827+
static StringRef getAttrName(DeclAttrKind Kind) {
828+
switch (Kind) {
829+
#define DECL_ATTR(NAME, CLASS, ...) \
830+
case DAK_##CLASS: \
831+
return DeclAttribute::isDeclModifier(DAK_##CLASS) ? #NAME : "@"#NAME;
832+
#include "swift/AST/Attr.def"
833+
case DAK_Count:
834+
llvm_unreachable("unrecognized attribute kind.");
835+
}
836+
llvm_unreachable("covered switch");
837+
}
838+
839+
static bool shouldDiagnoseAddingAttribute(SDKNodeDecl *D, DeclAttrKind Kind) {
840+
return true;
841+
}
842+
843+
static bool shouldDiagnoseRemovingAttribute(SDKNodeDecl *D, DeclAttrKind Kind) {
844+
return true;
845+
}
846+
827847
void swift::ide::api::SDKNodeDecl::diagnose(SDKNode *Right) {
828848
SDKNode::diagnose(Right);
829849
auto *RD = dyn_cast<SDKNodeDecl>(Right);
@@ -882,13 +902,27 @@ void swift::ide::api::SDKNodeDecl::diagnose(SDKNode *Right) {
882902
}
883903
}
884904

885-
// Check if some attributes with ABI/API-impact have been added/removed.
886-
for (auto &Info: Ctx.getBreakingAttributeInfo()) {
887-
if (hasDeclAttribute(Info.Kind) != RD->hasDeclAttribute(Info.Kind)) {
888-
auto Desc = hasDeclAttribute(Info.Kind) ?
889-
Ctx.buffer((llvm::Twine("without ") + Info.Content).str()):
890-
Ctx.buffer((llvm::Twine("with ") + Info.Content).str());
891-
emitDiag(diag::decl_new_attr, Desc);
905+
// Diagnose removing attributes.
906+
for (auto Kind: getDeclAttributes()) {
907+
if (!RD->hasDeclAttribute(Kind)) {
908+
if ((Ctx.checkingABI() ? DeclAttribute::isRemovingBreakingABI(Kind) :
909+
DeclAttribute::isRemovingBreakingAPI(Kind)) &&
910+
shouldDiagnoseRemovingAttribute(this, Kind)) {
911+
emitDiag(diag::decl_new_attr,
912+
Ctx.buffer((llvm::Twine("without ") + getAttrName(Kind)).str()));
913+
}
914+
}
915+
}
916+
917+
// Diagnose adding attributes.
918+
for (auto Kind: RD->getDeclAttributes()) {
919+
if (!hasDeclAttribute(Kind)) {
920+
if ((Ctx.checkingABI() ? DeclAttribute::isAddingBreakingABI(Kind) :
921+
DeclAttribute::isAddingBreakingAPI(Kind)) &&
922+
shouldDiagnoseAddingAttribute(this, Kind)) {
923+
emitDiag(diag::decl_new_attr,
924+
Ctx.buffer((llvm::Twine("with ") + getAttrName(Kind)).str()));
925+
}
892926
}
893927
}
894928

0 commit comments

Comments
 (0)