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.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@108128 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 12 insertions, 13 deletions

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index b907675b96..57b4232294 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -5319,17 +5319,18 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
     // 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-
diff --git a/test/Sema/self-comparison.c b/test/Sema/self-comparison.c
index 27d1eb3d40..c5c0611e7c 100644
--- a/test/Sema/self-comparison.c
+++ b/test/Sema/self-comparison.c
@@ -34,12 +34,10 @@ int bar2(float x) {
   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.