[Sema] Diagnose tautological comparison with type's min/max values

[lebedev.ri]

Summary

Currently, clang only diagnoses completely out-of-range comparisons (e.g. char and constant 300),
and comparisons of unsigned and 0. But gcc also does diagnose the comparisons with the
std::numeric_limits<>::max() / std::numeric_limits<>::min() so to speak

Finally Fixes https://bugs.llvm.org/show_bug.cgi?id=34147
Continuation of https://reviews.llvm.org/D37565

[lebedev.ri]

Tried stage2 build, so far only one warning.
But found out that ubsan_value.h's typedef s128 SIntMax; crashes this code because

class LLVM_NODISCARD APSInt : public APInt {
...
  /// \brief Get the correctly-extended \c int64_t value.
  int64_t getExtValue() const {
    assert(getMinSignedBits() <= 64 && "Too many bits for int64_t");
    return isSigned() ? getSExtValue() : getZExtValue();
  }

[lebedev.ri]

• Fixed handling of types > 64 bit (e.g. __int128), testcase added
• Stage-2 build now fully passes (only one new warning, D38132)
• Updated ReleaseNotes.rst

[lebedev.ri]

Rebased, ping.

Vanilla stage-2 build is now warning-clean, the only previous warning was fixed.

[lebedev.ri]

Ping :)
Does this need more, different tests?
Should i rewrite that lengthy if-else-if chain somehow differently?

[rsmith]

lib/Sema/SemaChecking.cpp

8583–8586 ↗

(On Diff #116857)

Use /// for documentation comments.

8586 ↗

(On Diff #116857)

Is there a reason to separate the checks for 0 from the other checks for min/max values? It looks like the code below would be slightly simpler if the two checks were combined.

8588 ↗

(On Diff #116857)

Value can't be null; pass it by reference instead.

8636–8637 ↗

(On Diff #116857)

This comment adds nothing to the following code; remove?

8720–8768 ↗

(On Diff #116857)

Please try to reduce/factor out the duplication here.

8796 ↗

(On Diff #116857)

Comments should start with a capital letter.

[lebedev.ri]

@rsmith thank you for the review!

Address review notes:

1. Merge CheckTautologicalComparisonWithZero() and CheckTautologicalComparisonWithMinMax()
2. Significantly deduplicate the code, hopefully not at the cost of readability.
3. Slightly cleanup/move around/update relevant tests for these three types of tautological comparisons.

[lebedev.ri]

lib/Sema/SemaChecking.cpp

8697 ↗

(On Diff #117710)

If the diagnostic we are about to output is disabled, should we still return true; and suppress potential -Wsign-compare warning?

[rsmith]

Thanks for the refactoring :)

lib/Sema/SemaChecking.cpp

8697 ↗

(On Diff #117710)

The general principle is to behave as if we produced all diagnostics, and then filtered out the ones that aren't enabled. So if a more specialized (eg tautological comparison) diagnostic is disabled, the more general (eg sign compare) diagnostic should not appear. In short, this is fine :)

8719 ↗

(On Diff #117710)

Is this necessary? (Aren't the type limit values always within the range of the type in question?)

Can we avoid evaluating Constant a extra time here? (We already have its value in Value.)

[lebedev.ri]

• Address review notes
• Avoid re-evaluating ICE when called from DiagnoseOutOfRangeComparison() which already knows the evaluated value
  
  Don't even check if this is a Type Limit in DiagnoseOutOfRangeComparison(). Perhaps @rtrieu can shed some light on this.
• Enhance test coverage a little (add a test with macro, with template)

lib/Sema/SemaChecking.cpp

8719 ↗

(On Diff #117710)

Uhm, good question :)
If i remove this, check-clang-sema and check-clang-semacxx still pass.
I agree that it does not make much sense. Initially it was only checking for Value == 0.
Git suggests that initially this branch was added by @rtrieu, maybe can help.

[lebedev.ri]

lib/Sema/SemaChecking.cpp

8719 ↗

(On Diff #117710)

The most original version of this code
After some though i think the initial check Value == 0 was simply to quickly bail out
out of DiagnoseOutOfRangeComparison(), and not waste any time for the obvious case
(0), which can't be out-of-range, ever. So i think the right behaviour could be:

1. Either simply restore the original check:

// 0 values are handled later by CheckTautologicalComparison().
if ((Value == 0) && (!OtherIsBooleanType))
  return;

And add a comment there about it

2. Or, drop it completely
3. Or, perhaps refactor CheckTautologicalComparison(), and more or less call it from function AnalyzeComparison(), before calling DiagnoseOutOfRangeComparison(), thus completely avoiding the need to re-evaluate Constant there later on, and simplify the logic in the process.

I personally think the 3. *might* be best.
WDYT?

[lebedev.ri]

lib/Sema/SemaChecking.cpp

8719 ↗

(On Diff #117710)

Tried implementing 3..
It won't work, because DiagnoseOutOfRangeComparison() needs the {L,R}HS
after IgnoreParenImpCasts(), and CheckTautologicalComparison() is not ok with that.
It seems that at most, i could re-use the detection of RhsConstant.

So, new options:

1. Either simply restore the original check, and add a comment there about the logic behind it
2. Or, drop the check completely
3. Or, move the CheckTautologicalComparison() call before DiagnoseOutOfRangeComparison() And if DiagnoseOutOfRangeComparison() has already emitted diagnostic, return. Much like what CheckTautologicalComparison() already does.

So i think 3. is still the best option :)
(tried implementing it, appears to work)

[lebedev.ri]

Rework AnalyzeComparison() and DiagnoseOutOfRangeComparison() - avoid that fast-return path in DiagnoseOutOfRangeComparison()
AnalyzeComparison() is now a bit smarter, and it

1. calls CheckTautologicalComparison()
2. returns if it diagnosed anything
3. else, calls DiagnoseOutOfRangeComparison()
4. returns if it diagnosed anything.

check-clang-sema and check-clang-semacxx are good.
Stage-2 build still good.

[rsmith]

lib/Sema/SemaChecking.cpp

8667–8679 ↗

(On Diff #118143)

The exhaustive case analysis is a little hard to verify. Perhaps something like this would be clearer:

bool ConstIsLowerBound = (Op == BO_LT || Op == BO_LE) ^ ConstOnRight;
bool ResultWhenConstEqualsOther = (Op == BO_LE || Op == BO_GE);
bool ResultWhenConstNeOther = ConstIsLowerBound ^ ValueType == LimitType::Max;
if (ResultWhenConstEqualsOther != ResultWhenConstNeOther)
  return false;

8940 ↗

(On Diff #118143)

It seems suboptimal to evaluate both sides of the comparison and then evaluate the entire comparison again in this case. Presumably the point is to catch comparisons whose operands are not of integral type (eg, floating point), but we could get that benefit and also catch more integral cases by switching from isIntegerConstantExpr to the more appropriate EvaluateAsRValue.

I'm fine with a cleanup to avoid the repeated evaluation here being deferred to a later patch.

8949 ↗

(On Diff #118143)

Pass LHSValue and RHSValue in here rather than recomputing them.

test/Sema/tautological-constant-compare.c

1–4 ↗

(On Diff #118143)

This test makes assumptions about the valid ranges of certain built-in types, so should specify a target triple. (Eg, -triple x86_64-linux-gnu)

[lebedev.ri]

@rsmith, thank you for the review!

Rebased, address review notes, simplified all the things even further.

Testing:

• $ ninja check-clang-sema check-clang-semacxx
• Stage 2 build (still good, no issues/warnings)

lib/Sema/SemaChecking.cpp

8667–8679 ↗

(On Diff #118143)

Oh, that looks better indeed :)
I did consider doing something like that, but my variant ended up looking worse than even the current if()'s

8940 ↗

(On Diff #118143)

If we look at this code closely, if hasUnsignedIntegerRepresentation() == false, we always do return AnalyzeImpConvsInComparison(S, E);, regardless of E->isIntegerConstantExpr(S.Context).
So if i move more stuff into if (T->isIntegralType(S.Context)), then E->isIntegerConstantExpr(S.Context); is simply gone.
At least the current tests say so.

8949 ↗

(On Diff #118143)

We can even go one step further, and pass the bool IsConstLiteralOnRHS

[rsmith]

Thanks, this looks great.

lib/Sema/SemaChecking.cpp

8924 ↗

(On Diff #118793)

I don't think this variable pulls its weight any more, especially given the adjacent comment. Inline its initializer into the if condition, maybe?

8930 ↗

(On Diff #118793)

It would probably be more obvious to use || here, since you already returned in the case where both sides are constant.

8940 ↗

(On Diff #118143)

It looks like the old behavior was to suppress the CheckTautologicalComparisonWithZero diagnostics when:

• the comparison has a constant value, and
• the types being compared are not integral types, and
• the types being compared do not have an unsigned integer representation

However, CheckTautologicalComparisonWithZero only emits diagnostics for comparisons with integral types. So I think you're right that the old codepath that evaluated E->isIntegerConstantExpr(S.Context) was pointless.

[lebedev.ri]

lib/Sema/SemaChecking.cpp

8930 ↗

(On Diff #118793)

And what if neither of them is constant?
I *think* we could reach this point in that case.
At least it is not immediately obvious to me why it can't happen.

[lebedev.ri]

Getting really weird problems when trying to use arc patch, maybe re-updating the differential helps

[lebedev.ri]

Reverted due to http://bb9.pgr.jp/#/builders/20/builds/59 that i don't currently know how to deal with.
It is really sad that i failed to encounter it during testing.

[lebedev.ri]

cfe/trunk/test/Sema/tautological-constant-compare.c

24

I'm having second thoughts about macro handling.
Right now we completely ignore the comparisons when the constant is anyhow involved with macros.
I.e.

unsigned char c;
if (c > 255) // expected-warning {{comparison 'unsigned char' > 255 is always false}}
  return;  // would be diagnosed correctly
// but
assert (c <= 255); // would be completely ignored.

Perhaps we should be a bit more strict, and should not bailout in such cases?

But i *guess* this case we still should ignore

#define macro(val) (val > 255)
if (macro(c))
  return;

(Even though gcc warns in the last variant too)

[lebedev.ri]

Forgot to add, i really noticed/though about it just now, in https://reviews.llvm.org/D38871, because i did not encountered any warnings in that code in stage-2 builds.

[materi]

Hi!

After this patch I started to see warnings:

e.c:8:23: warning: integer constant not in range of enumerated type 'enum E' [-Wassign-enum]
  enum E {a = 7,} e = 1000;
                      ^
e.c:10:12: warning: comparison 'enum E' > 7 is always false [-Wtautological-constant-compare]
  return e > 7;

Isn't the the "always false" message misleading? It's only "always false" if e was initialized with an in-range value. Maybe the tautology check should be on the enum's underlying type instead?

[lebedev.ri]

In D38101#901709, @materi wrote:

Hi!

Hi.

After this patch I started to see warnings:

Thank you for the report!

e.c:8:23: warning: integer constant not in range of enumerated type 'enum E' [-Wassign-enum]
  enum E {a = 7,} e = 1000;
                      ^
e.c:10:12: warning: comparison 'enum E' > 7 is always false [-Wtautological-constant-compare]
  return e > 7;

Isn't the the "always false" message misleading? It's only "always false" if e was initialized with an in-range value. Maybe the tautology check should be on the enum's underlying type instead?

I agree, please open a bug, i'll look into this.

[materi]

In D38101#901733, @lebedev.ri wrote:

In D38101#901709, @materi wrote:

Hi!

Hi.

After this patch I started to see warnings:

Thank you for the report!

e.c:8:23: warning: integer constant not in range of enumerated type 'enum E' [-Wassign-enum]
  enum E {a = 7,} e = 1000;
                      ^
e.c:10:12: warning: comparison 'enum E' > 7 is always false [-Wtautological-constant-compare]
  return e > 7;

Isn't the the "always false" message misleading? It's only "always false" if e was initialized with an in-range value. Maybe the tautology check should be on the enum's underlying type instead?

I agree, please open a bug, i'll look into this.

Here it is:
https://bugs.llvm.org/show_bug.cgi?id=35009

Thanks!

cfe/trunk/docs/ReleaseNotes.rst

 
 - ``-Wtautological-compare`` now warns when comparing an unsigned integer and 0
   regardless of whether the constant is signed or unsigned."
 
 - ``-Wtautological-compare`` now warns about comparing a signed integer and 0
   when the signed integer is coerced to an unsigned type for the comparison.
   ``-Wsign-compare`` was adjusted not to warn in this case.
 
+- ``-Wtautological-constant-compare`` is a new warning that warns on
+  tautological comparisons between integer variable of the type ``T`` and the
+  largest/smallest possible integer constant of that same type.
+
 - ``-Wnull-pointer-arithmetic`` now warns about performing pointer arithmetic
   on a null pointer. Such pointer arithmetic has an undefined behavior if the
   offset is nonzero. It also now warns about arithmetic on a null pointer
   treated as a cast from integer to pointer (GNU extension).
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------
 

cfe/trunk/include/clang/Basic/DiagnosticGroups.td

 def GNUStatementExpression : DiagGroup<"gnu-statement-expression">;
 def StringCompare : DiagGroup<"string-compare">;
 def StringPlusInt : DiagGroup<"string-plus-int">;
 def StringPlusChar : DiagGroup<"string-plus-char">;
 def StrncatSize : DiagGroup<"strncat-size">;
 def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">;
 def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">;
 def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">;
+def TautologicalConstantCompare : DiagGroup<"tautological-constant-compare",
+                                            [TautologicalUnsignedZeroCompare,
+                                             TautologicalUnsignedEnumZeroCompare,
+                                             TautologicalOutOfRangeCompare]>;
 def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">;
 def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">;
 def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">;
 def TautologicalCompare : DiagGroup<"tautological-compare",
-                                    [TautologicalUnsignedZeroCompare,
-                                     TautologicalUnsignedEnumZeroCompare,
-                                     TautologicalOutOfRangeCompare,
+                                    [TautologicalConstantCompare,
                                      TautologicalPointerCompare,
                                      TautologicalOverlapCompare,
                                      TautologicalUndefinedCompare]>;
 def HeaderHygiene : DiagGroup<"header-hygiene">;
 def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">;
 def CompareDistinctPointerType : DiagGroup<"compare-distinct-pointer-types">;
 def GNUUnionCast : DiagGroup<"gnu-union-cast">;
 def GNUVariableSizedTypeNotAtEnd : DiagGroup<"gnu-variable-sized-type-not-at-end">;

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

 def note_typecheck_assign_const : Note<
   "%select{"
   "function %1 which returns const-qualified type %2 declared here|"
   "variable %1 declared const here|"
   "%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_lunsigned_always_true_comparison : Warning<
-  "comparison of unsigned expression %0 is always %select{false|true}1">,
+def warn_unsigned_always_true_comparison : Warning<
+  "comparison of %select{%3|unsigned expression}0 %2 "
+  "%select{unsigned expression|%3}0 is always %select{false|true}4">,
   InGroup<TautologicalUnsignedZeroCompare>;
-def warn_runsigned_always_true_comparison : Warning<
-  "comparison of %0 unsigned expression is always %select{false|true}1">,
-  InGroup<TautologicalUnsignedZeroCompare>;
-def warn_lunsigned_enum_always_true_comparison : Warning<
-  "comparison of unsigned enum expression %0 is always %select{false|true}1">,
-  InGroup<TautologicalUnsignedEnumZeroCompare>;
-def warn_runsigned_enum_always_true_comparison : Warning<
-  "comparison of %0 unsigned enum expression is always %select{false|true}1">,
+def warn_unsigned_enum_always_true_comparison : Warning<
+  "comparison of %select{%3|unsigned enum expression}0 %2 "
+  "%select{unsigned enum expression|%3}0 is always %select{false|true}4">,
   InGroup<TautologicalUnsignedEnumZeroCompare>;
+def warn_tautological_constant_compare : Warning<
+  "comparison %select{%3|%1}0 %2 "
+  "%select{%1|%3}0 is always %select{false|true}4">,
+  InGroup<TautologicalConstantCompare>;
 
 def warn_mixed_sign_comparison : Warning<
   "comparison of integers of different signs: %0 and %1">,
   InGroup<SignCompare>, DefaultIgnore;
 def warn_out_of_range_compare : Warning<
   "comparison of %select{constant %0|true|false}1 with " 
   "%select{expression of type %2|boolean expression}3 is always "
   "%select{false|true}4">, InGroup<TautologicalOutOfRangeCompare>;

cfe/trunk/lib/Sema/SemaChecking.cpp

bool IsSameFloatAfterCast(const APValue &value,
 
   assert(value.isComplexFloat());
   return (IsSameFloatAfterCast(value.getComplexFloatReal(), Src, Tgt) &&
           IsSameFloatAfterCast(value.getComplexFloatImag(), Src, Tgt));
 }
 
 void AnalyzeImplicitConversions(Sema &S, Expr *E, SourceLocation CC);
 
-bool IsZero(Sema &S, Expr *E) {
+bool IsEnumConstOrFromMacro(Sema &S, Expr *E) {
   // Suppress cases where we are comparing against an enum constant.
   if (const DeclRefExpr *DR =
       dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts()))
     if (isa<EnumConstantDecl>(DR->getDecl()))
-      return false;
+      return true;
 
   // Suppress cases where the '0' value is expanded from a macro.
   if (E->getLocStart().isMacroID())
-    return false;
+    return true;
+
+}
 
-  llvm::APSInt Value;
-  return E->isIntegerConstantExpr(Value, S.Context) && Value == 0;
+bool isNonBooleanIntegerValue(Expr *E) {
+  return !E->isKnownToHaveBooleanValue() && E->getType()->isIntegerType();
+}
+
+bool isNonBooleanUnsignedValue(Expr *E) {
+  // We are checking that the expression is not known to have boolean value,
+  // is an integer type; and is either unsigned after implicit casts,
+  // or was unsigned before implicit casts.
+  return isNonBooleanIntegerValue(E) &&
+         (!E->getType()->isSignedIntegerType() ||
+          !E->IgnoreParenImpCasts()->getType()->isSignedIntegerType());
+}
+
+enum class LimitType {
+  Max, // e.g. 32767 for short
+  Min  // e.g. -32768 for short
+};
+
+/// Checks whether Expr 'Constant' may be the
+/// std::numeric_limits<>::max() or std::numeric_limits<>::min()
+/// of the Expr 'Other'. If true, then returns the limit type (min or max).
+/// The Value is the evaluation of Constant
+llvm::Optional<LimitType> IsTypeLimit(Sema &S, Expr *Constant, Expr *Other,
+                                      const llvm::APSInt &Value) {
+  if (IsEnumConstOrFromMacro(S, Constant))
+    return llvm::Optional<LimitType>();
+
+  if (isNonBooleanUnsignedValue(Other) && Value == 0)
+    return LimitType::Min;
+
+  // TODO: Investigate using GetExprRange() to get tighter bounds
+  // on the bit ranges.
+  QualType OtherT = Other->IgnoreParenImpCasts()->getType();
+  if (const auto *AT = OtherT->getAs<AtomicType>())
+    OtherT = AT->getValueType();
+
+  IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
+
+  if (llvm::APSInt::isSameValue(
+          llvm::APSInt::getMaxValue(OtherRange.Width,
+                                    OtherT->isUnsignedIntegerType()),
+          Value))
+    return LimitType::Max;
+
+  if (llvm::APSInt::isSameValue(
+          llvm::APSInt::getMinValue(OtherRange.Width,
+                                    OtherT->isUnsignedIntegerType()),
+          Value))
+    return LimitType::Min;
+
+  return llvm::Optional<LimitType>();
 }
 
 bool HasEnumType(Expr *E) {
   // Strip off implicit integral promotions.
   while (ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) {
     if (ICE->getCastKind() != CK_IntegralCast &&
         ICE->getCastKind() != CK_NoOp)
       break;
     E = ICE->getSubExpr();
   }
 
   return E->getType()->isEnumeralType();
 }
 
-bool isNonBooleanUnsignedValue(Expr *E) {
-  // We are checking that the expression is not known to have boolean value,
-  // is an integer type; and is either unsigned after implicit casts,
-  // or was unsigned before implicit casts.
-  return !E->isKnownToHaveBooleanValue() && E->getType()->isIntegerType() &&
-         (!E->getType()->isSignedIntegerType() ||
-          !E->IgnoreParenImpCasts()->getType()->isSignedIntegerType());
-}
-
-bool CheckTautologicalComparisonWithZero(Sema &S, BinaryOperator *E) {
-  // Disable warning in template instantiations.
-  if (S.inTemplateInstantiation())
+bool CheckTautologicalComparison(Sema &S, BinaryOperator *E, Expr *Constant,
+                                 Expr *Other, const llvm::APSInt &Value,
+                                 bool RhsConstant) {
+  // Disable warning in template instantiations
+  // and only analyze <, >, <= and >= operations.
+  if (S.inTemplateInstantiation() || !E->isRelationalOp())
     return false;
 
-  // bool values are handled by DiagnoseOutOfRangeComparison().
-
   BinaryOperatorKind Op = E->getOpcode();
-  if (E->isValueDependent())
+
+  QualType OType = Other->IgnoreParenImpCasts()->getType();
+
+  llvm::Optional<LimitType> ValueType; // Which limit (min/max) is the constant?
+
+  if (!(isNonBooleanIntegerValue(Other) &&
+        (ValueType = IsTypeLimit(S, Constant, Other, Value))))
     return false;
 
-  Expr *LHS = E->getLHS();
-  Expr *RHS = E->getRHS();
+  bool ConstIsLowerBound = (Op == BO_LT || Op == BO_LE) ^ RhsConstant;
+  bool ResultWhenConstEqualsOther = (Op == BO_LE || Op == BO_GE);
+  bool ResultWhenConstNeOther =
+      ConstIsLowerBound ^ (ValueType == LimitType::Max);
+  if (ResultWhenConstEqualsOther != ResultWhenConstNeOther)
+    return false; // The comparison is not tautological.
 
-  bool Match = true;
+  const bool Result = ResultWhenConstEqualsOther;
 
-  if (Op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
-    S.Diag(E->getOperatorLoc(),
-           HasEnumType(LHS) ? diag::warn_lunsigned_enum_always_true_comparison
-                            : diag::warn_lunsigned_always_true_comparison)
-        << "< 0" << false << LHS->getSourceRange() << RHS->getSourceRange();
-  } else if (Op == BO_GE && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
-    S.Diag(E->getOperatorLoc(),
-           HasEnumType(LHS) ? diag::warn_lunsigned_enum_always_true_comparison
-                            : diag::warn_lunsigned_always_true_comparison)
-        << ">= 0" << true << LHS->getSourceRange() << RHS->getSourceRange();
-  } else if (Op == BO_GT && isNonBooleanUnsignedValue(RHS) && IsZero(S, LHS)) {
-    S.Diag(E->getOperatorLoc(),
-           HasEnumType(RHS) ? diag::warn_runsigned_enum_always_true_comparison
-                            : diag::warn_runsigned_always_true_comparison)
-        << "0 >" << false << LHS->getSourceRange() << RHS->getSourceRange();
-  } else if (Op == BO_LE && isNonBooleanUnsignedValue(RHS) && IsZero(S, LHS)) {
-    S.Diag(E->getOperatorLoc(),
-           HasEnumType(RHS) ? diag::warn_runsigned_enum_always_true_comparison
-                            : diag::warn_runsigned_always_true_comparison)
-        << "0 <=" << true << LHS->getSourceRange() << RHS->getSourceRange();
-  } else
-    Match = false;
+  unsigned Diag = (isNonBooleanUnsignedValue(Other) && Value == 0)
+                      ? (HasEnumType(Other)
+                             ? diag::warn_unsigned_enum_always_true_comparison
+                             : diag::warn_unsigned_always_true_comparison)
+                      : diag::warn_tautological_constant_compare;
+
+  // Should be enough for uint128 (39 decimal digits)
+  SmallString<64> PrettySourceValue;
+  llvm::raw_svector_ostream OS(PrettySourceValue);
+  OS << Value;
 
-  return Match;
+  S.Diag(E->getOperatorLoc(), Diag)
+      << RhsConstant << OType << E->getOpcodeStr() << OS.str() << Result
+      << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
+
+  return true;
 }
 
-void DiagnoseOutOfRangeComparison(Sema &S, BinaryOperator *E, Expr *Constant,
+bool DiagnoseOutOfRangeComparison(Sema &S, BinaryOperator *E, Expr *Constant,
                                   Expr *Other, const llvm::APSInt &Value,
                                   bool RhsConstant) {
   // Disable warning in template instantiations.
   if (S.inTemplateInstantiation())
-    return;
+    return false;
+
+  Constant = Constant->IgnoreParenImpCasts();
+  Other = Other->IgnoreParenImpCasts();
 
   // TODO: Investigate using GetExprRange() to get tighter bounds
   // on the bit ranges.
   QualType OtherT = Other->getType();
   if (const auto *AT = OtherT->getAs<AtomicType>())
     OtherT = AT->getValueType();
   IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
   unsigned OtherWidth = OtherRange.Width;
 
   bool OtherIsBooleanType = Other->isKnownToHaveBooleanValue();
 
-  // 0 values are handled later by CheckTautologicalComparisonWithZero().
-  if ((Value == 0) && (!OtherIsBooleanType))
-    return;
-
   BinaryOperatorKind op = E->getOpcode();
   bool IsTrue = true;
 
   // Used for diagnostic printout.
   enum {
     LiteralConstant = 0,
     CXXBoolLiteralTrue,
     CXXBoolLiteralFalse
   } LiteralOrBoolConstant = LiteralConstant;
 
   if (!OtherIsBooleanType) {
     QualType ConstantT = Constant->getType();
     QualType CommonT = E->getLHS()->getType();
 
     if (S.Context.hasSameUnqualifiedType(OtherT, ConstantT))
-      return;
+      return false;
     assert((OtherT->isIntegerType() && ConstantT->isIntegerType()) &&
            "comparison with non-integer type");
 
     bool ConstantSigned = ConstantT->isSignedIntegerType();
     bool CommonSigned = CommonT->isSignedIntegerType();
 
     bool EqualityOnly = false;
 
     if (CommonSigned) {
       // The common type is signed, therefore no signed to unsigned conversion.
       if (!OtherRange.NonNegative) {
         // Check that the constant is representable in type OtherT.
         if (ConstantSigned) {
           if (OtherWidth >= Value.getMinSignedBits())
-            return;
+            return false;
         } else { // !ConstantSigned
           if (OtherWidth >= Value.getActiveBits() + 1)
-            return;
+            return false;
         }
       } else { // !OtherSigned
                // Check that the constant is representable in type OtherT.
         // Negative values are out of range.
         if (ConstantSigned) {
           if (Value.isNonNegative() && OtherWidth >= Value.getActiveBits())
-            return;
+            return false;
         } else { // !ConstantSigned
           if (OtherWidth >= Value.getActiveBits())
-            return;
+            return false;
         }
       }
     } else { // !CommonSigned
       if (OtherRange.NonNegative) {
         if (OtherWidth >= Value.getActiveBits())
-          return;
+          return false;
       } else { // OtherSigned
         assert(!ConstantSigned &&
                "Two signed types converted to unsigned types.");
         // Check to see if the constant is representable in OtherT.
         if (OtherWidth > Value.getActiveBits())
-          return;
+          return false;
         // Check to see if the constant is equivalent to a negative value
         // cast to CommonT.
         if (S.Context.getIntWidth(ConstantT) ==
                 S.Context.getIntWidth(CommonT) &&
             Value.isNegative() && Value.getMinSignedBits() <= OtherWidth)
-          return;
+          return false;
         // The constant value rests between values that OtherT can represent
         // after conversion.  Relational comparison still works, but equality
         // comparisons will be tautological.
         EqualityOnly = true;
       }
     }
 
     bool PositiveConstant = !ConstantSigned || Value.isNonNegative();
 
     if (op == BO_EQ || op == BO_NE) {
       IsTrue = op == BO_NE;
     } else if (EqualityOnly) {
-      return;
+      return false;
     } else if (RhsConstant) {
       if (op == BO_GT || op == BO_GE)
         IsTrue = !PositiveConstant;
       else // op == BO_LT || op == BO_LE
         IsTrue = PositiveConstant;
     } else {
       if (op == BO_LT || op == BO_LE)
         IsTrue = !PositiveConstant;
default:
       break;
     }
 
     if (CmpRes == AFals) {
       IsTrue = false;
     } else if (CmpRes == ATrue) {
       IsTrue = true;
     } else {
-      return;
+      return false;
     }
   }
 
   // If this is a comparison to an enum constant, include that
   // constant in the diagnostic.
   const EnumConstantDecl *ED = nullptr;
   if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(Constant))
     ED = dyn_cast<EnumConstantDecl>(DR->getDecl());
 
   SmallString<64> PrettySourceValue;
   llvm::raw_svector_ostream OS(PrettySourceValue);
   if (ED)
     OS << '\'' << *ED << "' (" << Value << ")";
   else
     OS << Value;
 
   S.DiagRuntimeBehavior(
     E->getOperatorLoc(), E,
     S.PDiag(diag::warn_out_of_range_compare)
         << OS.str() << LiteralOrBoolConstant
         << OtherT << (OtherIsBooleanType && !OtherT->isBooleanType()) << IsTrue
         << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange());
+
+   return true;
 }
 
 /// Analyze the operands of the given comparison.  Implements the
 /// fallback case from AnalyzeComparison.
 void AnalyzeImpConvsInComparison(Sema &S, BinaryOperator *E) {
   AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
   AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
 }
void AnalyzeComparison(Sema &S, BinaryOperator *E) {
   // the same type.
   if (!S.Context.hasSameUnqualifiedType(T, E->getRHS()->getType()))
     return AnalyzeImpConvsInComparison(S, E);
 
   // Don't analyze value-dependent comparisons directly.
   if (E->isValueDependent())
     return AnalyzeImpConvsInComparison(S, E);
 
-  Expr *LHS = E->getLHS()->IgnoreParenImpCasts();
-  Expr *RHS = E->getRHS()->IgnoreParenImpCasts();
-  
-  bool IsComparisonConstant = false;
-  
-  // Check whether an integer constant comparison results in a value
-  // of 'true' or 'false'.
+  Expr *LHS = E->getLHS();
+  Expr *RHS = E->getRHS();
   if (T->isIntegralType(S.Context)) {
     llvm::APSInt RHSValue;
-    bool IsRHSIntegralLiteral = 
-      RHS->isIntegerConstantExpr(RHSValue, S.Context);
     llvm::APSInt LHSValue;
-    bool IsLHSIntegralLiteral = 
-      LHS->isIntegerConstantExpr(LHSValue, S.Context);
-    if (IsRHSIntegralLiteral && !IsLHSIntegralLiteral)
-        DiagnoseOutOfRangeComparison(S, E, RHS, LHS, RHSValue, true);
-    else if (!IsRHSIntegralLiteral && IsLHSIntegralLiteral)
-      DiagnoseOutOfRangeComparison(S, E, LHS, RHS, LHSValue, false);
-    else
-      IsComparisonConstant = 
-        (IsRHSIntegralLiteral && IsLHSIntegralLiteral);
-  } else if (!T->hasUnsignedIntegerRepresentation())
-      IsComparisonConstant = E->isIntegerConstantExpr(S.Context);
-
-  // We don't care about value-dependent expressions or expressions
-  // whose result is a constant.
-  if (IsComparisonConstant)
+
+    bool IsRHSIntegralLiteral = RHS->isIntegerConstantExpr(RHSValue, S.Context);
+    bool IsLHSIntegralLiteral = LHS->isIntegerConstantExpr(LHSValue, S.Context);
+
+    // We don't care about expressions whose result is a constant.
+    if (IsRHSIntegralLiteral && IsLHSIntegralLiteral)
+      return AnalyzeImpConvsInComparison(S, E);
+
+    // We only care about expressions where just one side is literal
+    if (IsRHSIntegralLiteral ^ IsLHSIntegralLiteral) {
+      // Is the constant on the RHS or LHS?
+      const bool RhsConstant = IsRHSIntegralLiteral;
+      Expr *Const = RhsConstant ? RHS : LHS;
+      Expr *Other = RhsConstant ? LHS : RHS;
+      const llvm::APSInt &Value = RhsConstant ? RHSValue : LHSValue;
+
+      // Check whether an integer constant comparison results in a value
+      // of 'true' or 'false'.
+
+      if (CheckTautologicalComparison(S, E, Const, Other, Value, RhsConstant))
         return AnalyzeImpConvsInComparison(S, E);
 
-  // If this is a tautological comparison, suppress -Wsign-compare.
-  if (CheckTautologicalComparisonWithZero(S, E))
+      if (DiagnoseOutOfRangeComparison(S, E, Const, Other, Value, RhsConstant))
         return AnalyzeImpConvsInComparison(S, E);
+    }
+  }
 
+  if (!T->hasUnsignedIntegerRepresentation()) {
     // We don't do anything special if this isn't an unsigned integral
     // comparison:  we're only interested in integral comparisons, and
     // signed comparisons only happen in cases we don't care to warn about.
-  if (!T->hasUnsignedIntegerRepresentation())
     return AnalyzeImpConvsInComparison(S, E);
+  }
+
+  LHS = LHS->IgnoreParenImpCasts();
+  RHS = RHS->IgnoreParenImpCasts();
 
   // Check to see if one of the (unmodified) operands is of different
   // signedness.
   Expr *signedOperand, *unsignedOperand;
   if (LHS->getType()->hasSignedIntegerRepresentation()) {
     assert(!RHS->getType()->hasSignedIntegerRepresentation() &&
            "unsigned comparison between two signed integer expressions?");
     signedOperand = LHS;

cfe/trunk/test/Analysis/conversion.c

-// RUN: %clang_analyze_cc1 -Wno-conversion -analyzer-checker=core,alpha.core.Conversion -verify %s
+// RUN: %clang_analyze_cc1 -Wno-conversion -Wno-tautological-constant-compare -analyzer-checker=core,alpha.core.Conversion -verify %s
 
 unsigned char U8;
 signed char S8;
 
 void assign(unsigned U, signed S) {
   if (S < -10)
     U8 = S; // expected-warning {{Loss of sign in implicit conversion}}
   if (U > 300)

cfe/trunk/test/Analysis/null-deref-ps.c

-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=core,deadcode,alpha.core -std=gnu99 -analyzer-store=region -analyzer-purge=none -verify %s -Wno-error=return-type
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=core,deadcode,alpha.core -std=gnu99 -analyzer-store=region -verify %s -Wno-error=return-type
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -Wno-tautological-constant-compare -Wtautological-unsigned-zero-compare -analyzer-checker=core,deadcode,alpha.core -std=gnu99 -analyzer-store=region -analyzer-purge=none -verify %s -Wno-error=return-type
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -Wno-tautological-constant-compare -Wtautological-unsigned-zero-compare -analyzer-checker=core,deadcode,alpha.core -std=gnu99 -analyzer-store=region -verify %s -Wno-error=return-type
 
 typedef unsigned uintptr_t;
 
 extern void __assert_fail (__const char *__assertion, __const char *__file,
     unsigned int __line, __const char *__function)
      __attribute__ ((__noreturn__));
 
 #define assert(expr) \

cfe/trunk/test/Sema/outof-range-constant-compare.c

 // RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -Wtautological-constant-out-of-range-compare -verify %s 
 // rdar://12202422
 
 int value(void);
 
 int main()
 {
     int a = value();
 
-    if (a == 0x0000000000000000L)
-        return 0;
-    if (a != 0x0000000000000000L)
-        return 0;
-    if (a < 0x0000000000000000L)
-        return 0;
-    if (a <= 0x0000000000000000L)
-        return 0;
-    if (a > 0x0000000000000000L)
-        return 0;
-    if (a >= 0x0000000000000000L)
-        return 0;
-
-    if (0x0000000000000000L == a)
-        return 0;
-    if (0x0000000000000000L != a)
-        return 0;
-    if (0x0000000000000000L < a)
-        return 0;
-    if (0x0000000000000000L <= a)
-        return 0;
-    if (0x0000000000000000L > a)
-        return 0;
-    if (0x0000000000000000L >= a)
-        return 0;
-
-    if (a == 0x0000000000000000UL)
-        return 0;
-    if (a != 0x0000000000000000UL)
-        return 0;
-    if (a < 0x0000000000000000UL) // expected-warning {{comparison of unsigned expression < 0 is always false}}
-        return 0;
-    if (a <= 0x0000000000000000UL)
-        return 0;
-    if (a > 0x0000000000000000UL)
-        return 0;
-    if (a >= 0x0000000000000000UL) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
-        return 0;
-
-    if (0x0000000000000000UL == a)
-        return 0;
-    if (0x0000000000000000UL != a)
-        return 0;
-    if (0x0000000000000000UL < a)
-        return 0;
-    if (0x0000000000000000UL <= a) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
-        return 0;
-    if (0x0000000000000000UL > a) // expected-warning {{comparison of 0 > unsigned expression is always false}}
-        return 0;
-    if (0x0000000000000000UL >= a)
-        return 0;
-
     if (a == 0x1234567812345678L) // expected-warning {{comparison of constant 1311768465173141112 with expression of type 'int' is always false}}
         return 0;
     if (a != 0x1234567812345678L) // expected-warning {{comparison of constant 1311768465173141112 with expression of type 'int' is always true}}
         return 0;
     if (a < 0x1234567812345678L)  // expected-warning {{comparison of constant 1311768465173141112 with expression of type 'int' is always true}}
         return 0;
     if (a <= 0x1234567812345678L)  // expected-warning {{comparison of constant 1311768465173141112 with expression of type 'int' is always true}}
         return 0;
if (0x1234567812345678L < l)
         return 0;
     if (0x1234567812345678L <= l)
         return 0;
     if (0x1234567812345678L > l)
         return 0;
     if (0x1234567812345678L >= l)
         return 0;
 
-    unsigned un = 0;
-    if (un == 0x0000000000000000L)
-        return 0;
-    if (un != 0x0000000000000000L)
-        return 0;
-    if (un < 0x0000000000000000L) // expected-warning {{comparison of unsigned expression < 0 is always false}}
-        return 0;
-    if (un <= 0x0000000000000000L)
-        return 0;
-    if (un > 0x0000000000000000L)
-        return 0;
-    if (un >= 0x0000000000000000L) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
-        return 0;
-
-    if (0x0000000000000000L == un)
-        return 0;
-    if (0x0000000000000000L != un)
-        return 0;
-    if (0x0000000000000000L < un)
-        return 0;
-    if (0x0000000000000000L <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
-        return 0;
-    if (0x0000000000000000L > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
-        return 0;
-    if (0x0000000000000000L >= un)
-        return 0;
-
-    if (un == 0x0000000000000000UL)
-        return 0;
-    if (un != 0x0000000000000000UL)
-        return 0;
-    if (un < 0x0000000000000000UL) // expected-warning {{comparison of unsigned expression < 0 is always false}}
-        return 0;
-    if (un <= 0x0000000000000000UL)
-        return 0;
-    if (un > 0x0000000000000000UL)
-        return 0;
-    if (un >= 0x0000000000000000UL) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
-        return 0;
-
-    if (0x0000000000000000UL == un)
-        return 0;
-    if (0x0000000000000000UL != un)
-        return 0;
-    if (0x0000000000000000UL < un)
-        return 0;
-    if (0x0000000000000000UL <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
-        return 0;
-    if (0x0000000000000000UL > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
-        return 0;
-    if (0x0000000000000000UL >= un)
-        return 0;
-
-    float fl = 0;
-    if (fl == 0x0000000000000000L)
-        return 0;
-    if (fl != 0x0000000000000000L)
-        return 0;
-    if (fl < 0x0000000000000000L)
-        return 0;
-    if (fl <= 0x0000000000000000L)
-        return 0;
-    if (fl > 0x0000000000000000L)
-        return 0;
-    if (fl >= 0x0000000000000000L)
-        return 0;
-
-    if (0x0000000000000000L == fl)
-        return 0;
-    if (0x0000000000000000L != fl)
-        return 0;
-    if (0x0000000000000000L < fl)
-        return 0;
-    if (0x0000000000000000L <= fl)
-        return 0;
-    if (0x0000000000000000L > fl)
-        return 0;
-    if (0x0000000000000000L >= fl)
-        return 0;
-
-    double dl = 0;
-    if (dl == 0x0000000000000000L)
-        return 0;
-    if (dl != 0x0000000000000000L)
-        return 0;
-    if (dl < 0x0000000000000000L)
-        return 0;
-    if (dl <= 0x0000000000000000L)
-        return 0;
-    if (dl > 0x0000000000000000L)
-        return 0;
-    if (dl >= 0x0000000000000000L)
-        return 0;
-
-    if (0x0000000000000000L == dl)
-        return 0;
-    if (0x0000000000000000L != dl)
-        return 0;
-    if (0x0000000000000000L < dl)
-        return 0;
-    if (0x0000000000000000L <= dl)
-        return 0;
-    if (0x0000000000000000L > dl)
-        return 0;
-    if (0x0000000000000000L >= dl)
-        return 0;
-
     enum E {
     yes,
     no, 
     maybe
     };
     enum E e;
 
     if (e == 0x1234567812345678L) // expected-warning {{comparison of constant 1311768465173141112 with expression of type 'enum E' is always false}}
       return 0;
 
     return 1;
 }

cfe/trunk/test/Sema/tautological-constant-compare.c

+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -DTEST -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wno-tautological-constant-compare -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -DTEST -verify -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wno-tautological-constant-compare -verify -x c++ %s
+
+int value(void);
+
+#define macro(val) val
+
+#ifdef __cplusplus
+template<typename T>
+void TFunc() {
+  // Make sure that we do warn for normal variables in template functions !
+  unsigned char c = value();
+#ifdef TEST
+  if (c > 255) // expected-warning {{comparison 'unsigned char' > 255 is always false}}
+      return;
+#else
+  if (c > 255)
+      return;
+#endif
+
+  if (c > macro(255))
+      return;
+
+  T v = value();
+  if (v > 255)
+      return;
+  if (v > 32767)
+      return;
+}
+#endif
+
+int main()
+{
+#ifdef __cplusplus
+  TFunc<unsigned char>();
+  TFunc<signed short>();
+#endif
+
+  short s = value();
+
+#ifdef TEST
+  if (s == 32767)
+      return 0;
+  if (s != 32767)
+      return 0;
+  if (s < 32767)
+      return 0;
+  if (s <= 32767) // expected-warning {{comparison 'short' <= 32767 is always true}}
+      return 0;
+  if (s > 32767) // expected-warning {{comparison 'short' > 32767 is always false}}
+      return 0;
+  if (s >= 32767)
+      return 0;
+
+  if (32767 == s)
+      return 0;
+  if (32767 != s)
+      return 0;
+  if (32767 < s) // expected-warning {{comparison 32767 < 'short' is always false}}
+      return 0;
+  if (32767 <= s)
+      return 0;
+  if (32767 > s)
+      return 0;
+  if (32767 >= s) // expected-warning {{comparison 32767 >= 'short' is always true}}
+      return 0;
+
+  // FIXME: assumes two's complement
+  if (s == -32768)
+      return 0;
+  if (s != -32768)
+      return 0;
+  if (s < -32768) // expected-warning {{comparison 'short' < -32768 is always false}}
+      return 0;
+  if (s <= -32768)
+      return 0;
+  if (s > -32768)
+      return 0;
+  if (s >= -32768) // expected-warning {{comparison 'short' >= -32768 is always true}}
+      return 0;
+
+  if (-32768 == s)
+      return 0;
+  if (-32768 != s)
+      return 0;
+  if (-32768 < s)
+      return 0;
+  if (-32768 <= s) // expected-warning {{comparison -32768 <= 'short' is always true}}
+      return 0;
+  if (-32768 > s) // expected-warning {{comparison -32768 > 'short' is always false}}
+      return 0;
+  if (-32768 >= s)
+      return 0;
+
+  if (s == 32767UL)
+      return 0;
+  if (s != 32767UL)
+      return 0;
+  if (s < 32767UL)
+      return 0;
+  if (s <= 32767UL) // expected-warning {{comparison 'short' <= 32767 is always true}}
+      return 0;
+  if (s > 32767UL) // expected-warning {{comparison 'short' > 32767 is always false}}
+      return 0;
+  if (s >= 32767UL)
+      return 0;
+
+  if (32767UL == s)
+      return 0;
+  if (32767UL != s)
+      return 0;
+  if (32767UL < s) // expected-warning {{comparison 32767 < 'short' is always false}}
+      return 0;
+  if (32767UL <= s)
+      return 0;
+  if (32767UL > s)
+      return 0;
+  if (32767UL >= s) // expected-warning {{comparison 32767 >= 'short' is always true}}
+      return 0;
+
+  // FIXME: assumes two's complement
+  if (s == -32768L)
+      return 0;
+  if (s != -32768L)
+      return 0;
+  if (s < -32768L) // expected-warning {{comparison 'short' < -32768 is always false}}
+      return 0;
+  if (s <= -32768L)
+      return 0;
+  if (s > -32768L)
+      return 0;
+  if (s >= -32768L) // expected-warning {{comparison 'short' >= -32768 is always true}}
+      return 0;
+
+  if (-32768L == s)
+      return 0;
+  if (-32768L != s)
+      return 0;
+  if (-32768L < s)
+      return 0;
+  if (-32768L <= s) // expected-warning {{comparison -32768 <= 'short' is always true}}
+      return 0;
+  if (-32768L > s) // expected-warning {{comparison -32768 > 'short' is always false}}
+      return 0;
+  if (-32768L >= s)
+      return 0;
+#else
+  // expected-no-diagnostics
+  if (s == 32767)
+    return 0;
+  if (s != 32767)
+    return 0;
+  if (s < 32767)
+    return 0;
+  if (s <= 32767)
+    return 0;
+  if (s > 32767)
+    return 0;
+  if (s >= 32767)
+    return 0;
+
+  if (32767 == s)
+    return 0;
+  if (32767 != s)
+    return 0;
+  if (32767 < s)
+    return 0;
+  if (32767 <= s)
+    return 0;
+  if (32767 > s)
+    return 0;
+  if (32767 >= s)
+    return 0;
+
+  // FIXME: assumes two's complement
+  if (s == -32768)
+    return 0;
+  if (s != -32768)
+    return 0;
+  if (s < -32768)
+    return 0;
+  if (s <= -32768)
+    return 0;
+  if (s > -32768)
+    return 0;
+  if (s >= -32768)
+    return 0;
+
+  if (-32768 == s)
+    return 0;
+  if (-32768 != s)
+    return 0;
+  if (-32768 < s)
+    return 0;
+  if (-32768 <= s)
+    return 0;
+  if (-32768 > s)
+    return 0;
+  if (-32768 >= s)
+    return 0;
+
+  if (s == 32767UL)
+    return 0;
+  if (s != 32767UL)
+    return 0;
+  if (s < 32767UL)
+    return 0;
+  if (s <= 32767UL)
+    return 0;
+  if (s > 32767UL)
+    return 0;
+  if (s >= 32767UL)
+    return 0;
+
+  if (32767UL == s)
+    return 0;
+  if (32767UL != s)
+    return 0;
+  if (32767UL < s)
+    return 0;
+  if (32767UL <= s)
+    return 0;
+  if (32767UL > s)
+    return 0;
+  if (32767UL >= s)
+    return 0;
+
+  // FIXME: assumes two's complement
+  if (s == -32768L)
+    return 0;
+  if (s != -32768L)
+    return 0;
+  if (s < -32768L)
+    return 0;
+  if (s <= -32768L)
+    return 0;
+  if (s > -32768L)
+    return 0;
+  if (s >= -32768L)
+    return 0;
+
+  if (-32768L == s)
+    return 0;
+  if (-32768L != s)
+    return 0;
+  if (-32768L < s)
+    return 0;
+  if (-32768L <= s)
+    return 0;
+  if (-32768L > s)
+    return 0;
+  if (-32768L >= s)
+    return 0;
+#endif
+
+  if (s == 0)
+    return 0;
+  if (s != 0)
+    return 0;
+  if (s < 0)
+    return 0;
+  if (s <= 0)
+    return 0;
+  if (s > 0)
+    return 0;
+  if (s >= 0)
+    return 0;
+
+  if (0 == s)
+    return 0;
+  if (0 != s)
+    return 0;
+  if (0 < s)
+    return 0;
+  if (0 <= s)
+    return 0;
+  if (0 > s)
+    return 0;
+  if (0 >= s)
+    return 0;
+
+  // However the comparison with 0U would warn
+
+  unsigned short us = value();
+
+#ifdef TEST
+  if (us == 65535)
+      return 0;
+  if (us != 65535)
+      return 0;
+  if (us < 65535)
+      return 0;
+  if (us <= 65535) // expected-warning {{comparison 'unsigned short' <= 65535 is always true}}
+      return 0;
+  if (us > 65535) // expected-warning {{comparison 'unsigned short' > 65535 is always false}}
+      return 0;
+  if (us >= 65535)
+      return 0;
+
+  if (65535 == us)
+      return 0;
+  if (65535 != us)
+      return 0;
+  if (65535 < us) // expected-warning {{comparison 65535 < 'unsigned short' is always false}}
+      return 0;
+  if (65535 <= us)
+      return 0;
+  if (65535 > us)
+      return 0;
+  if (65535 >= us) // expected-warning {{comparison 65535 >= 'unsigned short' is always true}}
+      return 0;
+
+  if (us == 65535UL)
+      return 0;
+  if (us != 65535UL)
+      return 0;
+  if (us < 65535UL)
+      return 0;
+  if (us <= 65535UL) // expected-warning {{comparison 'unsigned short' <= 65535 is always true}}
+      return 0;
+  if (us > 65535UL) // expected-warning {{comparison 'unsigned short' > 65535 is always false}}
+      return 0;
+  if (us >= 65535UL)
+      return 0;
+
+  if (65535UL == us)
+      return 0;
+  if (65535UL != us)
+      return 0;
+  if (65535UL < us) // expected-warning {{comparison 65535 < 'unsigned short' is always false}}
+      return 0;
+  if (65535UL <= us)
+      return 0;
+  if (65535UL > us)
+      return 0;
+  if (65535UL >= us) // expected-warning {{comparison 65535 >= 'unsigned short' is always true}}
+      return 0;
+#else
+  // expected-no-diagnostics
+  if (us == 65535)
+      return 0;
+  if (us != 65535)
+      return 0;
+  if (us < 65535)
+      return 0;
+  if (us <= 65535)
+      return 0;
+  if (us > 65535)
+      return 0;
+  if (us >= 65535)
+      return 0;
+
+  if (65535 == us)
+      return 0;
+  if (65535 != us)
+      return 0;
+  if (65535 < us)
+      return 0;
+  if (65535 <= us)
+      return 0;
+  if (65535 > us)
+      return 0;
+  if (65535 >= us)
+      return 0;
+
+  if (us == 65535UL)
+      return 0;
+  if (us != 65535UL)
+      return 0;
+  if (us < 65535UL)
+      return 0;
+  if (us <= 65535UL)
+      return 0;
+  if (us > 65535UL)
+      return 0;
+  if (us >= 65535UL)
+      return 0;
+
+  if (65535UL == us)
+      return 0;
+  if (65535UL != us)
+      return 0;
+  if (65535UL < us)
+      return 0;
+  if (65535UL <= us)
+      return 0;
+  if (65535UL > us)
+      return 0;
+  if (65535UL >= us)
+      return 0;
+#endif
+
+  if (us == 32767)
+    return 0;
+  if (us != 32767)
+    return 0;
+  if (us < 32767)
+    return 0;
+  if (us <= 32767)
+    return 0;
+  if (us > 32767)
+    return 0;
+  if (us >= 32767)
+    return 0;
+
+  if (32767 == us)
+    return 0;
+  if (32767 != us)
+    return 0;
+  if (32767 < us)
+    return 0;
+  if (32767 <= us)
+    return 0;
+  if (32767 > us)
+    return 0;
+  if (32767 >= us)
+    return 0;
+
+  if (us == 32767UL)
+    return 0;
+  if (us != 32767UL)
+    return 0;
+  if (us < 32767UL)
+    return 0;
+  if (us <= 32767UL)
+    return 0;
+  if (us > 32767UL)
+    return 0;
+  if (us >= 32767UL)
+    return 0;
+
+  if (32767UL == us)
+    return 0;
+  if (32767UL != us)
+    return 0;
+  if (32767UL < us)
+    return 0;
+  if (32767UL <= us)
+    return 0;
+  if (32767UL > us)
+    return 0;
+  if (32767UL >= us)
+    return 0;
+
+#if __SIZEOF_INT128__
+  __int128 i128;
+  if (i128 == -1) // used to crash
+      return 0;
+#endif
+
+
+  enum E {
+  yes,
+  no,
+  maybe
+  };
+  enum E e;
+
+  if (e == yes)
+      return 0;
+  if (e != yes)
+      return 0;
+  if (e < yes)
+      return 0;
+  if (e <= yes)
+      return 0;
+  if (e > yes)
+      return 0;
+  if (e >= yes)
+      return 0;
+
+  if (yes == e)
+      return 0;
+  if (yes != e)
+      return 0;
+  if (yes < e)
+      return 0;
+  if (yes <= e)
+      return 0;
+  if (yes > e)
+      return 0;
+  if (yes >= e)
+      return 0;
+
+  if (e == maybe)
+      return 0;
+  if (e != maybe)
+      return 0;
+  if (e < maybe)
+      return 0;
+  if (e <= maybe)
+      return 0;
+  if (e > maybe)
+      return 0;
+  if (e >= maybe)
+      return 0;
+
+  if (maybe == e)
+      return 0;
+  if (maybe != e)
+      return 0;
+  if (maybe < e)
+      return 0;
+  if (maybe <= e)
+      return 0;
+  if (maybe > e)
+      return 0;
+  if (maybe >= e)
+      return 0;
+
+  return 1;
+}

cfe/trunk/test/Sema/tautological-unsigned-zero-compare.c

 // RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s
 // RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DTEST -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify -x c++ %s
 
-unsigned value(void);
+unsigned uvalue(void);
+signed int svalue(void);
 
-int main() {
-  unsigned un = value();
+#define macro(val) val
 
+#ifdef __cplusplus
+template<typename T>
+void TFunc() {
+  // Make sure that we do warn for normal variables in template functions !
+  unsigned char c = svalue();
 #ifdef TEST
+  if (c < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
+      return;
+#else
+  if (c < 0)
+      return;
+#endif
+
+  if (c < macro(0))
+      return;
+
+  T v = svalue();
+  if (v < 0)
+      return;
+}
+#endif
+
+int main()
+{
+#ifdef __cplusplus
+  TFunc<unsigned char>();
+  TFunc<unsigned short>();
+#endif
+
+  unsigned un = uvalue();
+
+#ifdef TEST
+  if (un == 0)
+      return 0;
+  if (un != 0)
+      return 0;
   if (un < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
       return 0;
+  if (un <= 0)
+      return 0;
+  if (un > 0)
+      return 0;
   if (un >= 0) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
       return 0;
+
+  if (0 == un)
+      return 0;
+  if (0 != un)
+      return 0;
+  if (0 < un)
+      return 0;
   if (0 <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
       return 0;
   if (0 > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
       return 0;
-  if (un < 0U) // expected-warning {{comparison of unsigned expression < 0 is always false}}
+  if (0 >= un)
+      return 0;
+
+  if (un == 0UL)
+      return 0;
+  if (un != 0UL)
       return 0;
-  if (un >= 0U) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
+  if (un < 0UL) // expected-warning {{comparison of unsigned expression < 0 is always false}}
       return 0;
-  if (0U <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
+  if (un <= 0UL)
       return 0;
-  if (0U > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
+  if (un > 0UL)
+      return 0;
+  if (un >= 0UL) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
+      return 0;
+
+  if (0UL == un)
+      return 0;
+  if (0UL != un)
+      return 0;
+  if (0UL < un)
+      return 0;
+  if (0UL <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
+      return 0;
+  if (0UL > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
+      return 0;
+  if (0UL >= un)
       return 0;
 #else
 // expected-no-diagnostics
+  if (un == 0)
+      return 0;
+  if (un != 0)
+      return 0;
   if (un < 0)
       return 0;
+  if (un <= 0)
+      return 0;
+  if (un > 0)
+      return 0;
   if (un >= 0)
       return 0;
+
+  if (0 == un)
+      return 0;
+  if (0 != un)
+      return 0;
+  if (0 < un)
+      return 0;
   if (0 <= un)
       return 0;
   if (0 > un)
       return 0;
-  if (un < 0U)
+  if (0 >= un)
+      return 0;
+
+  if (un == 0UL)
+      return 0;
+  if (un != 0UL)
+      return 0;
+  if (un < 0UL)
+      return 0;
+  if (un <= 0UL)
+      return 0;
+  if (un > 0UL)
+      return 0;
+  if (un >= 0UL)
+      return 0;
+
+  if (0UL == un)
+      return 0;
+  if (0UL != un)
+      return 0;
+  if (0UL < un)
+      return 0;
+  if (0UL <= un)
+      return 0;
+  if (0UL > un)
+      return 0;
+  if (0UL >= un)
+      return 0;
+#endif
+
+
+  signed int a = svalue();
+
+#ifdef TEST
+  if (a == 0)
+      return 0;
+  if (a != 0)
+      return 0;
+  if (a < 0)
+      return 0;
+  if (a <= 0)
+      return 0;
+  if (a > 0)
+      return 0;
+  if (a >= 0)
+      return 0;
+
+  if (0 == a)
+      return 0;
+  if (0 != a)
+      return 0;
+  if (0 < a)
+      return 0;
+  if (0 <= a)
+      return 0;
+  if (0 > a)
+      return 0;
+  if (0 >= a)
+      return 0;
+
+  if (a == 0UL)
+      return 0;
+  if (a != 0UL)
+      return 0;
+  if (a < 0UL) // expected-warning {{comparison of unsigned expression < 0 is always false}}
+      return 0;
+  if (a <= 0UL)
+      return 0;
+  if (a > 0UL)
+      return 0;
+  if (a >= 0UL) // expected-warning {{comparison of unsigned expression >= 0 is always true}}
+      return 0;
+
+  if (0UL == a)
+      return 0;
+  if (0UL != a)
+      return 0;
+  if (0UL < a)
+      return 0;
+  if (0UL <= a) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
+      return 0;
+  if (0UL > a) // expected-warning {{comparison of 0 > unsigned expression is always false}}
+      return 0;
+  if (0UL >= a)
+      return 0;
+#else
+// expected-no-diagnostics
+  if (a == 0)
+      return 0;
+  if (a != 0)
+      return 0;
+  if (a < 0)
+      return 0;
+  if (a <= 0)
+      return 0;
+  if (a > 0)
+      return 0;
+  if (a >= 0)
+      return 0;
+
+  if (0 == a)
+      return 0;
+  if (0 != a)
+      return 0;
+  if (0 < a)
       return 0;
-  if (un >= 0U)
+  if (0 <= a)
+      return 0;
+  if (0 > a)
+      return 0;
+  if (0 >= a)
+      return 0;
+
+  if (a == 0UL)
+      return 0;
+  if (a != 0UL)
+      return 0;
+  if (a < 0UL)
+      return 0;
+  if (a <= 0UL)
+      return 0;
+  if (a > 0UL)
+      return 0;
+  if (a >= 0UL)
+      return 0;
+
+  if (0UL == a)
       return 0;
-  if (0U <= un)
+  if (0UL != a)
       return 0;
-  if (0U > un)
+  if (0UL < a)
+      return 0;
+  if (0UL <= a)
+      return 0;
+  if (0UL > a)
+      return 0;
+  if (0UL >= a)
       return 0;
 #endif
 
+
+  float fl = 0;
+
+  if (fl == 0)
+      return 0;
+  if (fl != 0)
+      return 0;
+  if (fl < 0)
+      return 0;
+  if (fl <= 0)
+      return 0;
+  if (fl > 0)
+      return 0;
+  if (fl >= 0)
+      return 0;
+
+  if (0 == fl)
+      return 0;
+  if (0 != fl)
+      return 0;
+  if (0 < fl)
+      return 0;
+  if (0 <= fl)
+      return 0;
+  if (0 > fl)
+      return 0;
+  if (0 >= fl)
+      return 0;
+
+  if (fl == 0UL)
+      return 0;
+  if (fl != 0UL)
+      return 0;
+  if (fl < 0UL)
+      return 0;
+  if (fl <= 0UL)
+      return 0;
+  if (fl > 0UL)
+      return 0;
+  if (fl >= 0UL)
+      return 0;
+
+  if (0UL == fl)
+      return 0;
+  if (0UL != fl)
+      return 0;
+  if (0UL < fl)
+      return 0;
+  if (0UL <= fl)
+      return 0;
+  if (0UL > fl)
+      return 0;
+  if (0UL >= fl)
+      return 0;
+
+
+  double dl = 0;
+
+  if (dl == 0)
+      return 0;
+  if (dl != 0)
+      return 0;
+  if (dl < 0)
+      return 0;
+  if (dl <= 0)
+      return 0;
+  if (dl > 0)
+      return 0;
+  if (dl >= 0)
+      return 0;
+
+  if (0 == dl)
+      return 0;
+  if (0 != dl)
+      return 0;
+  if (0 < dl)
+      return 0;
+  if (0 <= dl)
+      return 0;
+  if (0 > dl)
+      return 0;
+  if (0 >= dl)
+      return 0;
+
+  if (dl == 0UL)
+      return 0;
+  if (dl != 0UL)
+      return 0;
+  if (dl < 0UL)
+      return 0;
+  if (dl <= 0UL)
+      return 0;
+  if (dl > 0UL)
+      return 0;
+  if (dl >= 0UL)
+      return 0;
+
+  if (0UL == dl)
+      return 0;
+  if (0UL != dl)
+      return 0;
+  if (0UL < dl)
+      return 0;
+  if (0UL <= dl)
+      return 0;
+  if (0UL > dl)
+      return 0;
+  if (0UL >= dl)
+      return 0;
+
   return 1;
 }