Skip to content

Commit

Permalink
Add support for attribute enum_extensibility.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ahatanaka committed Mar 21, 2017
1 parent 9a4bce7 commit 3c268af
Show file tree
Hide file tree
Showing 10 changed files with 362 additions and 15 deletions.
12 changes: 12 additions & 0 deletions clang/include/clang/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -3235,6 +3235,18 @@ class EnumDecl : public TagDecl {
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;
Expand Down
10 changes: 9 additions & 1 deletion clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
49 changes: 49 additions & 0 deletions clang/include/clang/Basic/AttrDocs.td
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [{
Expand Down
14 changes: 14 additions & 0 deletions clang/lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3742,6 +3742,20 @@ void EnumDecl::completeDefinition(QualType NewType,
TagDecl::completeDefinition();
}

bool EnumDecl::isClosed() const {
if (const auto *A = getAttr<EnumExtensibilityAttr>())
return A->getExtensibility() == EnumExtensibilityAttr::Closed;
return true;
}

bool EnumDecl::isClosedFlag() const {
return isClosed() && hasAttr<FlagEnumAttr>();
}

bool EnumDecl::isClosedNonFlag() const {
return isClosed() && !hasAttr<FlagEnumAttr>();
}

TemplateSpecializationKind EnumDecl::getTemplateSpecializationKind() const {
if (MemberSpecializationInfo *MSI = getMemberSpecializationInfo())
return MSI->getTemplateSpecializationKind();
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15358,7 +15358,7 @@ static void CheckForDuplicateEnumValues(Sema &S, ArrayRef<Decl *> Elements,

bool Sema::IsValueInFlagEnum(const EnumDecl *ED, const llvm::APInt &Val,
bool AllowMask) const {
assert(ED->hasAttr<FlagEnumAttr>() && "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()));
Expand Down Expand Up @@ -15603,7 +15603,7 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,

CheckForDuplicateEnumValues(*this, Elements, Enum, EnumType);

if (Enum->hasAttr<FlagEnumAttr>()) {
if (Enum->isClosedFlag()) {
for (Decl *D : Elements) {
EnumConstantDecl *ECD = cast_or_null<EnumConstantDecl>(D);
if (!ECD) continue; // Already issued a diagnostic.
Expand Down
25 changes: 25 additions & 0 deletions clang/lib/Sema/SemaDeclAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -5856,6 +5878,9 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
case AttributeList::AT_FlagEnum:
handleSimpleAttribute<FlagEnumAttr>(S, D, Attr);
break;
case AttributeList::AT_EnumExtensibility:
handleEnumExtensibilityAttr(S, D, Attr);
break;
case AttributeList::AT_Flatten:
handleSimpleAttribute<FlattenAttr>(S, D, Attr);
break;
Expand Down
21 changes: 13 additions & 8 deletions clang/lib/Sema/SemaStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<DeclRefExpr>(CaseExpr->IgnoreParenImpCasts())) {
if (const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
Expand All @@ -722,15 +725,14 @@ static bool ShouldDiagnoseSwitchCaseNotInEnum(const Sema &S,
}
}

if (ED->hasAttr<FlagEnumAttr>()) {
if (ED->hasAttr<FlagEnumAttr>())
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;
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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<FlagEnumAttr>()) {
if (!IsValueInFlagEnum(ED, RhsVal, true))
Diag(SrcExpr->getExprLoc(), diag::warn_not_in_enum_assignment)
Expand Down
130 changes: 130 additions & 0 deletions clang/test/Sema/enum-attr.c
Original file line number Diff line number Diff line change
@@ -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;
}
}
4 changes: 0 additions & 4 deletions clang/test/SemaCXX/attr-flag-enum-reject.cpp

This file was deleted.

Loading

0 comments on commit 3c268af

Please sign in to comment.