[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
Wed Oct 3 14:27:38 PDT 2012
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121003/6dffd6bd/attachment.html>
More information about the cfe-commits
mailing list