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 >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
