From 97c5a1e73508b5df7ee8eeb3d28bee4efc543c16 Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Tue, 18 Sep 2012 00:41:42 +0000 Subject: [PATCH] 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. llvm-svn: 164083 --- .../clang/Basic/DiagnosticSemaKinds.td | 11 - clang/lib/Sema/SemaDecl.cpp | 230 ------------------ clang/test/Sema/warn-duplicate-enum.c | 92 ------- clang/test/SemaCXX/warn-unique-enum.cpp | 27 -- 4 files changed, 360 deletions(-) delete mode 100644 clang/test/Sema/warn-duplicate-enum.c delete mode 100644 clang/test/SemaCXX/warn-unique-enum.cpp diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index db3e62908ff7..359b03ae6a93 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -20,17 +20,6 @@ def warn_variables_not_in_loop_body : Warning< "used in loop condition not modified in loop body">, InGroup>, DefaultIgnore; -def warn_identical_enum_values : Warning< - "all elements of %0 are initialized with literals to value %1">, - InGroup>; -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>, 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">; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index edcfcb68e1f7..83dc86f575c3 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -10521,233 +10521,6 @@ Decl *Sema::ActOnEnumConstant(Scope *S, Decl *theEnumDecl, Decl *lastEnumConst, 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(Elements[i]); - if (!ECD) - return; - - Expr *InitExpr = ECD->getInitExpr(); - if (!InitExpr) - return; - InitExpr = InitExpr->IgnoreImpCasts(); - if (!isa(InitExpr) && !isa(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(Elements[NumElements - 1]), - *Next = cast(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(InitExpr)) { - if (!BO->isAdditiveOp()) - return true; - IntegerLiteral *IL = dyn_cast(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(InitExpr); - if (!DRE) - return true; - - EnumConstantDecl *EnumConstant = dyn_cast(DRE->getDecl()); - if (!EnumConstant) - return true; - - if (cast(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 ECDVector; - typedef llvm::SmallVector DuplicatesVector; - - typedef llvm::PointerUnion DeclOrVector; - typedef llvm::DenseMap - 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(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(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()) { - // 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(); - // 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 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceLocation LBraceLoc, // 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, diff --git a/clang/test/Sema/warn-duplicate-enum.c b/clang/test/Sema/warn-duplicate-enum.c deleted file mode 100644 index 239f6f1995c3..000000000000 --- a/clang/test/Sema/warn-duplicate-enum.c +++ /dev/null @@ -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 -}; diff --git a/clang/test/SemaCXX/warn-unique-enum.cpp b/clang/test/SemaCXX/warn-unique-enum.cpp deleted file mode 100644 index 59a127807172..000000000000 --- a/clang/test/SemaCXX/warn-unique-enum.cpp +++ /dev/null @@ -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 -};