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

[Chandler Carruth]

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.