[AST] Cleanup AttrKind #71342
[AST] Cleanup AttrKind #71342
Conversation
Align with DeclAttrKind.
Align with other kind enum e.g. DeclKind.
Introduce NumDeclAttrKinds for number of enum values. Use optional for invalid attribute kind. This align with `TypeAttrKind`.
Instead of using dummy `_counting_KAK_*` global symbols, use `LAST_DECL_ATTR` metaprogramming technique to determine the number of enum values. This align with other "kind" enum e.g. `DeclKind`.
rintaro
requested review from
ahoppen,
bnbarham,
hamishknight,
QuietMisdreavus,
AnthonyLatsis,
hborla,
slavapestov,
xedin,
tshortli,
CodaFi,
DougGregor,
zoecarver,
hyp and
egorzhdan
as code owners
|
@swift-ci Please smoke test |
| #define TYPE_ATTR(X, C) ExcludeAttrs.insert(std::make_pair("TAK_" #X, TAK_##X)); | ||
| #include "swift/AST/Attr.def" | ||
| #define TYPE_ATTR(X, C) \ | ||
| ExcludeAttrs.insert(std::make_pair("TypeAttrKind::" #C, TypeAttrKind::C)); |
There was a problem hiding this comment.
This string is currently only used as keys of the temporary map. Changing the string value doesn't affect the functionality.
| #include "swift/AST/Attr.def" | ||
| // NOTE: For historical reasons. TypeAttribute uses the spelling, but | ||
| // DeclAttribute uses the kind name. | ||
| #define TYPE_ATTR(X, C) out.enumCase(value, #X, TypeAttrKind::C); |
There was a problem hiding this comment.
This is a little unfortunate, but we can't change swift-api-digester results.
| #define DECL_ATTR(_, NAME, ...) DAK_##NAME, | ||
| #include "swift/AST/Attr.def" | ||
| DAK_Count | ||
| enum class DeclAttrKind : unsigned { |
There was a problem hiding this comment.
Do you know if there's any reason DeclAttrKind has to be unsigned? If it's just for DAK_Count, NumDeclAttrKinds covers that now. Doesn't really matter either way though so 🤷 (just pointing it out since it seemed like part of the reason for this PR was to mostly "unify" decl/type attr kinds)
| GetScalarString(&N)); | ||
| return Result; | ||
| }); | ||
| for (auto &N : *Seq) { |
There was a problem hiding this comment.
Nice 👍 (though personally I'd type N since it's not super obvious what it is)
The main motivation is to make
DeclAttrKind/TypeAttrKindalign with other "kind" enums e.g.DeclKind.TypeAttrKind, use the class names instead of the spelling as the enum case names.DeclAttrKind::Availableinstead ofDAK_AvailableAttr.deftoDeclAttr.defandTypeAttr.def. Nothing was handling both attribute kind at the same time.DAK_Count(DeclAttrKind::Count). Instead, introduceNumDeclAttrKindsfor the number of the enum cases, and use optional to represent invalid attribute kind. (aligning withTypeAttrKind)LAST_TYPE_ATTRmetaprogramming macro to get the number of the enum cases, instead of using dummy_counting_TAK_*globally visible symbols. This aligns with other "kind" enums.