From 3c268af42f61c171da25180e826c15cce428a000 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Tue, 21 Mar 2017 02:23:00 +0000 Subject: [PATCH] Add support for attribute enum_extensibility. This commit adds support for a new attribute that will be used to distinguish between extensible and inextensible enums. There are three main purposes of this attribute: 1. Give better control over when enum-related warnings are issued. For example, in the code below, clang will not issue a -Wassign-enum warning if the enum is marked "open": enum __attribute__((enum_extensibility(closed))) EnumClosed { B0 = 1, B1 = 10 }; enum __attribute__((enum_extensibility(open))) EnumOpen { C0 = 1, C1 = 10 }; enum EnumClosed ec = 100; // warning issued enum EnumOpen eo = 100; // no warning 2. Enable code-completion and debugging tools to offer better suggestions. 3. Make it easier for swift's clang importer to determine which swift type an enum should be mapped to. For more details, see the discussion I started on cfe-dev: http://lists.llvm.org/pipermail/cfe-dev/2017-February/052748.html rdar://problem/12764379 rdar://problem/23145650 Differential Revision: https://reviews.llvm.org/D30766 llvm-svn: 298332 --- clang/include/clang/AST/Decl.h | 12 ++ clang/include/clang/Basic/Attr.td | 10 +- clang/include/clang/Basic/AttrDocs.td | 49 +++++++ clang/lib/AST/Decl.cpp | 14 ++ clang/lib/Sema/SemaDecl.cpp | 4 +- clang/lib/Sema/SemaDeclAttr.cpp | 25 ++++ clang/lib/Sema/SemaStmt.cpp | 21 +-- clang/test/Sema/enum-attr.c | 130 +++++++++++++++++++ clang/test/SemaCXX/attr-flag-enum-reject.cpp | 4 - clang/test/SemaCXX/enum-attr.cpp | 108 +++++++++++++++ 10 files changed, 362 insertions(+), 15 deletions(-) create mode 100644 clang/test/Sema/enum-attr.c delete mode 100644 clang/test/SemaCXX/attr-flag-enum-reject.cpp create mode 100644 clang/test/SemaCXX/enum-attr.cpp diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index d1c77f5e080c..623d9c4792e7 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -3235,6 +3235,18 @@ public: return isCompleteDefinition() || isFixed(); } + /// Returns true if this enum is either annotated with + /// enum_extensibility(closed) or isn't annotated with enum_extensibility. + bool isClosed() const; + + /// Returns true if this enum is annotated with flag_enum and isn't annotated + /// with enum_extensibility(open). + bool isClosedFlag() const; + + /// Returns true if this enum is annotated with neither flag_enum nor + /// enum_extensibility(open). + bool isClosedNonFlag() const; + /// \brief Retrieve the enum definition from which this enumeration could /// be instantiated, if it is an instantiation (rather than a non-template). EnumDecl *getTemplateInstantiationPattern() const; diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 5960cca1e795..ff62e10f8adb 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -881,7 +881,15 @@ def FlagEnum : InheritableAttr { let Spellings = [GNU<"flag_enum">]; let Subjects = SubjectList<[Enum]>; let Documentation = [FlagEnumDocs]; - let LangOpts = [COnly]; +} + +def EnumExtensibility : InheritableAttr { + let Spellings = [GNU<"enum_extensibility">, + CXX11<"clang", "enum_extensibility">]; + let Subjects = SubjectList<[Enum]>; + let Args = [EnumArgument<"Extensibility", "Kind", + ["closed", "open"], ["Closed", "Open"]>]; + let Documentation = [EnumExtensibilityDocs]; } def Flatten : InheritableAttr { diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index df299e07c673..23b9bb7188e7 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -1963,6 +1963,55 @@ manipulating bits of the enumerator when issuing warnings. }]; } +def EnumExtensibilityDocs : Documentation { + let Category = DocCatType; + let Content = [{ +Attribute ``enum_extensibility`` is used to distinguish between enum definitions +that are extensible and those that are not. The attribute can take either +``closed`` or ``open`` as an argument. ``closed`` indicates a variable of the +enum type takes a value that corresponds to one of the enumerators listed in the +enum definition or, when the enum is annotated with ``flag_enum``, a value that +can be constructed using values corresponding to the enumerators. ``open`` +indicates a variable of the enum type can take any values allowed by the +standard and instructs clang to be more lenient when issuing warnings. + +.. code-block:: c + + enum __attribute__((enum_extensibility(closed))) ClosedEnum { + A0, A1 + }; + + enum __attribute__((enum_extensibility(open))) OpenEnum { + B0, B1 + }; + + enum __attribute__((enum_extensibility(closed),flag_enum)) ClosedFlagEnum { + C0 = 1 << 0, C1 = 1 << 1 + }; + + enum __attribute__((enum_extensibility(open),flag_enum)) OpenFlagEnum { + D0 = 1 << 0, D1 = 1 << 1 + }; + + void foo1() { + enum ClosedEnum ce; + enum OpenEnum oe; + enum ClosedFlagEnum cfe; + enum OpenFlagEnum ofe; + + ce = A1; // no warnings + ce = 100; // warning issued + oe = B1; // no warnings + oe = 100; // no warnings + cfe = C0 | C1; // no warnings + cfe = C0 | C1 | 4; // warning issued + ofe = D0 | D1; // no warnings + ofe = D0 | D1 | 4; // no warnings + } + + }]; +} + def EmptyBasesDocs : Documentation { let Category = DocCatType; let Content = [{ diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index cc6f2fa70541..5b0404e38dde 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -3742,6 +3742,20 @@ void EnumDecl::completeDefinition(QualType NewType, TagDecl::completeDefinition(); } +bool EnumDecl::isClosed() const { + if (const auto *A = getAttr()) + return A->getExtensibility() == EnumExtensibilityAttr::Closed; + return true; +} + +bool EnumDecl::isClosedFlag() const { + return isClosed() && hasAttr(); +} + +bool EnumDecl::isClosedNonFlag() const { + return isClosed() && !hasAttr(); +} + TemplateSpecializationKind EnumDecl::getTemplateSpecializationKind() const { if (MemberSpecializationInfo *MSI = getMemberSpecializationInfo()) return MSI->getTemplateSpecializationKind(); diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index d7d71221b5de..bc5f60504df2 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -15358,7 +15358,7 @@ static void CheckForDuplicateEnumValues(Sema &S, ArrayRef Elements, bool Sema::IsValueInFlagEnum(const EnumDecl *ED, const llvm::APInt &Val, bool AllowMask) const { - assert(ED->hasAttr() && "looking for value in non-flag enum"); + assert(ED->isClosedFlag() && "looking for value in non-flag or open enum"); assert(ED->isCompleteDefinition() && "expected enum definition"); auto R = FlagBitsCache.insert(std::make_pair(ED, llvm::APInt())); @@ -15603,7 +15603,7 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange, CheckForDuplicateEnumValues(*this, Elements, Enum, EnumType); - if (Enum->hasAttr()) { + if (Enum->isClosedFlag()) { for (Decl *D : Elements) { EnumConstantDecl *ECD = cast_or_null(D); if (!ECD) continue; // Already issued a diagnostic. diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 3979383966c8..1c350a7e38b7 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -2943,6 +2943,28 @@ static void handleCleanupAttr(Sema &S, Decl *D, const AttributeList &Attr) { Attr.getAttributeSpellingListIndex())); } +static void handleEnumExtensibilityAttr(Sema &S, Decl *D, + const AttributeList &Attr) { + if (!Attr.isArgIdent(0)) { + S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type) + << Attr.getName() << 0 << AANT_ArgumentIdentifier; + return; + } + + EnumExtensibilityAttr::Kind ExtensibilityKind; + IdentifierInfo *II = Attr.getArgAsIdent(0)->Ident; + if (!EnumExtensibilityAttr::ConvertStrToKind(II->getName(), + ExtensibilityKind)) { + S.Diag(Attr.getLoc(), diag::warn_attribute_type_not_supported) + << Attr.getName() << II; + return; + } + + D->addAttr(::new (S.Context) EnumExtensibilityAttr( + Attr.getRange(), S.Context, ExtensibilityKind, + Attr.getAttributeSpellingListIndex())); +} + /// Handle __attribute__((format_arg((idx)))) attribute based on /// http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html static void handleFormatArgAttr(Sema &S, Decl *D, const AttributeList &Attr) { @@ -5856,6 +5878,9 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, case AttributeList::AT_FlagEnum: handleSimpleAttribute(S, D, Attr); break; + case AttributeList::AT_EnumExtensibility: + handleEnumExtensibilityAttr(S, D, Attr); + break; case AttributeList::AT_Flatten: handleSimpleAttribute(S, D, Attr); break; diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 3f6c46795514..f393e3e7941c 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -711,6 +711,9 @@ static bool ShouldDiagnoseSwitchCaseNotInEnum(const Sema &S, EnumValsTy::iterator &EI, EnumValsTy::iterator &EIEnd, const llvm::APSInt &Val) { + if (!ED->isClosed()) + return false; + if (const DeclRefExpr *DRE = dyn_cast(CaseExpr->IgnoreParenImpCasts())) { if (const VarDecl *VD = dyn_cast(DRE->getDecl())) { @@ -722,15 +725,14 @@ static bool ShouldDiagnoseSwitchCaseNotInEnum(const Sema &S, } } - if (ED->hasAttr()) { + if (ED->hasAttr()) return !S.IsValueInFlagEnum(ED, Val, false); - } else { - while (EI != EIEnd && EI->first < Val) - EI++; - if (EI != EIEnd && EI->first == Val) - return false; - } + while (EI != EIEnd && EI->first < Val) + EI++; + + if (EI != EIEnd && EI->first == Val) + return false; return true; } @@ -1147,7 +1149,7 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch, } } - if (TheDefaultStmt && UnhandledNames.empty()) + if (TheDefaultStmt && UnhandledNames.empty() && ED->isClosedNonFlag()) Diag(TheDefaultStmt->getDefaultLoc(), diag::warn_unreachable_default); // Produce a nice diagnostic if multiple values aren't handled. @@ -1198,6 +1200,9 @@ Sema::DiagnoseAssignmentEnum(QualType DstType, QualType SrcType, AdjustAPSInt(RhsVal, DstWidth, DstIsSigned); const EnumDecl *ED = ET->getDecl(); + if (!ED->isClosed()) + return; + if (ED->hasAttr()) { if (!IsValueInFlagEnum(ED, RhsVal, true)) Diag(SrcExpr->getExprLoc(), diag::warn_not_in_enum_assignment) diff --git a/clang/test/Sema/enum-attr.c b/clang/test/Sema/enum-attr.c new file mode 100644 index 000000000000..933d8ccdcd89 --- /dev/null +++ b/clang/test/Sema/enum-attr.c @@ -0,0 +1,130 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wassign-enum -Wswitch-enum -Wcovered-switch-default %s + +enum Enum { + A0 = 1, A1 = 10 +}; + +enum __attribute__((enum_extensibility(closed))) EnumClosed { + B0 = 1, B1 = 10 +}; + +enum __attribute__((enum_extensibility(open))) EnumOpen { + C0 = 1, C1 = 10 +}; + +enum __attribute__((flag_enum)) EnumFlag { + D0 = 1, D1 = 8 +}; + +enum __attribute__((flag_enum,enum_extensibility(closed))) EnumFlagClosed { + E0 = 1, E1 = 8 +}; + +enum __attribute__((flag_enum,enum_extensibility(open))) EnumFlagOpen { + F0 = 1, F1 = 8 +}; + +enum __attribute__((enum_extensibility(arg1))) EnumInvalidArg { // expected-warning{{'enum_extensibility' attribute argument not supported: 'arg1'}} + X0 +}; + +// FIXME: The warning should mention that enum_extensibility takes only one argument. +enum __attribute__((enum_extensibility(closed,open))) EnumTooManyArgs { // expected-error{{use of undeclared identifier 'open'}} + X1 +}; + +enum __attribute__((enum_extensibility())) EnumTooFewArgs { // expected-error{{'enum_extensibility' attribute takes one argument}} + X2 +}; + +struct __attribute__((enum_extensibility(open))) S { // expected-warning{{'enum_extensibility' attribute only applies to enums}}{ +}; + +void test() { + enum Enum t0 = 100; // expected-warning{{integer constant not in range of enumerated type}} + t0 = 1; + + switch (t0) { // expected-warning{{enumeration value 'A1' not handled in switch}} + case A0: break; + case 16: break; // expected-warning{{case value not in enumerated type}} + } + + switch (t0) { + case A0: break; + case A1: break; + default: break; // expected-warning{{default label in switch which covers all enumeration}} + } + + enum EnumClosed t1 = 100; // expected-warning{{integer constant not in range of enumerated type}} + t1 = 1; + + switch (t1) { // expected-warning{{enumeration value 'B1' not handled in switch}} + case B0: break; + case 16: break; // expected-warning{{case value not in enumerated type}} + } + + switch (t1) { + case B0: break; + case B1: break; + default: break; // expected-warning{{default label in switch which covers all enumeration}} + } + + enum EnumOpen t2 = 100; + t2 = 1; + + switch (t2) { // expected-warning{{enumeration value 'C1' not handled in switch}} + case C0: break; + case 16: break; + } + + switch (t2) { + case C0: break; + case C1: break; + default: break; + } + + enum EnumFlag t3 = 5; // expected-warning{{integer constant not in range of enumerated type}} + t3 = 9; + + switch (t3) { // expected-warning{{enumeration value 'D1' not handled in switch}} + case D0: break; + case 9: break; + case 16: break; // expected-warning{{case value not in enumerated type}} + } + + switch (t3) { + case D0: break; + case D1: break; + default: break; + } + + enum EnumFlagClosed t4 = 5; // expected-warning{{integer constant not in range of enumerated type}} + t4 = 9; + + switch (t4) { // expected-warning{{enumeration value 'E1' not handled in switch}} + case E0: break; + case 9: break; + case 16: break; // expected-warning{{case value not in enumerated type}} + } + + switch (t4) { + case E0: break; + case E1: break; + default: break; + } + + enum EnumFlagOpen t5 = 5; + t5 = 9; + + switch (t5) { // expected-warning{{enumeration value 'F1' not handled in switch}} + case F0: break; + case 9: break; + case 16: break; + } + + switch (t5) { + case F0: break; + case F1: break; + default: break; + } +} diff --git a/clang/test/SemaCXX/attr-flag-enum-reject.cpp b/clang/test/SemaCXX/attr-flag-enum-reject.cpp deleted file mode 100644 index f28d60c0c295..000000000000 --- a/clang/test/SemaCXX/attr-flag-enum-reject.cpp +++ /dev/null @@ -1,4 +0,0 @@ -// RUN: %clang_cc1 -verify -fsyntax-only -x c++ -Wassign-enum %s - -enum __attribute__((flag_enum)) flag { // expected-warning {{ignored}} -}; diff --git a/clang/test/SemaCXX/enum-attr.cpp b/clang/test/SemaCXX/enum-attr.cpp new file mode 100644 index 000000000000..7726aff4830a --- /dev/null +++ b/clang/test/SemaCXX/enum-attr.cpp @@ -0,0 +1,108 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wassign-enum -Wswitch-enum -Wcovered-switch-default -std=c++11 %s + +enum Enum { + A0 = 1, A1 = 10 +}; + +enum __attribute__((enum_extensibility(closed))) EnumClosed { + B0 = 1, B1 = 10 +}; + +enum [[clang::enum_extensibility(open)]] EnumOpen { + C0 = 1, C1 = 10 +}; + +enum __attribute__((flag_enum)) EnumFlag { + D0 = 1, D1 = 8 +}; + +enum __attribute__((flag_enum,enum_extensibility(closed))) EnumFlagClosed { + E0 = 1, E1 = 8 +}; + +enum __attribute__((flag_enum,enum_extensibility(open))) EnumFlagOpen { + F0 = 1, F1 = 8 +}; + +void test() { + enum Enum t0; + + switch (t0) { // expected-warning{{enumeration value 'A1' not handled in switch}} + case A0: break; + case 16: break; // expected-warning{{case value not in enumerated type}} + } + + switch (t0) { + case A0: break; + case A1: break; + default: break; // expected-warning{{default label in switch which covers all enumeration}} + } + + enum EnumClosed t1; + + switch (t1) { // expected-warning{{enumeration value 'B1' not handled in switch}} + case B0: break; + case 16: break; // expected-warning{{case value not in enumerated type}} + } + + switch (t1) { + case B0: break; + case B1: break; + default: break; // expected-warning{{default label in switch which covers all enumeration}} + } + + enum EnumOpen t2; + + switch (t2) { // expected-warning{{enumeration value 'C1' not handled in switch}} + case C0: break; + case 16: break; + } + + switch (t2) { + case C0: break; + case C1: break; + default: break; + } + + enum EnumFlag t3; + + switch (t3) { // expected-warning{{enumeration value 'D1' not handled in switch}} + case D0: break; + case 9: break; + case 16: break; // expected-warning{{case value not in enumerated type}} + } + + switch (t3) { + case D0: break; + case D1: break; + default: break; + } + + enum EnumFlagClosed t4; + + switch (t4) { // expected-warning{{enumeration value 'E1' not handled in switch}} + case E0: break; + case 9: break; + case 16: break; // expected-warning{{case value not in enumerated type}} + } + + switch (t4) { + case E0: break; + case E1: break; + default: break; + } + + enum EnumFlagOpen t5; + + switch (t5) { // expected-warning{{enumeration value 'F1' not handled in switch}} + case F0: break; + case 9: break; + case 16: break; + } + + switch (t5) { + case F0: break; + case F1: break; + default: break; + } +}