Remove warning for conditional operands of differend signedness from -Wsign-compare. Cases that previously warn on this will have a different warning emitted from -Wsign-conversion.

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

7 files changed, 128 insertions, 113 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 2b96e0bef1..fec587e4ae 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3054,9 +3054,6 @@ def err_stmtexpr_file_scope : Error<
 def warn_mixed_sign_comparison : Warning<
   "comparison of integers of different signs: %0 and %1">,
   InGroup<SignCompare>, DefaultIgnore;
-def warn_mixed_sign_conditional : Warning<
-  "operands of ? are integers of different signs: %0 and %1">,
-  InGroup<SignCompare>, DefaultIgnore;
 def warn_lunsigned_always_true_comparison : Warning<
   "comparison of unsigned%select{| enum}2 expression %0 is always %1">,
   InGroup<TautologicalCompare>;
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 690a29d281..cdcacd84d7 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -3279,29 +3279,16 @@ void CheckConditionalOperator(Sema &S, ConditionalOperator *E, QualType T) {
                                  CC))
     return;
 
-  // ...and -Wsign-compare isn't...
-  if (!S.Diags.getDiagnosticLevel(diag::warn_mixed_sign_conditional, CC))
-    return;
-
   // ...then check whether it would have warned about either of the
   // candidates for a signedness conversion to the condition type.
-  if (E->getType() != T) {
-    Suspicious = false;
-    CheckImplicitConversion(S, E->getTrueExpr()->IgnoreParenImpCasts(),
+  if (E->getType() == T) return;
+ 
+  Suspicious = false;
+  CheckImplicitConversion(S, E->getTrueExpr()->IgnoreParenImpCasts(),
+                          E->getType(), CC, &Suspicious);
+  if (!Suspicious)
+    CheckImplicitConversion(S, E->getFalseExpr()->IgnoreParenImpCasts(),
                             E->getType(), CC, &Suspicious);
-    if (!Suspicious)
-      CheckImplicitConversion(S, E->getFalseExpr()->IgnoreParenImpCasts(),
-                              E->getType(), CC, &Suspicious);
-    if (!Suspicious)
-      return;
-  }
-
-  // If so, emit a diagnostic under -Wsign-compare.
-  Expr *lex = E->getTrueExpr()->IgnoreParenImpCasts();
-  Expr *rex = E->getFalseExpr()->IgnoreParenImpCasts();
-  S.Diag(E->getQuestionLoc(), diag::warn_mixed_sign_conditional)
-    << lex->getType() << rex->getType()
-    << lex->getSourceRange() << rex->getSourceRange();
 }
 
 /// AnalyzeImplicitConversions - Find and report any interesting
