Index: include/clang/Basic/Attr.td =================================================================== --- include/clang/Basic/Attr.td +++ include/clang/Basic/Attr.td @@ -675,6 +675,28 @@ let Documentation = [EnableIfDocs]; } +def EnumRole : InheritableAttr { + let Spellings = [GNU<"enum_role">]; + let Subjects = SubjectList<[Enum]>; + let Documentation = [EnumRoleDocs]; + let LangOpts = [COnly]; + let Args = [EnumArgument<"Role", "EnumRole", + ["choice", "options"], + ["ER_Choice", "ER_Options"]>]; + let AdditionalMembers = [{ +private: + llvm::APInt OptionBits; +public: + llvm::APInt &getOptionBits() { + return OptionBits; + } + + const llvm::APInt &getOptionBits() const { + return OptionBits; + } +}]; +} + def ExtVectorType : Attr { let Spellings = [GNU<"ext_vector_type">]; let Subjects = SubjectList<[TypedefName], ErrorDiag>; @@ -709,25 +731,6 @@ let Documentation = [Undocumented]; } -def FlagEnum : InheritableAttr { - let Spellings = [GNU<"flag_enum">]; - let Subjects = SubjectList<[Enum]>; - let Documentation = [FlagEnumDocs]; - let LangOpts = [COnly]; - let AdditionalMembers = [{ -private: - llvm::APInt FlagBits; -public: - llvm::APInt &getFlagBits() { - return FlagBits; - } - - const llvm::APInt &getFlagBits() const { - return FlagBits; - } -}]; -} - def Flatten : InheritableAttr { let Spellings = [GCC<"flatten">]; let Subjects = SubjectList<[Function], ErrorDiag>; Index: include/clang/Basic/AttrDocs.td =================================================================== --- include/clang/Basic/AttrDocs.td +++ include/clang/Basic/AttrDocs.td @@ -1196,13 +1196,36 @@ }]; } -def FlagEnumDocs : Documentation { +def EnumRoleDocs : Documentation { let Category = DocCatType; let Content = [{ -This attribute can be added to an enumerator to signal to the compiler that it -is intended to be used as a flag type. This will cause the compiler to assume -that the range of the type includes all of the values that you can get by -manipulating bits of the enumerator when issuing warnings. +The enum_role attribute can be added to an enum declaration to provide a hint to +the compiler about its meaning. Currently, two roles are recognized. + +The 'choice' role, which is the default, means that the enum is intended to be +used as a set of distinct values, and a value outside the defined enumerators is +invalid. + +The 'options' role means that the enum is intended to be used as a set of +bitwise options, and a valid value may have any or all of the enumerator bits +set. + + .. code-block:: c + + enum __attribute__((enum_role(choice))) choice { + choice_1 = 0, + choice_2 = 1, + choice_3 = 2, + choice_4 = 3, + }; + + enum __attribute__((enum_role(options))) options { + option_1 = 0x1, + option_2 = 0x2, + option_3 = 0x4, + option_4 = 0x8, + option_5 = 0x10, + }; }]; } Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -7966,11 +7966,11 @@ Expr *SrcExpr, AssignmentAction Action, bool *Complained = nullptr); - /// IsValueInFlagEnum - Determine if a value is allowed as part of a flag - /// enum. If AllowMask is true, then we also allow the complement of a valid - /// value, to be used as a mask. - bool IsValueInFlagEnum(const EnumDecl *ED, const llvm::APInt &Val, - bool AllowMask) const; + /// IsValueInOptionsEnum - Determine if a value is allowed as part of an + /// options enum. If AllowMask is true, then we also allow the complement of a + /// valid value, to be used as a mask. + bool IsValueInOptionsEnum(const EnumDecl *ED, const llvm::APInt &Val, + bool AllowMask) const; /// DiagnoseAssignmentEnum - Warn if assignment to enum is a constant /// integer not in the range of enum values. Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -13505,26 +13505,27 @@ } bool -Sema::IsValueInFlagEnum(const EnumDecl *ED, const llvm::APInt &Val, - bool AllowMask) const { - FlagEnumAttr *FEAttr = ED->getAttr(); - assert(FEAttr && "looking for value in non-flag enum"); +Sema::IsValueInOptionsEnum(const EnumDecl *ED, const llvm::APInt &Val, + bool AllowMask) const { + EnumRoleAttr *ERAttr = ED->getAttr(); + assert(ERAttr && ERAttr->getRole() == EnumRoleAttr::ER_Options && + "looking for value in non-flag enum"); - llvm::APInt FlagMask = ~FEAttr->getFlagBits(); - unsigned Width = FlagMask.getBitWidth(); + llvm::APInt OptionsMask = ~ERAttr->getOptionBits(); + unsigned Width = OptionsMask.getBitWidth(); // We will try a zero-extended value for the regular check first. llvm::APInt ExtVal = Val.zextOrSelf(Width); - // A value is in a flag enum if either its bits are a subset of the enum's - // flag bits (the first condition) or we are allowing masks and the same is + // A value is in an options enum if either its bits are a subset of the enum's + // foptionsbits (the first condition) or we are allowing masks and the same is // true of its complement (the second condition). When masks are allowed, we // allow the common idiom of ~(enum1 | enum2) to be a valid enum value. // // While it's true that any value could be used as a mask, the assumption is // that a mask will have all of the insignificant bits set. Anything else is // likely a logic error. - if (!(FlagMask & ExtVal)) + if (!(OptionsMask & ExtVal)) return true; if (AllowMask) { @@ -13540,7 +13541,7 @@ // though, it can be fixed by making it a signed type (e.g. ~0x1), so it may // be fine just to accept this as a warning. ExtVal |= llvm::APInt::getHighBitsSet(Width, Width - Val.getBitWidth()); - if (!(FlagMask & ~ExtVal)) + if (!(OptionsMask & ~ExtVal)) return true; } @@ -13698,9 +13699,9 @@ } } - FlagEnumAttr *FEAttr = Enum->getAttr(); - if (FEAttr) - FEAttr->getFlagBits() = llvm::APInt(BestWidth, 0); + EnumRoleAttr *ERAttr = Enum->getAttr(); + if (ERAttr) + ERAttr->getOptionBits() = llvm::APInt(BestWidth, 0); // Loop over all of the enumerator constants, changing their types to match // the type of the enum if needed. If we have a flag type, we also prepare the @@ -13767,28 +13768,27 @@ flagbits: // Check to see if we have a constant with exactly one bit set. Note that x // & (x - 1) will be nonzero if and only if x has more than one bit set. - if (FEAttr) { + if (ERAttr && ERAttr->getRole() == EnumRoleAttr::ER_Options) { llvm::APInt ExtVal = InitVal.zextOrSelf(BestWidth); if (ExtVal != 0 && !(ExtVal & (ExtVal - 1))) { - FEAttr->getFlagBits() |= ExtVal; + ERAttr->getOptionBits() |= ExtVal; } } } - if (FEAttr) { + if (ERAttr && ERAttr->getRole() == EnumRoleAttr::ER_Options) { for (Decl *D : Elements) { EnumConstantDecl *ECD = cast_or_null(D); if (!ECD) continue; // Already issued a diagnostic. llvm::APSInt InitVal = ECD->getInitVal(); - if (InitVal != 0 && !IsValueInFlagEnum(Enum, InitVal, true)) + if (InitVal != 0 && !IsValueInOptionsEnum(Enum, InitVal, true)) Diag(ECD->getLocation(), diag::warn_flag_enum_constant_out_of_range) << ECD << Enum; } } - Enum->completeDefinition(BestType, BestPromotionType, NumPositiveBits, NumNegativeBits); Index: lib/Sema/SemaDeclAttr.cpp =================================================================== --- lib/Sema/SemaDeclAttr.cpp +++ lib/Sema/SemaDeclAttr.cpp @@ -1024,6 +1024,36 @@ Attr.getAttributeSpellingListIndex())); } +static void handleEnumRoleAttr(Sema &S, Decl *D, const AttributeList &Attr) { + if (!Attr.isArgIdent(0)) { + S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type) + << Attr.getName() << 1 << AANT_ArgumentIdentifier; + return; + } + + IdentifierLoc *IL = Attr.getArgAsIdent(0); + EnumRoleAttr::EnumRole ER; + if (!EnumRoleAttr::ConvertStrToEnumRole(IL->Ident->getName(), ER)) { + S.Diag(IL->Loc, diag::warn_attribute_type_not_supported) << Attr.getName() + << IL->Ident; + return; + } + + if (EnumRoleAttr *ERAttr = D->getAttr()) { + if (ERAttr->getRole() == ER) + S.Diag(Attr.getLoc(), diag::warn_duplicate_attribute_exact) + << Attr.getName(); + else + S.Diag(Attr.getLoc(), diag::warn_duplicate_attribute) << Attr.getName(); + + return; + } + + D->addAttr(::new (S.Context) + EnumRoleAttr(Attr.getLoc(), S.Context, ER, + Attr.getAttributeSpellingListIndex())); +} + static void handleExtVectorTypeAttr(Sema &S, Scope *scope, Decl *D, const AttributeList &Attr) { // Remember this typedef decl, we will need it later for diagnostics. @@ -4403,6 +4433,9 @@ case AttributeList::AT_EnableIf: handleEnableIfAttr(S, D, Attr); break; + case AttributeList::AT_EnumRole: + handleEnumRoleAttr(S, D, Attr); + break; case AttributeList::AT_ExtVectorType: handleExtVectorTypeAttr(S, scope, D, Attr); break; @@ -4412,9 +4445,6 @@ case AttributeList::AT_OptimizeNone: handleOptimizeNoneAttr(S, D, Attr); break; - case AttributeList::AT_FlagEnum: - handleSimpleAttribute(S, D, Attr); - break; case AttributeList::AT_Flatten: handleSimpleAttribute(S, D, Attr); break; Index: lib/Sema/SemaStmt.cpp =================================================================== --- lib/Sema/SemaStmt.cpp +++ lib/Sema/SemaStmt.cpp @@ -697,8 +697,6 @@ EnumValsTy::iterator &EI, EnumValsTy::iterator &EIEnd, const llvm::APSInt &Val) { - bool FlagType = ED->hasAttr(); - if (const DeclRefExpr *DRE = dyn_cast(CaseExpr->IgnoreParenImpCasts())) { if (const VarDecl *VD = dyn_cast(DRE->getDecl())) { @@ -710,8 +708,9 @@ } } - if (FlagType) { - return !S.IsValueInFlagEnum(ED, Val, false); + EnumRoleAttr *ERAttr = ED->getAttr(); + if (ERAttr && ERAttr->getRole() == EnumRoleAttr::ER_Options) { + return !S.IsValueInOptionsEnum(ED, Val, false); } else { while (EI != EIEnd && EI->first < Val) EI++; @@ -1198,8 +1197,9 @@ AdjustAPSInt(RhsVal, DstWidth, DstIsSigned); const EnumDecl *ED = ET->getDecl(); - if (ED->hasAttr()) { - if (!IsValueInFlagEnum(ED, RhsVal, true)) + EnumRoleAttr *ERAttr = ED->getAttr(); + if (ERAttr && ERAttr->getRole() == EnumRoleAttr::ER_Options) { + if (!IsValueInOptionsEnum(ED, RhsVal, true)) Diag(SrcExpr->getExprLoc(), diag::warn_not_in_enum_assignment) << DstType.getUnqualifiedType(); } else { Index: test/Sema/attr-enum-role.c =================================================================== --- /dev/null +++ test/Sema/attr-enum-role.c @@ -0,0 +1,90 @@ +// RUN: %clang_cc1 -verify -fsyntax-only -std=c11 -Wassign-enum %s + +__attribute__((enum_role(options))) int i; // expected-warning {{only applies to enums}} +typedef struct __attribute__((enum_role(choice))) s { } t; // expected-warning {{only applies to enums}} + +enum __attribute__((enum_role(options))) flag { + ea = 0x1, + eb = 0x2, + ec = 0x8, +}; + +enum __attribute__((enum_role(options))) flag2 { + ga = 0x1, + gb = 0x4, + + gc = 0x5, // no-warning + gd = 0x7, // expected-warning {{enumeration value 'gd' is out of range}} + ge = ~0x2, // expected-warning {{enumeration value 'ge' is out of range}} + gf = ~0x4, // no-warning + gg = ~0x1, // no-warning + gh = ~0x5, // no-warning + gi = ~0x11, // expected-warning {{enumeration value 'gi' is out of range}} +}; + +enum __attribute__((enum_role(options))) flag3 { + fa = 0x1, + fb = ~0x1u, // no-warning +}; + +// What happens here is that ~0x2 is negative, and so the enum must be signed. +// But ~0x1u is unsigned and has the high bit set, so the enum must be 64-bit. +// The result is that ~0x1u does not have high bits set, and so it is considered +// to be an invalid value. See Sema::IsValueInOptionsEnum in SemaDecl.cpp for +// more discussion. +enum __attribute__((enum_role(options))) flag4 { + ha = 0x1, + hb = 0x2, + + hc = ~0x1u, // expected-warning {{enumeration value 'hc' is out of range}} + hd = ~0x2, // no-warning +}; + +enum __attribute__((enum_role(choice))) choice { + za = 0x1, + zb = 0x2, +}; + +enum __attribute__((enum_role)) invalid1 { inv1 }; // expected-error {{takes one argument}} +enum __attribute__((enum_role(foo))) invalid2 { inv2 }; // expected-warning {{argument not supported}} +enum __attribute__((enum_role(1))) invalid3 { inv3 }; // expected-error {{requires parameter 1 to be an identifier}} + +enum __attribute__((enum_role(options), enum_role(options))) dupgood { optionforsure }; // expected-warning {{already applied}} +enum __attribute__((enum_role(options), enum_role(choice))) dupbad { confusion }; // expected-warning {{already applied with different parameters}} + +void f(void) { + enum flag e = 0; // no-warning + e = 0x1; // no-warning + e = 0x3; // no-warning + e = 0xa; // no-warning + e = 0x4; // expected-warning {{integer constant not in range of enumerated type}} + e = 0xf; // expected-warning {{integer constant not in range of enumerated type}} + e = ~0; // no-warning + e = ~0x1; // no-warning + e = ~0x2; // no-warning + e = ~0x3; // no-warning + e = ~0x4; // expected-warning {{integer constant not in range of enumerated type}} + + switch (e) { + case 0: break; // no-warning + case 0x1: break; // no-warning + case 0x3: break; // no-warning + case 0xa: break; // no-warning + case 0x4: break; // expected-warning {{case value not in enumerated type}} + case 0xf: break; // expected-warning {{case value not in enumerated type}} + case ~0: break; // expected-warning {{case value not in enumerated type}} + case ~0x1: break; // expected-warning {{case value not in enumerated type}} + case ~0x2: break; // expected-warning {{case value not in enumerated type}} + case ~0x3: break; // expected-warning {{case value not in enumerated type}} + case ~0x4: break; // expected-warning {{case value not in enumerated type}} + default: break; + } + + enum flag2 f = ~0x1; // no-warning + f = ~0x1u; // no-warning + + enum flag4 h = ~0x1; // no-warning + h = ~0x1u; // expected-warning {{integer constant not in range of enumerated type}} + + enum choice z = 0x3; // expected-warning {{integer constant not in range of enumerated type}} +} Index: test/Sema/attr-flag-enum.c =================================================================== --- test/Sema/attr-flag-enum.c +++ /dev/null @@ -1,73 +0,0 @@ -// RUN: %clang_cc1 -verify -fsyntax-only -std=c11 -Wassign-enum %s - -enum __attribute__((flag_enum)) flag { - ea = 0x1, - eb = 0x2, - ec = 0x8, -}; - -enum __attribute__((flag_enum)) flag2 { - ga = 0x1, - gb = 0x4, - - gc = 0x5, // no-warning - gd = 0x7, // expected-warning {{enumeration value 'gd' is out of range}} - ge = ~0x2, // expected-warning {{enumeration value 'ge' is out of range}} - gf = ~0x4, // no-warning - gg = ~0x1, // no-warning - gh = ~0x5, // no-warning - gi = ~0x11, // expected-warning {{enumeration value 'gi' is out of range}} -}; - -enum __attribute__((flag_enum)) flag3 { - fa = 0x1, - fb = ~0x1u, // no-warning -}; - -// What happens here is that ~0x2 is negative, and so the enum must be signed. -// But ~0x1u is unsigned and has the high bit set, so the enum must be 64-bit. -// The result is that ~0x1u does not have high bits set, and so it is considered -// to be an invalid value. See Sema::IsValueInFlagEnum in SemaDecl.cpp for more -// discussion. -enum __attribute__((flag_enum)) flag4 { - ha = 0x1, - hb = 0x2, - - hc = ~0x1u, // expected-warning {{enumeration value 'hc' is out of range}} - hd = ~0x2, // no-warning -}; - -void f(void) { - enum flag e = 0; // no-warning - e = 0x1; // no-warning - e = 0x3; // no-warning - e = 0xa; // no-warning - e = 0x4; // expected-warning {{integer constant not in range of enumerated type}} - e = 0xf; // expected-warning {{integer constant not in range of enumerated type}} - e = ~0; // no-warning - e = ~0x1; // no-warning - e = ~0x2; // no-warning - e = ~0x3; // no-warning - e = ~0x4; // expected-warning {{integer constant not in range of enumerated type}} - - switch (e) { - case 0: break; // no-warning - case 0x1: break; // no-warning - case 0x3: break; // no-warning - case 0xa: break; // no-warning - case 0x4: break; // expected-warning {{case value not in enumerated type}} - case 0xf: break; // expected-warning {{case value not in enumerated type}} - case ~0: break; // expected-warning {{case value not in enumerated type}} - case ~0x1: break; // expected-warning {{case value not in enumerated type}} - case ~0x2: break; // expected-warning {{case value not in enumerated type}} - case ~0x3: break; // expected-warning {{case value not in enumerated type}} - case ~0x4: break; // expected-warning {{case value not in enumerated type}} - default: break; - } - - enum flag2 f = ~0x1; // no-warning - f = ~0x1u; // no-warning - - enum flag4 h = ~0x1; // no-warning - h = ~0x1u; // expected-warning {{integer constant not in range of enumerated type}} -} Index: test/SemaCXX/attr-enum-role.cpp =================================================================== --- /dev/null +++ test/SemaCXX/attr-enum-role.cpp @@ -0,0 +1,3 @@ +// RUN: %clang_cc1 -verify -fsyntax-only -std=c++14 %s + +enum __attribute__((enum_role(options))) wrong_lang { yerp }; // expected-warning {{attribute ignored}}