[cfe-commits] r164083 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp test/Sema/warn-duplicate-enum.c test/SemaCXX/warn-unique-enum.cpp
Richard Trieu
rtrieu at google.com
Thu Nov 1 14:08:10 PDT 2012
On Wed, Oct 3, 2012 at 2:27 PM, Richard Trieu <rtrieu at google.com> wrote:
> On Mon, Sep 17, 2012 at 5:41 PM, Ted Kremenek <kremenek at apple.com> wrote:
>
>> Author: kremenek
>> Date: Mon Sep 17 19:41:42 2012
>> New Revision: 164083
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=164083&view=rev
>> Log:
>> Per discussion on cfe-dev, remove -Wunique-enums entirely. There
>> is no compelling argument that this is a generally useful warning,
>> and imposes a strong stylistic argument on code beyond what it was
>> intended to find warnings in.
>>
>> Removed:
>> cfe/trunk/test/Sema/warn-duplicate-enum.c
>> cfe/trunk/test/SemaCXX/warn-unique-enum.cpp
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/lib/Sema/SemaDecl.cpp
>>
>> Hey Ted,
>
> Why was -Wduplicate-enum also removed in this commit?
>
> Richard.
>
>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=164083&r1=164082&r2=164083&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Sep 17
>> 19:41:42 2012
>> @@ -20,17 +20,6 @@
>> "used in loop condition not modified in loop body">,
>> InGroup<DiagGroup<"loop-analysis">>, DefaultIgnore;
>>
>> -def warn_identical_enum_values : Warning<
>> - "all elements of %0 are initialized with literals to value %1">,
>> - InGroup<DiagGroup<"unique-enum">>;
>> -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<
>> "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=164083&r1=164082&r2=164083&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Sep 17 19:41:42 2012
>> @@ -10521,233 +10521,6 @@
>> return New;
>> }
>>
>> -// Emits a warning if every element in the enum is the same value and if
>> -// every element is initialized with a integer or boolean literal.
>> -static void CheckForUniqueEnumValues(Sema &S, Decl **Elements,
>> - unsigned NumElements, EnumDecl
>> *Enum,
>> - QualType EnumType) {
>> - if (S.Diags.getDiagnosticLevel(diag::warn_identical_enum_values,
>> - Enum->getLocation()) ==
>> - DiagnosticsEngine::Ignored)
>> - return;
>> -
>> - if (NumElements < 2)
>> - return;
>> -
>> - if (!Enum->getIdentifier())
>> - return;
>> -
>> - llvm::APSInt FirstVal;
>> -
>> - for (unsigned i = 0; i != NumElements; ++i) {
>> - EnumConstantDecl *ECD = cast_or_null<EnumConstantDecl>(Elements[i]);
>> - if (!ECD)
>> - return;
>> -
>> - Expr *InitExpr = ECD->getInitExpr();
>> - if (!InitExpr)
>> - return;
>> - InitExpr = InitExpr->IgnoreImpCasts();
>> - if (!isa<IntegerLiteral>(InitExpr) &&
>> !isa<CXXBoolLiteralExpr>(InitExpr))
>> - return;
>> -
>> - if (i == 0) {
>> - FirstVal = ECD->getInitVal();
>> - continue;
>> - }
>> -
>> - if (!llvm::APSInt::isSameValue(FirstVal, ECD->getInitVal()))
>> - return;
>> - }
>> -
>> - S.Diag(Enum->getLocation(), diag::warn_identical_enum_values)
>> - << EnumType << FirstVal.toString(10)
>> - << Enum->getSourceRange();
>> -
>> - EnumConstantDecl *Last = cast<EnumConstantDecl>(Elements[NumElements -
>> 1]),
>> - *Next = cast<EnumConstantDecl>(Elements[NumElements -
>> 2]);
>> -
>> - S.Diag(Last->getLocation(), diag::note_identical_enum_values)
>> - <<
>> FixItHint::CreateReplacement(Last->getInitExpr()->getSourceRange(),
>> - 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,
>> @@ -10970,9 +10743,6 @@
>> // it needs to go into the function scope.
>> if (InFunctionDeclarator)
>> DeclsInPrototypeScope.push_back(Enum);
>> -
>> - CheckForUniqueEnumValues(*this, Elements, NumElements, Enum, EnumType);
>> - CheckForDuplicateEnumValues(*this, Elements, NumElements, Enum,
>> EnumType);
>> }
>>
>> Decl *Sema::ActOnFileScopeAsmDecl(Expr *expr,
>>
>> Removed: cfe/trunk/test/Sema/warn-duplicate-enum.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-duplicate-enum.c?rev=164082&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Sema/warn-duplicate-enum.c (original)
>> +++ cfe/trunk/test/Sema/warn-duplicate-enum.c (removed)
>> @@ -1,92 +0,0 @@
>> -// 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
>> -};
>>
>> Removed: cfe/trunk/test/SemaCXX/warn-unique-enum.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unique-enum.cpp?rev=164082&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/warn-unique-enum.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/warn-unique-enum.cpp (removed)
>> @@ -1,27 +0,0 @@
>> -// RUN: %clang_cc1 %s -fsyntax-only -verify -Wunique-enum
>> -enum A { A1 = 1, A2 = 1, A3 = 1 }; // expected-warning {{all elements
>> of 'A' are initialized with literals to value 1}} \
>> -// expected-note {{initialize the last element with the previous element
>> to silence this warning}}
>> -enum { B1 = 1, B2 = 1, B3 = 1 }; // no warning
>> -enum C { // expected-warning {{all elements of 'C' are initialized with
>> literals to value 1}}
>> - C1 = true,
>> - C2 = true // expected-note {{initialize the last element with the
>> previous element to silence this warning}}
>> -};
>> -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}} \
>> -// expected-note {{initialize the last element with the previous element
>> to silence this warning}}
>> -
>> -// Don't warn on enums with less than 2 elements.
>> -enum E { E1 = 4 };
>> -enum F { F1 };
>> -enum G {};
>> -
>> -// Don't warn when integer literals do not initialize the elements.
>> -enum H { H1 = 4, H_MAX = H1, H_MIN = H1 };
>> -enum I { I1 = H1, I2 = 4 };
>> -enum J { J1 = 4, J2 = I2 };
>> -enum K { K1, K2, K3, K4 };
>> -
>> -// Don't crash or warn on this one.
>> -// rdar://11875995
>> -enum L {
>> - L1 = 0x8000000000000000ULL, L2 = 0x0000000000000001ULL
>> -};
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
> Why was -Wduplicate-enum removed? It is a separate warning from
-Wunique-enum.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121101/607292ef/attachment.html>
More information about the cfe-commits
mailing list