[cfe-commits] r108128 - in /cfe/trunk: lib/Sema/SemaExpr.cpp test/Sema/self-comparison.c

[Chris Lattner]

Looks good to me, thanks Chandler.  Should the PR be closed?

-Chris

On Jul 11, 2010, at 11:23 PM, Chandler Carruth wrote:

> Author: chandlerc
> Date: Mon Jul 12 01:23:38 2010
> New Revision: 108128
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=108128&view=rev
> Log:
> Fix another aspect of PR7047, macro expansions. Previously, this was hacked
> around by exempting enums from the check, but this doesn't handle a lot of
> cases. A better approach is to directly check if the operator comes from
> a macro expansion.
> 
> I've removed a reference to the rdar that originally led to the enum
> suppression when removing it's overly contrived test case. Let me know if that
> number or a more reasilistic test case involving enums is still needed.
> 
> Modified:
>    cfe/trunk/lib/Sema/SemaExpr.cpp
>    cfe/trunk/test/Sema/self-comparison.c
> 
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=108128&r1=108127&r2=108128&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Jul 12 01:23:38 2010
> @@ -5319,17 +5319,18 @@
>     // For non-floating point types, check for self-comparisons of the form
>     // x == x, x != x, x < x, etc.  These always evaluate to a constant, and
>     // often indicate logic errors in the program.
> -    // NOTE: Don't warn about comparisons of enum constants. These can arise
> -    //  from macro expansions, and are usually quite deliberate. Also don't
> -    //  warn about comparisons which are only self comparisons within
> -    //  a template specialization. The warnings should catch obvious cases in
> -    //  the definition of the template anyways.
> +    //
> +    // NOTE: Don't warn about comparison expressions resulting from macro
> +    // expansion. Also don't warn about comparisons which are only self
> +    // comparisons within a template specialization. The warnings should catch
> +    // obvious cases in the definition of the template anyways. The idea is to
> +    // warn when the typed comparison operator will always evaluate to the same
> +    // result.
>     Expr *LHSStripped = lex->IgnoreParens();
>     Expr *RHSStripped = rex->IgnoreParens();
>     if (DeclRefExpr* DRL = dyn_cast<DeclRefExpr>(LHSStripped)) {
>       if (DeclRefExpr* DRR = dyn_cast<DeclRefExpr>(RHSStripped)) {
> -        if (DRL->getDecl() == DRR->getDecl() &&
> -            !isa<EnumConstantDecl>(DRL->getDecl()) &&
> +        if (DRL->getDecl() == DRR->getDecl() && !Loc.isMacroID() &&
>             !IsWithinTemplateSpecialization(DRL->getDecl())) {
>           DiagRuntimeBehavior(Loc, PDiag(diag::warn_comparison_always)
>                               << 0 // self-
> 
> Modified: cfe/trunk/test/Sema/self-comparison.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/self-comparison.c?rev=108128&r1=108127&r2=108128&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/self-comparison.c (original)
> +++ cfe/trunk/test/Sema/self-comparison.c Mon Jul 12 01:23:38 2010
> @@ -34,12 +34,10 @@
>   return x != x; // no-warning
> }
> 
> -// Motivated by <rdar://problem/6703892>, self-comparisons of enum constants
> -// should not be warned about.  These can be expanded from macros, and thus
> -// are usually deliberate.
> -int compare_enum() {
> -  enum { A };
> -  return A == A; // no-warning
> +#define IS_THE_ANSWER(x) (x == 42)
> +
> +int macro_comparison() {
> +  return IS_THE_ANSWER(42);
> }
> 
> // Don't complain in unevaluated contexts.
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits