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

[AntonBikineev]

Summary

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.

[AntonBikineev]

friendly ping..

[AntonBikineev]

Fix formatting

[thakis]

Seems harmless, so lg :)

clang/include/clang/Basic/DiagnosticGroups.td

 def StringCompare : DiagGroup<"string-compare">;
 def StringPlusInt : DiagGroup<"string-plus-int">;
 def StringPlusChar : DiagGroup<"string-plus-char">;
 def StrncatSize : DiagGroup<"strncat-size">;
 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.
 def TautologicalValueRangeCompare : DiagGroup<"tautological-value-range-compare">;
 def TautologicalInRangeCompare : DiagGroup<"tautological-constant-in-range-compare",
                                            [TypeLimits, TautologicalValueRangeCompare]>;
 def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">;
 def TautologicalConstantCompare : DiagGroup<"tautological-constant-compare",

clang/include/clang/Basic/DiagnosticSemaKinds.td

def note_typecheck_assign_const : Note<
   "%select{non-|}1static data member %2 declared const here|"
   "member function %q1 is declared const here|"
   "%select{|nested }1data member %2 declared const here}0">;
 
 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">,
   InGroup<TautologicalUnsignedEnumZeroCompare>, DefaultIgnore;
 def warn_tautological_constant_compare : Warning<
   "result of comparison %select{%3|%1}0 %2 "
   "%select{%1|%3}0 is always %4">,
   InGroup<TautologicalTypeLimitCompare>, DefaultIgnore;

clang/lib/Sema/SemaChecking.cpp

if (!InRange || Other->isKnownToHaveBooleanValue()) {
     S.DiagRuntimeBehavior(
         E->getOperatorLoc(), E,
         S.PDiag(!InRange ? diag::warn_out_of_range_compare
                          : diag::warn_tautological_bool_compare)
             << OS.str() << classifyConstantValue(Constant) << OtherT
             << 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)
+               : IsCharTy ? diag::warn_unsigned_char_always_true_comparison
 
     S.Diag(E->getOperatorLoc(), Diag)
         << RhsConstant << OtherT << E->getOpcodeStr() << OS.str() << *Result
         << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
   }
 
   return true;
 }

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

+// 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@-2 {{comparison of char expression < 0 is always false, since char is interpreted as unsigned}}
+  if (uc < 0)
+    return;
+  // unsigned-warning@-2 {{comparison of unsigned expression < 0 is always false}}
+  // signed-warning@-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@-2 {{comparison of char expression < 0 is always false, since char is interpreted as unsigned}}
+  if (uc < 0)
+    return;
+  // unsigned-warning@-2 {{comparison of unsigned expression < 0 is always false}}
+  // signed-warning@-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;
+}