Add -Wduplicate-enum warning. Clang will emit this warning when an implicitly

initiated enum constant has the same value as another enum constant.

For instance:
enum test { A, B, C = -1, D, E = 1 };
Clang will warn that:
 A and D both have value 0
 B and E both have value 1

A few exceptions are made to keep the noise down.  Enum constants which are
initialized to another enum constant, or an enum constant plus or minus 1 will
not trigger this warning.  Also, anonymous enums are not checked.



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

3 files changed, 273 insertions, 0 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 3779da7b1b..b8d1c6901a 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -26,6 +26,10 @@ def warn_identical_enum_values : Warning<
 def note_identical_enum_values : Note<
   "initialize the last element with the previous element to silence "
   "this warning">;
+def warn_duplicate_enum_values : Warning<
+  "element %0 has been implicitly assigned %1 which another element has "
+  "been assigned">, InGroup<DiagGroup<"duplicate-enum">>, DefaultIgnore;
+def note_duplicate_element : Note<"element %0 also has value %1">;
 
 // Constant expressions
 def err_expr_not_ice : Error<
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 2fe4e63dbb..082fb13fb5 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -10559,6 +10559,182 @@ static void CheckForUniqueEnumValues(Sema &S, Decl **Elements,
                                     Next->getName());
 }
 
+// Returns true when the enum initial expression does not trigger the
+// duplicate enum warning.  A few common cases are exempted as follows:
+// Element2 = Element1
+// Element2 = Element1 + 1
+// Element2 = Element1 - 1
+// Where Element2 and Element1 are from the same enum.
+static bool ValidDuplicateEnum(EnumConstantDecl *ECD, EnumDecl *Enum) {
+  Expr *InitExpr = ECD->getInitExpr();
+  if (!InitExpr)
+    return true;
+  InitExpr = InitExpr->IgnoreImpCasts();
+
+  if (BinaryOperator *BO = dyn_cast<BinaryOperator>(InitExpr)) {
+    if (!BO->isAdditiveOp())
+      return true;
+    IntegerLiteral *IL = dyn_cast<IntegerLiteral>(BO->getRHS());
+    if (!IL)
+      return true;
+    if (IL->getValue() != 1)
+      return true;
+
+    InitExpr = BO->getLHS();
+  }
+
+  // This checks if the elements are from the same enum.
+  DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(InitExpr);
+  if (!DRE)
+    return true;
+
+  EnumConstantDecl *EnumConstant = dyn_cast<EnumConstantDecl>(DRE->getDecl());
+  if (!EnumConstant)
+    return true;
+
+  if (cast<EnumDecl>(TagDecl::castFromDeclContext(ECD->getDeclContext())) !=
+      Enum)
+    return true;
+
+  return false;
+}
+
+struct DupKey {
+  int64_t val;
+  bool isTombstoneOrEmptyKey;
+  DupKey(int64_t val, bool isTombstoneOrEmptyKey)
+    : val(val), isTombstoneOrEmptyKey(isTombstoneOrEmptyKey) {}
+};
+
+static DupKey GetDupKey(const llvm::APSInt& Val) {
+  return DupKey(Val.isSigned() ? Val.getSExtValue() : Val.getZExtValue(),
+                false);
+}
+
+struct DenseMapInfoDupKey {
+  static DupKey getEmptyKey() { return DupKey(0, true); }
+  static DupKey getTombstoneKey() { return DupKey(1, true); }
+  static unsigned getHashValue(const DupKey Key) {
+    return (unsigned)(Key.val * 37);
+  }
+  static bool isEqual(const DupKey& LHS, const DupKey& RHS) {
+    return LHS.isTombstoneOrEmptyKey == RHS.isTombstoneOrEmptyKey &&
+           LHS.val == RHS.val;
+  }
+};
+
+// Emits a warning when an element is implicitly set a value that
+// a previous element has already been set to.
+static void CheckForDuplicateEnumValues(Sema &S, Decl **Elements,
+                                        unsigned NumElements, EnumDecl *Enum,
+                                        QualType EnumType) {
+  if (S.Diags.getDiagnosticLevel(diag::warn_duplicate_enum_values,
+                                 Enum->getLocation()) ==
+      DiagnosticsEngine::Ignored)
+    return;
+  // Avoid anonymous enums
+  if (!Enum->getIdentifier())
+    return;
+
+  // Only check for small enums.
+  if (Enum->getNumPositiveBits() > 63 || Enum->getNumNegativeBits() > 64)
+    return;
+
+  typedef llvm::SmallVector<EnumConstantDecl*, 3> ECDVector;
+  typedef llvm::SmallVector<ECDVector*, 3> DuplicatesVector;
+
+  typedef llvm::PointerUnion<EnumConstantDecl*, ECDVector*> DeclOrVector;
+  typedef llvm::DenseMap<DupKey, DeclOrVector, DenseMapInfoDupKey>
+          ValueToVectorMap;
+
+  DuplicatesVector DupVector;
+  ValueToVectorMap EnumMap;
+
+  // Populate the EnumMap with all values represented by enum constants without
+  // an initialier.
+  for (unsigned i = 0; i < NumElements; ++i) {
+    EnumConstantDecl *ECD = cast<EnumConstantDecl>(Elements[i]);
+
+    // Null EnumConstantDecl means a previous diagnostic has been emitted for
+    // this constant.  Skip this enum since it may be ill-formed.
+    if (!ECD) {
+      return;
+    }
+
+    if (ECD->getInitExpr())
+      continue;
+
+    DupKey Key = GetDupKey(ECD->getInitVal());
+    DeclOrVector &Entry = EnumMap[Key];
+
+    // First time encountering this value.
+    if (Entry.isNull())
+      Entry = ECD;
+  }
+
+  // Create vectors for any values that has duplicates.
+  for (unsigned i = 0; i < NumElements; ++i) {
+    EnumConstantDecl *ECD = cast<EnumConstantDecl>(Elements[i]);
+    if (!ValidDuplicateEnum(ECD, Enum))
+      continue;
+
+    DupKey Key = GetDupKey(ECD->getInitVal());
+
+    DeclOrVector& Entry = EnumMap[Key];
+    if (Entry.isNull())
+      continue;
+
+    if (EnumConstantDecl *D = Entry.dyn_cast<EnumConstantDecl*>()) {
+      // Ensure constants are different.
+      if (D == ECD)
+        continue;
+
+      // Create new vector and push values onto it.
+      ECDVector *Vec = new ECDVector();
+      Vec->push_back(D);
+      Vec->push_back(ECD);
+
+      // Update entry to point to the duplicates vector.
+      Entry = Vec;
+
+      // Store the vector somewhere we can consult later for quick emission of
+      // diagnostics.
+      DupVector.push_back(Vec);
+      continue;
+    }
+
+    ECDVector *Vec = Entry.get<ECDVector*>();
+    // Make sure constants are not added more than once.
+    if (*Vec->begin() == ECD)
+      continue;
+
+    Vec->push_back(ECD);
+  }
+
+  // Emit diagnostics.
+  for (DuplicatesVector::iterator DupVectorIter = DupVector.begin(),
+                                  DupVectorEnd = DupVector.end();
+       DupVectorIter != DupVectorEnd; ++DupVectorIter) {
+    ECDVector *Vec = *DupVectorIter;
+    assert(Vec->size() > 1 && "ECDVector should have at least 2 elements.");
+
+    // Emit warning for one enum constant.
+    ECDVector::iterator I = Vec->begin();
+    S.Diag((*I)->getLocation(), diag::warn_duplicate_enum_values)
+      << (*I)->getName() << (*I)->getInitVal().toString(10)
+      << (*I)->getSourceRange();
+    ++I;
+
+    // Emit one note for each of the remaining enum constants with
+    // the same value.
+    for (ECDVector::iterator E = Vec->end(); I != E; ++I)
+      S.Diag((*I)->getLocation(), diag::note_duplicate_element)
+        << (*I)->getName() << (*I)->getInitVal().toString(10)
+        << (*I)->getSourceRange();
+    delete Vec;
+  }
+}
+
 void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceLocation LBraceLoc,
                          SourceLocation RBraceLoc, Decl *EnumDeclX,
                          Decl **Elements, unsigned NumElements,
