<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt">On Wed, Oct 3, 2012 at 2:27 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>></span> wrote:<br>
<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Mon, Sep 17, 2012 at 5:41 PM, Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com" target="_blank">kremenek@apple.com</a>></span> wrote:<br>
</div><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: kremenek<br>
Date: Mon Sep 17 19:41:42 2012<br>
New Revision: 164083<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=164083&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=164083&view=rev</a><br>
Log:<br>
Per discussion on cfe-dev, remove -Wunique-enums entirely. There<br>
is no compelling argument that this is a generally useful warning,<br>
and imposes a strong stylistic argument on code beyond what it was<br>
intended to find warnings in.<br>
<br>
Removed:<br>
cfe/trunk/test/Sema/warn-duplicate-enum.c<br>
cfe/trunk/test/SemaCXX/warn-unique-enum.cpp<br>
Modified:<br>
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
cfe/trunk/lib/Sema/SemaDecl.cpp<br>
<br></blockquote></div><div><div>Hey Ted,</div><div><br></div><div>Why was -Wduplicate-enum also removed in this commit?</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>Richard.</div></font></span></div>
<div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=164083&r1=164082&r2=164083&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=164083&r1=164082&r2=164083&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Sep 17 19:41:42 2012<br>
@@ -20,17 +20,6 @@<br>
"used in loop condition not modified in loop body">,<br>
InGroup<DiagGroup<"loop-analysis">>, DefaultIgnore;<br>
<br>
-def warn_identical_enum_values : Warning<<br>
- "all elements of %0 are initialized with literals to value %1">,<br>
- InGroup<DiagGroup<"unique-enum">>;<br>
-def note_identical_enum_values : Note<<br>
- "initialize the last element with the previous element to silence "<br>
- "this warning">;<br>
-def warn_duplicate_enum_values : Warning<<br>
- "element %0 has been implicitly assigned %1 which another element has "<br>
- "been assigned">, InGroup<DiagGroup<"duplicate-enum">>, DefaultIgnore;<br>
-def note_duplicate_element : Note<"element %0 also has value %1">;<br>
-<br>
// Constant expressions<br>
def err_expr_not_ice : Error<<br>
"expression is not an %select{integer|integral}0 constant expression">;<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=164083&r1=164082&r2=164083&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=164083&r1=164082&r2=164083&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Sep 17 19:41:42 2012<br>
@@ -10521,233 +10521,6 @@<br>
return New;<br>
}<br>
<br>
-// Emits a warning if every element in the enum is the same value and if<br>
-// every element is initialized with a integer or boolean literal.<br>
-static void CheckForUniqueEnumValues(Sema &S, Decl **Elements,<br>
- unsigned NumElements, EnumDecl *Enum,<br>
- QualType EnumType) {<br>
- if (S.Diags.getDiagnosticLevel(diag::warn_identical_enum_values,<br>
- Enum->getLocation()) ==<br>
- DiagnosticsEngine::Ignored)<br>
- return;<br>
-<br>
- if (NumElements < 2)<br>
- return;<br>
-<br>
- if (!Enum->getIdentifier())<br>
- return;<br>
-<br>
- llvm::APSInt FirstVal;<br>
-<br>
- for (unsigned i = 0; i != NumElements; ++i) {<br>
- EnumConstantDecl *ECD = cast_or_null<EnumConstantDecl>(Elements[i]);<br>
- if (!ECD)<br>
- return;<br>
-<br>
- Expr *InitExpr = ECD->getInitExpr();<br>
- if (!InitExpr)<br>
- return;<br>
- InitExpr = InitExpr->IgnoreImpCasts();<br>
- if (!isa<IntegerLiteral>(InitExpr) && !isa<CXXBoolLiteralExpr>(InitExpr))<br>
- return;<br>
-<br>
- if (i == 0) {<br>
- FirstVal = ECD->getInitVal();<br>
- continue;<br>
- }<br>
-<br>
- if (!llvm::APSInt::isSameValue(FirstVal, ECD->getInitVal()))<br>
- return;<br>
- }<br>
-<br>
- S.Diag(Enum->getLocation(), diag::warn_identical_enum_values)<br>
- << EnumType << FirstVal.toString(10)<br>
- << Enum->getSourceRange();<br>
-<br>
- EnumConstantDecl *Last = cast<EnumConstantDecl>(Elements[NumElements - 1]),<br>
- *Next = cast<EnumConstantDecl>(Elements[NumElements - 2]);<br>
-<br>
- S.Diag(Last->getLocation(), diag::note_identical_enum_values)<br>
- << FixItHint::CreateReplacement(Last->getInitExpr()->getSourceRange(),<br>
- Next->getName());<br>
-}<br>
-<br>
-// Returns true when the enum initial expression does not trigger the<br>
-// duplicate enum warning. A few common cases are exempted as follows:<br>
-// Element2 = Element1<br>
-// Element2 = Element1 + 1<br>
-// Element2 = Element1 - 1<br>
-// Where Element2 and Element1 are from the same enum.<br>
-static bool ValidDuplicateEnum(EnumConstantDecl *ECD, EnumDecl *Enum) {<br>
- Expr *InitExpr = ECD->getInitExpr();<br>
- if (!InitExpr)<br>
- return true;<br>
- InitExpr = InitExpr->IgnoreImpCasts();<br>
-<br>
- if (BinaryOperator *BO = dyn_cast<BinaryOperator>(InitExpr)) {<br>
- if (!BO->isAdditiveOp())<br>
- return true;<br>
- IntegerLiteral *IL = dyn_cast<IntegerLiteral>(BO->getRHS());<br>
- if (!IL)<br>
- return true;<br>
- if (IL->getValue() != 1)<br>
- return true;<br>
-<br>
- InitExpr = BO->getLHS();<br>
- }<br>
-<br>
- // This checks if the elements are from the same enum.<br>
- DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(InitExpr);<br>
- if (!DRE)<br>
- return true;<br>
-<br>
- EnumConstantDecl *EnumConstant = dyn_cast<EnumConstantDecl>(DRE->getDecl());<br>
- if (!EnumConstant)<br>
- return true;<br>
-<br>
- if (cast<EnumDecl>(TagDecl::castFromDeclContext(ECD->getDeclContext())) !=<br>
- Enum)<br>
- return true;<br>
-<br>
- return false;<br>
-}<br>
-<br>
-struct DupKey {<br>
- int64_t val;<br>
- bool isTombstoneOrEmptyKey;<br>
- DupKey(int64_t val, bool isTombstoneOrEmptyKey)<br>
- : val(val), isTombstoneOrEmptyKey(isTombstoneOrEmptyKey) {}<br>
-};<br>
-<br>
-static DupKey GetDupKey(const llvm::APSInt& Val) {<br>
- return DupKey(Val.isSigned() ? Val.getSExtValue() : Val.getZExtValue(),<br>
- false);<br>
-}<br>
-<br>
-struct DenseMapInfoDupKey {<br>
- static DupKey getEmptyKey() { return DupKey(0, true); }<br>
- static DupKey getTombstoneKey() { return DupKey(1, true); }<br>
- static unsigned getHashValue(const DupKey Key) {<br>
- return (unsigned)(Key.val * 37);<br>
- }<br>
- static bool isEqual(const DupKey& LHS, const DupKey& RHS) {<br>
- return LHS.isTombstoneOrEmptyKey == RHS.isTombstoneOrEmptyKey &&<br>
- LHS.val == RHS.val;<br>
- }<br>
-};<br>
-<br>
-// Emits a warning when an element is implicitly set a value that<br>
-// a previous element has already been set to.<br>
-static void CheckForDuplicateEnumValues(Sema &S, Decl **Elements,<br>
- unsigned NumElements, EnumDecl *Enum,<br>
- QualType EnumType) {<br>
- if (S.Diags.getDiagnosticLevel(diag::warn_duplicate_enum_values,<br>
- Enum->getLocation()) ==<br>
- DiagnosticsEngine::Ignored)<br>
- return;<br>
- // Avoid anonymous enums<br>
- if (!Enum->getIdentifier())<br>
- return;<br>
-<br>
- // Only check for small enums.<br>
- if (Enum->getNumPositiveBits() > 63 || Enum->getNumNegativeBits() > 64)<br>
- return;<br>
-<br>
- typedef llvm::SmallVector<EnumConstantDecl*, 3> ECDVector;<br>
- typedef llvm::SmallVector<ECDVector*, 3> DuplicatesVector;<br>
-<br>
- typedef llvm::PointerUnion<EnumConstantDecl*, ECDVector*> DeclOrVector;<br>
- typedef llvm::DenseMap<DupKey, DeclOrVector, DenseMapInfoDupKey><br>
- ValueToVectorMap;<br>
-<br>
- DuplicatesVector DupVector;<br>
- ValueToVectorMap EnumMap;<br>
-<br>
- // Populate the EnumMap with all values represented by enum constants without<br>
- // an initialier.<br>
- for (unsigned i = 0; i < NumElements; ++i) {<br>
- EnumConstantDecl *ECD = cast<EnumConstantDecl>(Elements[i]);<br>
-<br>
- // Null EnumConstantDecl means a previous diagnostic has been emitted for<br>
- // this constant. Skip this enum since it may be ill-formed.<br>
- if (!ECD) {<br>
- return;<br>
- }<br>
-<br>
- if (ECD->getInitExpr())<br>
- continue;<br>
-<br>
- DupKey Key = GetDupKey(ECD->getInitVal());<br>
- DeclOrVector &Entry = EnumMap[Key];<br>
-<br>
- // First time encountering this value.<br>
- if (Entry.isNull())<br>
- Entry = ECD;<br>
- }<br>
-<br>
- // Create vectors for any values that has duplicates.<br>
- for (unsigned i = 0; i < NumElements; ++i) {<br>
- EnumConstantDecl *ECD = cast<EnumConstantDecl>(Elements[i]);<br>
- if (!ValidDuplicateEnum(ECD, Enum))<br>
- continue;<br>
-<br>
- DupKey Key = GetDupKey(ECD->getInitVal());<br>
-<br>
- DeclOrVector& Entry = EnumMap[Key];<br>
- if (Entry.isNull())<br>
- continue;<br>
-<br>
- if (EnumConstantDecl *D = Entry.dyn_cast<EnumConstantDecl*>()) {<br>
- // Ensure constants are different.<br>
- if (D == ECD)<br>
- continue;<br>
-<br>
- // Create new vector and push values onto it.<br>
- ECDVector *Vec = new ECDVector();<br>
- Vec->push_back(D);<br>
- Vec->push_back(ECD);<br>
-<br>
- // Update entry to point to the duplicates vector.<br>
- Entry = Vec;<br>
-<br>
- // Store the vector somewhere we can consult later for quick emission of<br>
- // diagnostics.<br>
- DupVector.push_back(Vec);<br>
- continue;<br>
- }<br>
-<br>
- ECDVector *Vec = Entry.get<ECDVector*>();<br>
- // Make sure constants are not added more than once.<br>
- if (*Vec->begin() == ECD)<br>
- continue;<br>
-<br>
- Vec->push_back(ECD);<br>
- }<br>
-<br>
- // Emit diagnostics.<br>
- for (DuplicatesVector::iterator DupVectorIter = DupVector.begin(),<br>
- DupVectorEnd = DupVector.end();<br>
- DupVectorIter != DupVectorEnd; ++DupVectorIter) {<br>
- ECDVector *Vec = *DupVectorIter;<br>
- assert(Vec->size() > 1 && "ECDVector should have at least 2 elements.");<br>
-<br>
- // Emit warning for one enum constant.<br>
- ECDVector::iterator I = Vec->begin();<br>
- S.Diag((*I)->getLocation(), diag::warn_duplicate_enum_values)<br>
- << (*I)->getName() << (*I)->getInitVal().toString(10)<br>
- << (*I)->getSourceRange();<br>
- ++I;<br>
-<br>
- // Emit one note for each of the remaining enum constants with<br>
- // the same value.<br>
- for (ECDVector::iterator E = Vec->end(); I != E; ++I)<br>
- S.Diag((*I)->getLocation(), diag::note_duplicate_element)<br>
- << (*I)->getName() << (*I)->getInitVal().toString(10)<br>
- << (*I)->getSourceRange();<br>
- delete Vec;<br>
- }<br>
-}<br>
-<br>
void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceLocation LBraceLoc,<br>
SourceLocation RBraceLoc, Decl *EnumDeclX,<br>
Decl **Elements, unsigned NumElements,<br>
@@ -10970,9 +10743,6 @@<br>
// it needs to go into the function scope.<br>
if (InFunctionDeclarator)<br>
DeclsInPrototypeScope.push_back(Enum);<br>
-<br>
- CheckForUniqueEnumValues(*this, Elements, NumElements, Enum, EnumType);<br>
- CheckForDuplicateEnumValues(*this, Elements, NumElements, Enum, EnumType);<br>
}<br>
<br>
Decl *Sema::ActOnFileScopeAsmDecl(Expr *expr,<br>
<br>
Removed: cfe/trunk/test/Sema/warn-duplicate-enum.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-duplicate-enum.c?rev=164082&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-duplicate-enum.c?rev=164082&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Sema/warn-duplicate-enum.c (original)<br>
+++ cfe/trunk/test/Sema/warn-duplicate-enum.c (removed)<br>
@@ -1,92 +0,0 @@<br>
-// RUN: %clang_cc1 %s -fsyntax-only -verify -Wduplicate-enum<br>
-// RUN: %clang_cc1 %s -x c++ -fsyntax-only -verify -Wduplicate-enum<br>
-enum A {<br>
- A1 = 0, // expected-note {{element A1 also has value 0}}<br>
- A2 = -1,<br>
- A3, // expected-warning {{element A3 has been implicitly assigned 0 which another element has been assigned}}<br>
- A4};<br>
-<br>
-enum B {<br>
- B1 = -1, // expected-note {{element B1 also has value -1}}<br>
- B2, // expected-warning {{element B2 has been implicitly assigned 0 which another element has been assigned}}<br>
- B3,<br>
- B4 = -2,<br>
- B5, // expected-warning {{element B5 has been implicitly assigned -1 which another element has been assigned}}<br>
- B6 // expected-note {{element B6 also has value 0}}<br>
-};<br>
-<br>
-enum C { C1, C2 = -1, C3 }; // expected-warning{{element C1 has been implicitly assigned 0 which another element has been assigned}} \<br>
- // expected-note {{element C3 also has value 0}}<br>
-<br>
-enum D {<br>
- D1,<br>
- D2,<br>
- D3, // expected-warning{{element D3 has been implicitly assigned 2 which another element has been assigned}}<br>
- D4 = D2, // no warning<br>
- D5 = 2 // expected-note {{element D5 also has value 2}}<br>
-};<br>
-<br>
-enum E {<br>
- E1,<br>
- E2 = E1,<br>
- E3 = E2<br>
-};<br>
-<br>
-enum F {<br>
- F1,<br>
- F2,<br>
- FCount,<br>
- FMax = FCount - 1<br>
-};<br>
-<br>
-enum G {<br>
- G1,<br>
- G2,<br>
- GMax = G2,<br>
- GCount = GMax + 1<br>
-};<br>
-<br>
-enum {<br>
- H1 = 0,<br>
- H2 = -1,<br>
- H3,<br>
- H4};<br>
-<br>
-enum {<br>
- I1 = -1,<br>
- I2,<br>
- I3,<br>
- I4 = -2,<br>
- I5,<br>
- I6<br>
-};<br>
-<br>
-enum { J1, J2 = -1, J3 };<br>
-<br>
-enum {<br>
- K1,<br>
- K2,<br>
- K3,<br>
- K4 = K2,<br>
- K5 = 2<br>
-};<br>
-<br>
-enum {<br>
- L1,<br>
- L2 = L1,<br>
- L3 = L2<br>
-};<br>
-<br>
-enum {<br>
- M1,<br>
- M2,<br>
- MCount,<br>
- MMax = MCount - 1<br>
-};<br>
-<br>
-enum {<br>
- N1,<br>
- N2,<br>
- NMax = N2,<br>
- NCount = NMax + 1<br>
-};<br>
<br>
Removed: cfe/trunk/test/SemaCXX/warn-unique-enum.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unique-enum.cpp?rev=164082&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unique-enum.cpp?rev=164082&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/warn-unique-enum.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/warn-unique-enum.cpp (removed)<br>
@@ -1,27 +0,0 @@<br>
-// RUN: %clang_cc1 %s -fsyntax-only -verify -Wunique-enum<br>
-enum A { A1 = 1, A2 = 1, A3 = 1 }; // expected-warning {{all elements of 'A' are initialized with literals to value 1}} \<br>
-// expected-note {{initialize the last element with the previous element to silence this warning}}<br>
-enum { B1 = 1, B2 = 1, B3 = 1 }; // no warning<br>
-enum C { // expected-warning {{all elements of 'C' are initialized with literals to value 1}}<br>
- C1 = true,<br>
- C2 = true // expected-note {{initialize the last element with the previous element to silence this warning}}<br>
-};<br>
-enum D { D1 = 5, D2 = 5L, D3 = 5UL, D4 = 5LL, D5 = 5ULL }; // expected-warning {{all elements of 'D' are initialized with literals to value 5}} \<br>
-// expected-note {{initialize the last element with the previous element to silence this warning}}<br>
-<br>
-// Don't warn on enums with less than 2 elements.<br>
-enum E { E1 = 4 };<br>
-enum F { F1 };<br>
-enum G {};<br>
-<br>
-// Don't warn when integer literals do not initialize the elements.<br>
-enum H { H1 = 4, H_MAX = H1, H_MIN = H1 };<br>
-enum I { I1 = H1, I2 = 4 };<br>
-enum J { J1 = 4, J2 = I2 };<br>
-enum K { K1, K2, K3, K4 };<br>
-<br>
-// Don't crash or warn on this one.<br>
-// rdar://11875995<br>
-enum L {<br>
- L1 = 0x8000000000000000ULL, L2 = 0x0000000000000001ULL<br>
-};<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div><br>
</blockquote></div>Why was -Wduplicate-enum removed? It is a separate warning from -Wunique-enum.</div>