Thanks Ted. I can see how these two warnings would get confused. I did work on both around the same time, both deal with enums and are adjacent to each other in code. I do hope that users find this warning useful.
On Fri, Dec 21, 2012 at 5:22 PM, Ted Kremenek <[email protected]> wrote: > Hi Richard, > > I was looking through my unread messages, and spotted this one. I'm so > sorry for missing this. > > I'm trying to page this back in now, and it all centered around the > following thread on cfe-dev: > > http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-September/024191.html > > That thread clearly discusses -Wunique-enum. I've long paged this out, so > I tried to look back and the thread and the original complaints I had > received against -Wunique-enum from users and I don't see anything about > -Wduplicate-enum. I honestly may have intended to rip it out, or maybe I > just got confused because the names were so close. Since I don't know, I > think it's reasonable to put the warning back in. If we get user feedback > that specifically complains about -Wduplicate-enum (instead of > -Wunique-enum), we can act based on concrete information. > > Ted > > On Nov 1, 2012, at 2:08 PM, Richard Trieu <[email protected]> wrote: > > 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