@@ -10783,6 +10959,7 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceLocation LBraceLoc,
     DeclsInPrototypeScope.push_back(Enum);
 
   CheckForUniqueEnumValues(*this, Elements, NumElements, Enum, EnumType);
+  CheckForDuplicateEnumValues(*this, Elements, NumElements, Enum, EnumType);
 }
 
 Decl *Sema::ActOnFileScopeAsmDecl(Expr *expr,
diff --git a/test/Sema/warn-duplicate-enum.c b/test/Sema/warn-duplicate-enum.c
new file mode 100644
index 0000000000..239f6f1995
--- /dev/null
+++ b/test/Sema/warn-duplicate-enum.c
@@ -0,0 +1,92 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -Wduplicate-enum
+// RUN: %clang_cc1 %s -x c++ -fsyntax-only -verify -Wduplicate-enum
+enum A {
+  A1 = 0,  // expected-note {{element A1 also has value 0}}
+  A2 = -1,
+  A3,  // expected-warning {{element A3 has been implicitly assigned 0 which another element has been assigned}}
+  A4};
+
+enum B {
+  B1 = -1,  // expected-note {{element B1 also has value -1}}
+  B2,       // expected-warning {{element B2 has been implicitly assigned 0 which another element has been assigned}}
+  B3,
+  B4 = -2,
+  B5,  // expected-warning {{element B5 has been implicitly assigned -1 which another element has been assigned}}
+  B6   // expected-note {{element B6 also has value 0}}
+};
+
+enum C { C1, C2 = -1, C3 }; // expected-warning{{element C1 has been implicitly assigned 0 which another element has been assigned}} \
+  // expected-note {{element C3 also has value 0}}
+
+enum D {
+  D1,
+  D2,
+  D3,  // expected-warning{{element D3 has been implicitly assigned 2 which another element has been assigned}}
+  D4 = D2,  // no warning
+  D5 = 2  // expected-note {{element D5 also has value 2}}
+};
+
+enum E {
+  E1,
+  E2 = E1,
+  E3 = E2
+};
+
+enum F {
+  F1,
+  F2,
+  FCount,
+  FMax = FCount - 1
+};
+
+enum G {
+  G1,
+  G2,
+  GMax = G2,
+  GCount = GMax + 1
+};
+
+enum {
+  H1 = 0,
+  H2 = -1,
+  H3,
+  H4};
+
+enum {
+  I1 = -1,
+  I2,
+  I3,
+  I4 = -2,
+  I5,
+  I6
+};
+
+enum { J1, J2 = -1, J3 };
+
+enum { 
+  K1, 
+  K2, 
+  K3,
+  K4 = K2,
+  K5 = 2
+};
+
+enum {
+  L1,
+  L2 = L1,
+  L3 = L2
+};
+
+enum {
+  M1,
+  M2,
+  MCount,
+  MMax = MCount - 1
+};
+
+enum {
+  N1,
+  N2,
+  NMax = N2,
+  NCount = NMax + 1
+};