[clang] 6954515 - [Sema] Move 'char-expression-as-unsigned < 0' into a separate diagnostic

[Anton Bikineev via cfe-commits]

Author: Anton Bikineev
Date: 2021-04-14T01:01:40+02:00
New Revision: 69545154cc28a0a7f813174253c6cb428666eb3a

URL: https://github.com/llvm/llvm-project/commit/69545154cc28a0a7f813174253c6cb428666eb3a
DIFF: https://github.com/llvm/llvm-project/commit/69545154cc28a0a7f813174253c6cb428666eb3a.diff

LOG: [Sema] Move 'char-expression-as-unsigned < 0' into a separate diagnostic

This change splits '-Wtautological-unsigned-zero-compare' by reporting
char-expressions-interpreted-as-unsigned under a separate diagnostic
'-Wtautological-unsigned-char-zero-compare'. This is beneficial for
projects that want to enable '-Wtautological-unsigned-zero-compare' but at
the same time want to keep code portable for platforms with char being
signed or unsigned, such as Chromium.

Differential Revision: https://reviews.llvm.org/D99808

Added: 
    clang/test/Sema/tautological-unsigned-char-zero-compare.cc

Modified: 
    clang/include/clang/Basic/DiagnosticGroups.td
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/Sema/SemaChecking.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 85f798013a3d..e202645d1f2b 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -591,11 +591,13 @@ def SwiftNameAttribute : DiagGroup<"swift-name-attribute">;
 def IntInBoolContext : DiagGroup<"int-in-bool-context">;
 def TautologicalTypeLimitCompare : DiagGroup<"tautological-type-limit-compare">;
 def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">;
+def TautologicalUnsignedCharZeroCompare : DiagGroup<"tautological-unsigned-char-zero-compare">;
 def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">;
 // For compatibility with GCC. Tautological comparison warnings for constants
 // that are an extremal value of the type.
 def TypeLimits : DiagGroup<"type-limits", [TautologicalTypeLimitCompare,
                                            TautologicalUnsignedZeroCompare,
+                                           TautologicalUnsignedCharZeroCompare,
                                            TautologicalUnsignedEnumZeroCompare]>;
 // Additional tautological comparison warnings based on the expression, not
 // only on its type.

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index af7eea06f6f5..afef86ab5890 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6784,6 +6784,10 @@ def warn_unsigned_always_true_comparison : Warning<
   "result of comparison of %select{%3|unsigned expression}0 %2 "
   "%select{unsigned expression|%3}0 is always %4">,
   InGroup<TautologicalUnsignedZeroCompare>, DefaultIgnore;
+def warn_unsigned_char_always_true_comparison : Warning<
+  "result of comparison of %select{%3|char expression}0 %2 "
+  "%select{char expression|%3}0 is always %4, since char is interpreted as "
+  "unsigned">, InGroup<TautologicalUnsignedCharZeroCompare>, DefaultIgnore;
 def warn_unsigned_enum_always_true_comparison : Warning<
   "result of comparison of %select{%3|unsigned enum expression}0 %2 "
   "%select{unsigned enum expression|%3}0 is always %4">,

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index fa5e184d2864..7b6e0541aa4d 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -11446,11 +11446,14 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E,
             << OtherIsBooleanDespiteType << *Result
             << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange());
   } else {
-    unsigned Diag = (isKnownToHaveUnsignedValue(OriginalOther) && Value == 0)
-                        ? (HasEnumType(OriginalOther)
-                               ? diag::warn_unsigned_enum_always_true_comparison
-                               : diag::warn_unsigned_always_true_comparison)
-                        : diag::warn_tautological_constant_compare;
+    bool IsCharTy = OtherT.withoutLocalFastQualifiers() == S.Context.CharTy;
+    unsigned Diag =
+        (isKnownToHaveUnsignedValue(OriginalOther) && Value == 0)
+            ? (HasEnumType(OriginalOther)
+                   ? diag::warn_unsigned_enum_always_true_comparison
+                   : IsCharTy ? diag::warn_unsigned_char_always_true_comparison
+                              : diag::warn_unsigned_always_true_comparison)
+            : diag::warn_tautological_constant_compare;
 
     S.Diag(E->getOperatorLoc(), Diag)
         << RhsConstant << OtherT << E->getOpcodeStr() << OS.str() << *Result

diff  --git a/clang/test/Sema/tautological-unsigned-char-zero-compare.cc b/clang/test/Sema/tautological-unsigned-char-zero-compare.cc
new file mode 100644
index 000000000000..4d14954b3213
--- /dev/null
+++ b/clang/test/Sema/tautological-unsigned-char-zero-compare.cc
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -fsyntax-only \
+// RUN:            -fno-signed-char \
+// RUN:            -Wtautological-unsigned-zero-compare \
+// RUN:            -Wtautological-unsigned-char-zero-compare \
+// RUN:            -verify=unsigned %s
+// RUN: %clang_cc1 -fsyntax-only \
+// RUN:            -Wtautological-unsigned-zero-compare \
+// RUN:            -Wtautological-unsigned-char-zero-compare \
+// RUN:            -verify=signed %s
+
+void f(char c, unsigned char uc, signed char cc) {
+  if (c < 0)
+    return;
+  // unsigned-warning at -2 {{comparison of char expression < 0 is always false, since char is interpreted as unsigned}}
+  if (uc < 0)
+    return;
+  // unsigned-warning at -2 {{comparison of unsigned expression < 0 is always false}}
+  // signed-warning at -3 {{comparison of unsigned expression < 0 is always false}}
+  if (cc < 0)
+    return;
+  // Promoted to integer expressions should not warn.
+  if (c - 4 < 0)
+    return;
+}
+
+void ref(char &c, unsigned char &uc, signed char &cc) {
+  if (c < 0)
+    return;
+  // unsigned-warning at -2 {{comparison of char expression < 0 is always false, since char is interpreted as unsigned}}
+  if (uc < 0)
+    return;
+  // unsigned-warning at -2 {{comparison of unsigned expression < 0 is always false}}
+  // signed-warning at -3 {{comparison of unsigned expression < 0 is always false}}
+  if (cc < 0)
+    return;
+  // Promoted to integer expressions should not warn.
+  if (c - 4 < 0)
+    return;
+}