On Wed, Oct 3, 2012 at 2:27 PM, Richard Trieu <[email protected]> wrote:
> On Mon, Sep 17, 2012 at 5:41 PM, Ted Kremenek <[email protected]> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > > Why was -Wduplicate-enum removed? It is a separate warning from -Wunique-enum.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
