r298332 - Add support for attribute enum_extensibility.

Akira Hatanaka via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 20 19:23:01 PDT 2017


Author: ahatanak
Date: Mon Mar 20 21:23:00 2017
New Revision: 298332

URL: http://llvm.org/viewvc/llvm-project?rev=298332&view=rev
Log:
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

Added:
    cfe/trunk/test/Sema/enum-attr.c
    cfe/trunk/test/SemaCXX/enum-attr.cpp
Removed:
    cfe/trunk/test/SemaCXX/attr-flag-enum-reject.cpp
Modified:
    cfe/trunk/include/clang/AST/Decl.h
    cfe/trunk/include/clang/Basic/Attr.td
    cfe/trunk/include/clang/Basic/AttrDocs.td
    cfe/trunk/lib/AST/Decl.cpp
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/lib/Sema/SemaDeclAttr.cpp
    cfe/trunk/lib/Sema/SemaStmt.cpp

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=298332&r1=298331&r2=298332&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Mon Mar 20 21:23:00 2017
@@ -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;

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=298332&r1=298331&r2=298332&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Mon Mar 20 21:23:00 2017
@@ -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 {

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=298332&r1=298331&r2=298332&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon Mar 20 21:23:00 2017
@@ -1963,6 +1963,55 @@ manipulating bits of the enumerator when
   }];
 }
 
+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 = [{

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=298332&r1=298331&r2=298332&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Mon Mar 20 21:23:00 2017
@@ -3742,6 +3742,20 @@ void EnumDecl::completeDefinition(QualTy
   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();

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=298332&r1=298331&r2=298332&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Mar 20 21:23:00 2017
@@ -15358,7 +15358,7 @@ static void CheckForDuplicateEnumValues(
 
 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()));
@@ -15603,7 +15603,7 @@ void Sema::ActOnEnumBody(SourceLocation
 
   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.

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=298332&r1=298331&r2=298332&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon Mar 20 21:23:00 2017
@@ -2943,6 +2943,28 @@ static void handleCleanupAttr(Sema &S, D
                          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
   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;

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=298332&r1=298331&r2=298332&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Mon Mar 20 21:23:00 2017
@@ -711,6 +711,9 @@ static bool ShouldDiagnoseSwitchCaseNotI
                                               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())) {
@@ -722,15 +725,14 @@ static bool ShouldDiagnoseSwitchCaseNotI
     }
   }
 
-  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;
 }
@@ -1147,7 +1149,7 @@ Sema::ActOnFinishSwitchStmt(SourceLocati
         }
       }
 
-      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 Ds
         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)

Added: cfe/trunk/test/Sema/enum-attr.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/enum-attr.c?rev=298332&view=auto
==============================================================================
--- cfe/trunk/test/Sema/enum-attr.c (added)
+++ cfe/trunk/test/Sema/enum-attr.c Mon Mar 20 21:23:00 2017
@@ -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;
+  }
+}

Removed: cfe/trunk/test/SemaCXX/attr-flag-enum-reject.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-flag-enum-reject.cpp?rev=298331&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/attr-flag-enum-reject.cpp (original)
+++ cfe/trunk/test/SemaCXX/attr-flag-enum-reject.cpp (removed)
@@ -1,4 +0,0 @@
-// RUN: %clang_cc1 -verify -fsyntax-only -x c++ -Wassign-enum %s
-
-enum __attribute__((flag_enum)) flag { // expected-warning {{ignored}}
-};

Added: cfe/trunk/test/SemaCXX/enum-attr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enum-attr.cpp?rev=298332&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/enum-attr.cpp (added)
+++ cfe/trunk/test/SemaCXX/enum-attr.cpp Mon Mar 20 21:23:00 2017
@@ -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;
+  }
+}




More information about the cfe-commits mailing list