]> granicus.if.org Git - clang/commitdiff
Add support for attribute enum_extensibility.
authorAkira Hatanaka <ahatanaka@apple.com>
Tue, 21 Mar 2017 02:23:00 +0000 (02:23 +0000)
committerAkira Hatanaka <ahatanaka@apple.com>
Tue, 21 Mar 2017 02:23:00 +0000 (02:23 +0000)
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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@298332 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/AST/Decl.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
lib/AST/Decl.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaStmt.cpp
test/Sema/enum-attr.c [new file with mode: 0644]
test/SemaCXX/attr-flag-enum-reject.cpp [deleted file]
test/SemaCXX/enum-attr.cpp [new file with mode: 0644]

index d1c77f5e080c5e1e58c53d959f78d8f71d56cd44..623d9c4792e7ae5a4782d06d51d9cc012f95d655 100644 (file)
@@ -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;
index 5960cca1e795bd8814f3f579e2535be221f1dc0f..ff62e10f8adb29b1d533963d63af545741053e19 100644 (file)
@@ -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 {
index df299e07c673218d9be6440a2bdb1e7ed97c1a92..23b9bb7188e7864ad0ca69a363b224dcc617c692 100644 (file)
@@ -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 = [{
index cc6f2fa7054156c682042f966fbd92217089aace..5b0404e38dde00774a76dd43cf7aa1b51580baba 100644 (file)
@@ -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();
index d7d71221b5de25f05b5f9742227ccfb3a4cfa4e4..bc5f60504df26ad2c828b6f8c7232d00050460a0 100644 (file)
@@ -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()));
@@ -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.
index 3979383966c80171b70aab91fea750f65722418b..1c350a7e38b7d722416e7397bfbcbd7528f264ae 100644 (file)
@@ -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<FlagEnumAttr>(S, D, Attr);
     break;
+  case AttributeList::AT_EnumExtensibility:
+    handleEnumExtensibilityAttr(S, D, Attr);
+    break;
   case AttributeList::AT_Flatten:
     handleSimpleAttribute<FlattenAttr>(S, D, Attr);
     break;
index 3f6c4679551420627dcfac9f1fde5bf945e45b12..f393e3e7941ca19a0b3da9245dbc7c288dc19166 100644 (file)
@@ -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())) {
@@ -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;
 }
@@ -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<FlagEnumAttr>()) {
           if (!IsValueInFlagEnum(ED, RhsVal, true))
             Diag(SrcExpr->getExprLoc(), diag::warn_not_in_enum_assignment)
diff --git a/test/Sema/enum-attr.c b/test/Sema/enum-attr.c
new file mode 100644 (file)
index 0000000..933d8cc
--- /dev/null
@@ -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/test/SemaCXX/attr-flag-enum-reject.cpp b/test/SemaCXX/attr-flag-enum-reject.cpp
deleted file mode 100644 (file)
index f28d60c..0000000
+++ /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/test/SemaCXX/enum-attr.cpp b/test/SemaCXX/enum-attr.cpp
new file mode 100644 (file)
index 0000000..7726aff
--- /dev/null
@@ -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;
+  }
+}