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

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index d1c77f5..623d9c4 100644 (file)
--- 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 5960cca..ff62e10 100644 (file)
--- 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 df299e0..23b9bb7 100644 (file)
--- 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 cc6f2fa..5b0404e 100644 (file)
--- 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<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();

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index d7d7122..bc5f605 100644 (file)
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -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.

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 3979383..1c350a7 100644 (file)
--- 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<FlagEnumAttr>(S, D, Attr);
     break;
+  case AttributeList::AT_EnumExtensibility:
+    handleEnumExtensibilityAttr(S, D, Attr);
+    break;
   case AttributeList::AT_Flatten:
     handleSimpleAttribute<FlattenAttr>(S, D, Attr);
     break;

diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 3f6c467..f393e3e 100644 (file)
--- 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<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/clang/test/Sema/enum-attr.c b/clang/test/Sema/enum-attr.c
new file mode 100644 (file)
index 0000000..933d8cc
--- /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 (file)
index f28d60c..0000000
--- 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 (file)
index 0000000..7726aff
--- /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;
+  }
+}