diff --git a/test/Sema/conditional-expr.c b/test/Sema/conditional-expr.c
index 7a8c9e9f36..436ecdbebc 100644
--- a/test/Sema/conditional-expr.c
+++ b/test/Sema/conditional-expr.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -pedantic -Wsign-compare %s
+// RUN: %clang_cc1 -fsyntax-only -verify -pedantic -Wsign-conversion %s
 void foo() {
   *(0 ? (double *)0 : (void *)0) = 0;
   // FIXME: GCC doesn't consider the the following two statements to be errors.
@@ -36,12 +36,12 @@ void foo() {
   *(0 ? (asdf) 0 : &x) = 10;
 
   unsigned long test0 = 5;
-  test0 = test0 ? (long) test0 : test0; // expected-warning {{operands of ? are integers of different signs}}
-  test0 = test0 ? (int) test0 : test0; // expected-warning {{operands of ? are integers of different signs}}
-  test0 = test0 ? (short) test0 : test0; // expected-warning {{operands of ? are integers of different signs}}
-  test0 = test0 ? test0 : (long) test0; // expected-warning {{operands of ? are integers of different signs}}
-  test0 = test0 ? test0 : (int) test0; // expected-warning {{operands of ? are integers of different signs}}
-  test0 = test0 ? test0 : (short) test0; // expected-warning {{operands of ? are integers of different signs}}
+  test0 = test0 ? (long) test0 : test0; // expected-warning {{operand of ? changes signedness: 'long' to 'unsigned long'}}
+  test0 = test0 ? (int) test0 : test0; // expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
+  test0 = test0 ? (short) test0 : test0; // expected-warning {{operand of ? changes signedness: 'short' to 'unsigned long'}}
+  test0 = test0 ? test0 : (long) test0; // expected-warning {{operand of ? changes signedness: 'long' to 'unsigned long'}}
+  test0 = test0 ? test0 : (int) test0; // expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
+  test0 = test0 ? test0 : (short) test0; // expected-warning {{operand of ? changes signedness: 'short' to 'unsigned long'}}
   test0 = test0 ? test0 : (long) 10;
   test0 = test0 ? test0 : (int) 10;
   test0 = test0 ? test0 : (short) 10;
@@ -49,12 +49,17 @@ void foo() {
   test0 = test0 ? (int) 10 : test0;
   test0 = test0 ? (short) 10 : test0;
 
+  int test1;
   enum Enum { EVal };
   test0 = test0 ? EVal : test0;
-  test0 = test0 ? EVal : (int) test0; // okay: EVal is an int
-  test0 = test0 ? // expected-warning {{operands of ? are integers of different signs}}
+  test1 = test0 ? EVal : (int) test0;
+  test0 = test0 ?
                   (unsigned) EVal
-                : (int) test0;
+                : (int) test0;  // expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
+
+  test0 = test0 ? EVal : test1; // expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
+  test0 = test0 ? test1 : EVal; // expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
+
 }
 
 int Postgresql() {
diff --git a/test/SemaCXX/compare.cpp b/test/SemaCXX/compare.cpp
index ca8af2186f..28e2dd0ad5 100644
--- a/test/SemaCXX/compare.cpp
+++ b/test/SemaCXX/compare.cpp
@@ -212,3 +212,14 @@ static const unsigned int kMax = 0;
 int pr7536() {
   return (kMax > 0);
 }
+
+// -Wsign-compare should not warn when ?: operands have different signedness.
+// This will be caught by -Wsign-conversion
+void test3() {
+  unsigned long a;
+  signed long b;
+  (void) (true ? a : b);
+  (void) (true ? (unsigned int)a : (signed int)b);
+  (void) (true ? b : a);
+  (void) (true ? (unsigned char)b : (signed char)a);
+}
diff --git a/test/SemaCXX/conditional-expr.cpp b/test/SemaCXX/conditional-expr.cpp
index b95700e9ba..fa5c974d1f 100644
--- a/test/SemaCXX/conditional-expr.cpp
+++ b/test/SemaCXX/conditional-expr.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++0x -Wsign-compare %s
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++0x -Wsign-conversion %s
 
 // C++ rules for ?: are a lot stricter than C rules, and have to take into
 // account more conversion options.
@@ -180,12 +180,12 @@ void test()
   
 
   unsigned long test0 = 5;
-  test0 = test0 ? (long) test0 : test0; // expected-warning {{operands of ? are integers of different signs}}
-  test0 = test0 ? (int) test0 : test0; // expected-warning {{operands of ? are integers of different signs}}
-  test0 = test0 ? (short) test0 : test0; // expected-warning {{operands of ? are integers of different signs}}
-  test0 = test0 ? test0 : (long) test0; // expected-warning {{operands of ? are integers of different signs}}
-  test0 = test0 ? test0 : (int) test0; // expected-warning {{operands of ? are integers of different signs}}
-  test0 = test0 ? test0 : (short) test0; // expected-warning {{operands of ? are integers of different signs}}
+  test0 = test0 ? (long) test0 : test0; // expected-warning {{operand of ? changes signedness: 'long' to 'unsigned long'}}
+  test0 = test0 ? (int) test0 : test0; // expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
+  test0 = test0 ? (short) test0 : test0; // expected-warning {{operand of ? changes signedness: 'short' to 'unsigned long'}}
+  test0 = test0 ? test0 : (long) test0; // expected-warning {{operand of ? changes signedness: 'long' to 'unsigned long'}}
+  test0 = test0 ? test0 : (int) test0; // expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
+  test0 = test0 ? test0 : (short) test0; // expected-warning {{operand of ? changes signedness: 'short' to 'unsigned long'}}
   test0 = test0 ? test0 : (long) 10;
   test0 = test0 ? test0 : (int) 10;
   test0 = test0 ? test0 : (short) 10;
@@ -193,8 +193,15 @@ void test()
   test0 = test0 ? (int) 10 : test0;
   test0 = test0 ? (short) 10 : test0;
 
+  int test1;
   test0 = test0 ? EVal : test0;
-  test0 = test0 ? EVal : (int) test0;
+  test1 = test0 ? EVal : (int) test0;
+
+  test0 = test0 ? EVal : test1; // expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
+  test0 = test0 ? test1 : EVal; // expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
+
+  test1 = test0 ? EVal : (int) test0;
+  test1 = test0 ? (int) test0 : EVal;
 
   // Note the thing that this does not test: since DR446, various situations
   // *must* create a separate temporary copy of class objects. This can only
diff --git a/test/SemaCXX/warn-sign-compare.cpp b/test/SemaCXX/warn-sign-compare.cpp
deleted file mode 100644
index 3042bfde6e..0000000000
--- a/test/SemaCXX/warn-sign-compare.cpp
+++ /dev/null
@@ -1,72 +0,0 @@
-// RUN: %clang_cc1 -verify -fsyntax-only -Wsign-compare %s
-
-// NOTE: When a 'enumeral mismatch' warning is implemented then expect several
-// of the following cases to be impacted.
-
-// namespace for anonymous enums tests
-namespace test1 {
-  enum { A };
-  enum { B = -1 };
-
-  template <typename T> struct Foo {
-    enum { C };
-    enum { D = ~0U };
-  };
-
-  enum { E = ~0U };
-
-  void doit_anonymous( int i ) {
-    int a1 = 1 ? i : A;
-    int a2 = 1 ? A : i;
-
-    int b1 = 1 ? i : B;
-    int b2 = 1 ? B : i;
-
-    int c1 = 1 ? i : Foo<bool>::C;
-    int c2 = 1 ? Foo<bool>::C : i;
-
-    int d1 = 1 ? i : Foo<bool>::D; // expected-warning {{operands of ? are integers of different signs}}
-    int d2 = 1 ? Foo<bool>::D : i; // expected-warning {{operands of ? are integers of different signs}}
-    int d3 = 1 ? B : Foo<bool>::D; // expected-warning {{operands of ? are integers of different signs}}
-    int d4 = 1 ? Foo<bool>::D : B; // expected-warning {{operands of ? are integers of different signs}}
-
-    int e1 = 1 ? i : E; // expected-warning {{operands of ? are integers of different signs}}
-    int e2 = 1 ? E : i; // expected-warning {{operands of ? are integers of different signs}}
-    int e3 = 1 ? E : B; // expected-warning {{operands of ? are integers of different signs}}
-    int e4 = 1 ? B : E; // expected-warning {{operands of ? are integers of different signs}}
-  }
-}
-
-// namespace for named enums tests
-namespace test2 {
-  enum Named1 { A };
-  enum Named2 { B = -1 };
-
-  template <typename T> struct Foo {
-    enum Named3 { C };
-    enum Named4 { D = ~0U };
-  };
-
-  enum Named5 { E = ~0U };
-
-  void doit_anonymous( int i ) {
-    int a1 = 1 ? i : A;
-    int a2 = 1 ? A : i;
-
-    int b1 = 1 ? i : B;
-    int b2 = 1 ? B : i;
-
-    int c1 = 1 ? i : Foo<bool>::C;
-    int c2 = 1 ? Foo<bool>::C : i;
-
-    int d1 = 1 ? i : Foo<bool>::D; // expected-warning {{operands of ? are integers of different signs}}
-    int d2 = 1 ? Foo<bool>::D : i; // expected-warning {{operands of ? are integers of different signs}}
-    int d3 = 1 ? B : Foo<bool>::D; // expected-warning {{operands of ? are integers of different signs}}
-    int d4 = 1 ? Foo<bool>::D : B; // expected-warning {{operands of ? are integers of different signs}}
-
-    int e1 = 1 ? i : E; // expected-warning {{operands of ? are integers of different signs}}
-    int e2 = 1 ? E : i; // expected-warning {{operands of ? are integers of different signs}}
-    int e3 = 1 ? E : B; // expected-warning {{operands of ? are integers of different signs}}
-    int e4 = 1 ? B : E; // expected-warning {{operands of ? are integers of different signs}}
-  }
-}
diff --git a/test/SemaCXX/warn-sign-conversion.cpp b/test/SemaCXX/warn-sign-conversion.cpp
new file mode 100644
index 0000000000..77aa958068
--- /dev/null
+++ b/test/SemaCXX/warn-sign-conversion.cpp
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -Wsign-conversion %s
+
+// NOTE: When a 'enumeral mismatch' warning is implemented then expect several
+// of the following cases to be impacted.
+
+// namespace for anonymous enums tests
+namespace test1 {
+  enum { A };
+  enum { B = -1 };
+
+  template <typename T> struct Foo {
+    enum { C };
+    enum { D = ~0U };
+  };
+
+  enum { E = ~0U };
+
+  void doit_anonymous( int i ) {
+    int a1 = 1 ? i : A;
+    int a2 = 1 ? A : i;
+
+    int b1 = 1 ? i : B;
+    int b2 = 1 ? B : i;
+
+    int c1 = 1 ? i : Foo<bool>::C;
+    int c2 = 1 ? Foo<bool>::C : i;
+
+    int d1a = 1 ? i : Foo<bool>::D; // expected-warning {{test1::Foo<bool>::<anonymous enum at }}
+    int d1b = 1 ? i : Foo<bool>::D; // expected-warning {{warn-sign-conversion.cpp:13:5>' to 'int'}}
+    int d2a = 1 ? Foo<bool>::D : i; // expected-warning {{operand of ? changes signedness: 'test1::Foo<bool>::<anonymous enum at }}
+    int d2b = 1 ? Foo<bool>::D : i; // expected-warning {{SemaCXX/warn-sign-conversion.cpp:13:5>' to 'int'}}
+    int d3a = 1 ? B : Foo<bool>::D; // expected-warning {{operand of ? changes signedness: 'test1::Foo<bool>::<anonymous enum at }}
+    int d3b = 1 ? B : Foo<bool>::D; // expected-warning {{warn-sign-conversion.cpp:13:5>' to 'int'}}
+    int d4a = 1 ? Foo<bool>::D : B; // expected-warning {{operand of ? changes signedness: 'test1::Foo<bool>::<anonymous enum at }}
+    int d4b = 1 ? Foo<bool>::D : B; // expected-warning {{warn-sign-conversion.cpp:13:5>' to 'int'}}
+
+    int e1a = 1 ? i : E; // expected-warning {{operand of ? changes signedness: 'test1::<anonymous enum at }}
+    int e1b = 1 ? i : E; // expected-warning {{warn-sign-conversion.cpp:16:3>' to 'int'}}
+    int e2a = 1 ? E : i; // expected-warning {{operand of ? changes signedness: 'test1::<anonymous enum at }}
+    int e2b = 1 ? E : i; // expected-warning {{warn-sign-conversion.cpp:16:3>' to 'int'}}
+    int e3a = 1 ? E : B; // expected-warning {{operand of ? changes signedness: 'test1::<anonymous enum at }}
+    int e3b = 1 ? E : B; // expected-warning {{warn-sign-conversion.cpp:16:3>' to 'int'}}
+    int e4a = 1 ? B : E; // expected-warning {{operand of ? changes signedness: 'test1::<anonymous enum at }}
+    int e4b = 1 ? B : E; // expected-warning {{warn-sign-conversion.cpp:16:3>' to 'int'}}
+  }
+}
+
+// namespace for named enums tests
+namespace test2 {
+  enum Named1 { A };
+  enum Named2 { B = -1 };
+
+  template <typename T> struct Foo {
+    enum Named3 { C };
+    enum Named4 { D = ~0U };
+  };
+
+  enum Named5 { E = ~0U };
+
+  void doit_anonymous( int i ) {
+    int a1 = 1 ? i : A;
+    int a2 = 1 ? A : i;
+
+    int b1 = 1 ? i : B;
+    int b2 = 1 ? B : i;
+
+    int c1 = 1 ? i : Foo<bool>::C;
+    int c2 = 1 ? Foo<bool>::C : i;
+
+    int d1 = 1 ? i : Foo<bool>::D; // expected-warning {{operand of ? changes signedness: 'test2::Foo<bool>::Named4' to 'int'}}
+    int d2 = 1 ? Foo<bool>::D : i; // expected-warning {{operand of ? changes signedness: 'test2::Foo<bool>::Named4' to 'int'}}
+    int d3 = 1 ? B : Foo<bool>::D; // expected-warning {{operand of ? changes signedness: 'test2::Foo<bool>::Named4' to 'int'}}
+    int d4 = 1 ? Foo<bool>::D : B; // expected-warning {{operand of ? changes signedness: 'test2::Foo<bool>::Named4' to 'int'}}
+
+    int e1 = 1 ? i : E; // expected-warning {{operand of ? changes signedness: 'test2::Named5' to 'int'}}
+    int e2 = 1 ? E : i; // expected-warning {{operand of ? changes signedness: 'test2::Named5' to 'int'}}
+    int e3 = 1 ? E : B; // expected-warning {{operand of ? changes signedness: 'test2::Named5' to 'int'}}
+    int e4 = 1 ? B : E; // expected-warning {{operand of ? changes signedness: 'test2::Named5' to 'int'}}
+  }
+}