[cfe-commits] r157666 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp test/Sema/switch.c test/SemaCXX/warn-unique-enum.cpp

[David Blaikie]

On Tue, May 29, 2012 at 6:01 PM, Richard Trieu <rtrieu at google.com> wrote:

> Author: rtrieu
> Date: Tue May 29 20:01:11 2012
> New Revision: 157666
>
> URL: http://llvm.org/viewvc/llvm-project?rev=157666&view=rev
> Log:
> Add new -Wunique-enum which will warn on enums which all elements have the
> same value and were initialized with literals.


This currently produces a false positive on some anonymous-enum'd constants
in Diagnostic.h:635:

  *enum* {
    */// MaxArguments - The maximum number of arguments we can hold.
We currently*    */// only support up to 10 arguments (%0-%9).  A
single diagnostic with more*    */// than that almost certainly has to
be simplified anyway.*    MaxArguments = 10,

    */// MaxRanges - The maximum number of ranges we can hold.*
MaxRanges = 10,

    */// MaxFixItHints - The maximum number of ranges we can hold.*
MaxFixItHints = 10
  };
Perhaps we should ignore this warning if the enum is unnamed? Or
specifically if it's both unnamed and has no instances (so it could
still fire on "enum { x, y = 0 } a, b;")

Since this warning is on by default, this breaks a self-host Clang
-Werror build.

- David

Clang will warn on code like
> this:
>
> enum A {
>  FIRST = 1,
>  SECOND = 1
> };
>
>
> Added:
>    cfe/trunk/test/SemaCXX/warn-unique-enum.cpp
> Modified:
>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>    cfe/trunk/lib/Sema/SemaDecl.cpp
>    cfe/trunk/test/Sema/switch.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=157666&r1=157665&r2=157666&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May 29
> 20:01:11 2012
> @@ -20,6 +20,10 @@
>   "used in loop condition not modified in loop body">,
>   InGroup<DiagGroup<"loop-analysis">>, DefaultIgnore;
>
> +def warn_identical_enum_values : Warning<
> +  "all elements of %select{anonymous enum|%1}0 are initialized with
> literals "
> +  "to value %2">, InGroup<DiagGroup<"unique-enum">>;
> +
>  // 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=157666&r1=157665&r2=157666&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue May 29 20:01:11 2012
> @@ -10232,6 +10232,48 @@
>   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;
> +
> +  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 (FirstVal != ECD->getInitVal())
> +      return;
> +  }
> +
> +  bool hasIdentifier = Enum->getIdentifier();
> +  S.Diag(Enum->getLocation(), diag::warn_identical_enum_values)
> +      << hasIdentifier << EnumType << FirstVal.toString(10)
> +      << Enum->getSourceRange();
> +}
> +
>  void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceLocation LBraceLoc,
>                          SourceLocation RBraceLoc, Decl *EnumDeclX,
>                          Decl **Elements, unsigned NumElements,
> @@ -10455,6 +10497,7 @@
>   if (InFunctionDeclarator)
>     DeclsInPrototypeScope.push_back(Enum);
>
> +  CheckForUniqueEnumValues(*this, Elements, NumElements, Enum, EnumType);
>  }
>
>  Decl *Sema::ActOnFileScopeAsmDecl(Expr *expr,
>
> Modified: cfe/trunk/test/Sema/switch.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/switch.c?rev=157666&r1=157665&r2=157666&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Sema/switch.c (original)
> +++ cfe/trunk/test/Sema/switch.c Tue May 29 20:01:11 2012
> @@ -326,7 +326,7 @@
>  void test19(int i) {
>   enum {
>     kTest19Enum1 = 7,
> -    kTest19Enum2 = 7
> +    kTest19Enum2 = kTest19Enum1
>   };
>   const int a = 3;
>   switch (i) {
>
> Added: cfe/trunk/test/SemaCXX/warn-unique-enum.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unique-enum.cpp?rev=157666&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-unique-enum.cpp (added)
> +++ cfe/trunk/test/SemaCXX/warn-unique-enum.cpp Tue May 29 20:01:11 2012
> @@ -0,0 +1,16 @@
> +// 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}}
> +enum { B1 = 1, B2 = 1, B3 = 1 };  // expected-warning {{all elements of
> anonymous enum are initialized with literals to value 1}}
> +enum C { C1 = true, C2 = true}; // expected-warning {{all elements of 'C'
> are initialized with literals to value 1}}
> +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}}
> +
> +// 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 };
>
>
> _______________________________________________
> 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/20120530/65279b7b/attachment.html>