[cfe-commits] r170974 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp test/Sema/warn-duplicate-enum.c
Ted Kremenek
kremenek at apple.com
Fri Dec 21 17:34:09 PST 2012
Author: kremenek
Date: Fri Dec 21 19:34:09 2012
New Revision: 170974
URL: http://llvm.org/viewvc/llvm-project?rev=170974&view=rev
Log:
Add back -Wduplicate-enum which I mistakenly removed.
This was removed with -Wunique-enum, which is still removed. The
corresponding thread on cfe-comments for that warning is here:
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-September/024224.html
If we get specific user feedback for -Wduplicate-enum we can evaluate
whether or not to keep it.
Added:
cfe/trunk/test/Sema/warn-duplicate-enum.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDecl.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=170974&r1=170973&r2=170974&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Dec 21 19:34:09 2012
@@ -20,6 +20,11 @@
"used in loop condition not modified in loop body">,
InGroup<DiagGroup<"loop-analysis">>, DefaultIgnore;
+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<
"expression is not an %select{integer|integral}0 constant expression">;
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=170974&r1=170973&r2=170974&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Dec 21 19:34:09 2012
@@ -10640,6 +10640,182 @@
return New;
}
+// 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,
@@ -10862,6 +11038,8 @@
// it needs to go into the function scope.
if (InFunctionDeclarator)
DeclsInPrototypeScope.push_back(Enum);
+
+ CheckForDuplicateEnumValues(*this, Elements, NumElements, Enum, EnumType);
}
Decl *Sema::ActOnFileScopeAsmDecl(Expr *expr,
Added: cfe/trunk/test/Sema/warn-duplicate-enum.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-duplicate-enum.c?rev=170974&view=auto
==============================================================================
--- cfe/trunk/test/Sema/warn-duplicate-enum.c (added)
+++ cfe/trunk/test/Sema/warn-duplicate-enum.c Fri Dec 21 19:34:09 2012
@@ -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
+};
More information about the cfe-commits
mailing list