[clang] [sema] Improve -Wsign-compare (PR #65684)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 8 11:09:28 PDT 2023
=?utf-8?q?Félix?= Cloutier <fcloutier at apple.com>
https://github.com/apple-fcloutier updated https://github.com/llvm/llvm-project/pull/65684:
>From 466a68e59891e0565b8de3201305d7a881785474 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix=20Cloutier?= <fcloutier at apple.com>
Date: Wed, 6 Sep 2023 13:22:42 -0400
Subject: [PATCH 1/2] [sema] Improve -Wsign-compare
Comparisons of integers with different signs are reported by -Wsign-compare
off-by-default diagnostic:
```c
int foo(int s, unsigned u) {
return s < u;
// ^ comparison of integers of different signs: 'int' and 'unsigned int'
}
```
This diagnostic reports a problem that can be rather serious, but it's
non-actionable in its current form when the user doesn't know what happens when
integers of different signs are compared (the signed one is cast to an unsigned
value).
This PR improves the diagnostic to actually explain what potentially unwanted
thing is going to happen and suggest a solution:
```c
int foo(int s, unsigned u) {
return s < u;
// ^ warning: comparison of integers of different signs implicitly casts
// right-side operand from 'int' to 'unsigned int'
// ^ note: consider treating negative values as smaller than any unsigned
// value
}
```
In most cases where the comparison doesn't have side effects, it also suggests
a fixit:
```c
int foo(int s, unsigned u) {
return s < 0 || (unsigned int)s < u;
}
```
Explicitly casting `s` to an unsigned value is what actually silences the
warning.
Radar-ID: 115057570
---
.../clang/Basic/DiagnosticSemaKinds.td | 8 +-
clang/lib/Sema/SemaChecking.cpp | 194 ++++++++++++------
clang/test/FixIt/sign-comparison.c | 129 ++++++++++++
clang/test/Sema/compare.c | 50 ++---
clang/test/SemaCXX/compare.cpp | 62 +++---
5 files changed, 322 insertions(+), 121 deletions(-)
create mode 100644 clang/test/FixIt/sign-comparison.c
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 0ac4df8edb242f6..de653f72da8eb7e 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7237,8 +7237,12 @@ def warn_tautological_compare_value_range : Warning<
InGroup<TautologicalValueRangeCompare>, DefaultIgnore;
def warn_mixed_sign_comparison : Warning<
- "comparison of integers of different signs: %0 and %1">,
- InGroup<SignCompare>, DefaultIgnore;
+ "comparison of integers of different signs implicitly casts "
+ "%select{left|right}0-side operand from %1 to %2">, InGroup<SignCompare>,
+ DefaultIgnore;
+def note_treat_negative_as_smaller : Note<
+ "consider verifying that the signed value is non-negative before the "
+ "comparison; silence this warning by casting to an unsigned type">;
def warn_out_of_range_compare : Warning<
"result of comparison of %select{constant %0|true|false}1 with "
"%select{expression of type %2|boolean expression}3 is always %4">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 3932d9cd07d9864..0f560282deb4cfc 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13710,13 +13710,9 @@ static bool IsSameFloatAfterCast(const APValue &value,
IsSameFloatAfterCast(value.getComplexFloatImag(), Src, Tgt));
}
-static void AnalyzeImplicitConversions(Sema &S, Expr *E, SourceLocation CC,
- bool IsListInit = false);
-
static 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 (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts()))
if (isa<EnumConstantDecl>(DR->getDecl()))
return true;
@@ -14043,28 +14039,61 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E,
return true;
}
+namespace {
+struct AnalyzeImplicitConversionsWorkItem {
+ Expr *E;
+ SourceLocation CC;
+ unsigned IsListInit : 1;
+ unsigned IsTopLevelExpr : 1;
+};
+
+class ImplicitConversionChecker {
+ Sema &S;
+ Expr *TopLevelExpr;
+
+ void AnalyzeCompoundAssignment(BinaryOperator *E);
+ void AnalyzeImplicitConversions(
+ AnalyzeImplicitConversionsWorkItem Item,
+ llvm::SmallVectorImpl<AnalyzeImplicitConversionsWorkItem> &WorkList);
+ void AnalyzeImpConvsInComparison(BinaryOperator *E);
+ void AnalyzeComparison(BinaryOperator *E);
+ void AnalyzeAssignment(BinaryOperator *E);
+ void CheckConditionalOperand(Expr *E, QualType T, SourceLocation CC,
+ bool &ICContext);
+ void CheckConditionalOperator(AbstractConditionalOperator *E,
+ SourceLocation CC, QualType T);
+
+public:
+ ImplicitConversionChecker(Sema &S) : S(S), TopLevelExpr(nullptr) {}
+
+ void AnalyzeImplicitConversions(Expr *E, SourceLocation CC,
+ bool IsInitList = false,
+ bool IsTopLevelExpr = false);
+};
+} // namespace
+
/// Analyze the operands of the given comparison. Implements the
/// fallback case from AnalyzeComparison.
-static void AnalyzeImpConvsInComparison(Sema &S, BinaryOperator *E) {
- AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
- AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+void ImplicitConversionChecker::AnalyzeImpConvsInComparison(BinaryOperator *E) {
+ AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(), false, false);
+ AnalyzeImplicitConversions(E->getRHS(), E->getOperatorLoc(), false, false);
}
/// Implements -Wsign-compare.
///
/// \param E the binary operator to check for warnings
-static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
+void ImplicitConversionChecker::AnalyzeComparison(BinaryOperator *E) {
// The type the comparison is being performed in.
QualType T = E->getLHS()->getType();
// Only analyze comparison operators where both sides have been converted to
// the same type.
if (!S.Context.hasSameUnqualifiedType(T, E->getRHS()->getType()))
- return AnalyzeImpConvsInComparison(S, E);
+ return AnalyzeImpConvsInComparison(E);
// Don't analyze value-dependent comparisons directly.
if (E->isValueDependent())
- return AnalyzeImpConvsInComparison(S, E);
+ return AnalyzeImpConvsInComparison(E);
Expr *LHS = E->getLHS();
Expr *RHS = E->getRHS();
@@ -14077,7 +14106,7 @@ static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
// We don't care about expressions whose result is a constant.
if (RHSValue && LHSValue)
- return AnalyzeImpConvsInComparison(S, E);
+ return AnalyzeImpConvsInComparison(E);
// We only care about expressions where just one side is literal
if ((bool)RHSValue ^ (bool)LHSValue) {
@@ -14090,7 +14119,7 @@ static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
// 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);
+ return AnalyzeImpConvsInComparison(E);
}
}
@@ -14098,7 +14127,7 @@ static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
// 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.
- return AnalyzeImpConvsInComparison(S, E);
+ return AnalyzeImpConvsInComparison(E);
}
LHS = LHS->IgnoreParenImpCasts();
@@ -14126,7 +14155,7 @@ static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
signedOperand = RHS;
unsignedOperand = LHS;
} else {
- return AnalyzeImpConvsInComparison(S, E);
+ return AnalyzeImpConvsInComparison(E);
}
// Otherwise, calculate the effective range of the signed operand.
@@ -14135,8 +14164,8 @@ static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
// Go ahead and analyze implicit conversions in the operands. Note
// that we skip the implicit conversions on both sides.
- AnalyzeImplicitConversions(S, LHS, E->getOperatorLoc());
- AnalyzeImplicitConversions(S, RHS, E->getOperatorLoc());
+ AnalyzeImplicitConversions(LHS, E->getOperatorLoc(), false, false);
+ AnalyzeImplicitConversions(RHS, E->getOperatorLoc(), false, false);
// If the signed range is non-negative, -Wsign-compare won't fire.
if (signedRange.NonNegative)
@@ -14160,10 +14189,55 @@ static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
return;
}
- S.DiagRuntimeBehavior(E->getOperatorLoc(), E,
- S.PDiag(diag::warn_mixed_sign_comparison)
- << LHS->getType() << RHS->getType()
- << LHS->getSourceRange() << RHS->getSourceRange());
+ // For `S < U`, propose something like `S < 0 || (typeof(U))S < U`.
+ // The whole operation needs to have no side effects, since the signed
+ // operand will be duplicated, and the unsigned operand might not execute
+ // anymore.
+ auto MixedSignDiag = S.PDiag(diag::warn_mixed_sign_comparison)
+ << (signedOperand == RHS) << signedOperand->getType()
+ << T << signedOperand->getSourceRange();
+ if (!E->HasSideEffects(S.Context)) {
+ bool ParenthesizeCheck =
+ !TopLevelExpr || E != TopLevelExpr->IgnoreImplicit();
+ std::string Prefix;
+ llvm::raw_string_ostream PrefixS(Prefix);
+ if (ParenthesizeCheck)
+ PrefixS << '(';
+ auto TokRange =
+ CharSourceRange::getTokenRange(signedOperand->getSourceRange());
+ bool Invalid;
+ PrefixS << Lexer::getSourceText(TokRange, S.SourceMgr, S.getLangOpts(),
+ &Invalid);
+ if (!Invalid) {
+ bool ParenthesizeForCast = isa<BinaryOperator>(signedOperand) ||
+ isa<ConditionalOperator>(signedOperand);
+ bool IsSignedLHS = signedOperand == LHS;
+ bool IsLTOrLE = E->getOpcode() == BO_LT || E->getOpcode() == BO_LE;
+ bool IsGTOrGE = E->getOpcode() == BO_GT || E->getOpcode() == BO_GE;
+ if ((IsSignedLHS && IsLTOrLE) || (!IsSignedLHS && IsGTOrGE)) {
+ PrefixS << " < 0 || (";
+ } else {
+ PrefixS << " >= 0 && (";
+ }
+ PrefixS << T.getAsString(S.getLangOpts());
+ PrefixS << ")";
+ if (ParenthesizeForCast)
+ PrefixS << "(";
+ PrefixS.flush();
+ MixedSignDiag << FixItHint::CreateInsertion(LHS->getBeginLoc(), Prefix);
+ if (ParenthesizeForCast) {
+ SourceLocation EndLoc = S.getLocForEndOfToken(LHS->getEndLoc());
+ MixedSignDiag << FixItHint::CreateInsertion(EndLoc, ")");
+ }
+ if (ParenthesizeCheck) {
+ SourceLocation EndLoc = S.getLocForEndOfToken(RHS->getEndLoc());
+ MixedSignDiag << FixItHint::CreateInsertion(EndLoc, ")");
+ }
+ }
+ }
+ S.DiagRuntimeBehavior(E->getOperatorLoc(), E, MixedSignDiag);
+ S.DiagRuntimeBehavior(LHS->getBeginLoc(), E,
+ S.PDiag(diag::note_treat_negative_as_smaller));
}
/// Analyzes an attempt to assign the given value to a bitfield.
@@ -14308,9 +14382,9 @@ static bool AnalyzeBitFieldAssignment(Sema &S, FieldDecl *Bitfield, Expr *Init,
/// Analyze the given simple or compound assignment for warning-worthy
/// operations.
-static void AnalyzeAssignment(Sema &S, BinaryOperator *E) {
+void ImplicitConversionChecker::AnalyzeAssignment(BinaryOperator *E) {
// Just recurse on the LHS.
- AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
+ AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(), false, true);
// We want to recurse on the RHS as normal unless we're assigning to
// a bitfield.
@@ -14318,12 +14392,12 @@ static void AnalyzeAssignment(Sema &S, BinaryOperator *E) {
if (AnalyzeBitFieldAssignment(S, Bitfield, E->getRHS(),
E->getOperatorLoc())) {
// Recurse, ignoring any implicit conversions on the RHS.
- return AnalyzeImplicitConversions(S, E->getRHS()->IgnoreParenImpCasts(),
- E->getOperatorLoc());
+ return AnalyzeImplicitConversions(E->getRHS()->IgnoreParenImpCasts(),
+ E->getOperatorLoc(), false, true);
}
}
- AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+ AnalyzeImplicitConversions(E->getRHS(), E->getOperatorLoc(), false, true);
// Diagnose implicitly sequentially-consistent atomic assignment.
if (E->getLHS()->getType()->isAtomicType())
@@ -14490,12 +14564,12 @@ static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T,
/// Analyze the given compound assignment for the possible losing of
/// floating-point precision.
-static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
+void ImplicitConversionChecker::AnalyzeCompoundAssignment(BinaryOperator *E) {
assert(isa<CompoundAssignOperator>(E) &&
"Must be compound assignment operation");
// Recurse on the LHS and RHS in here
- AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
- AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+ AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(), false, true);
+ AnalyzeImplicitConversions(E->getRHS(), E->getOperatorLoc(), false, true);
if (E->getLHS()->getType()->isAtomicType())
S.Diag(E->getOperatorLoc(), diag::warn_atomic_implicit_seq_cst);
@@ -15277,35 +15351,33 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
}
}
-static void CheckConditionalOperator(Sema &S, AbstractConditionalOperator *E,
- SourceLocation CC, QualType T);
-
-static void CheckConditionalOperand(Sema &S, Expr *E, QualType T,
- SourceLocation CC, bool &ICContext) {
+void ImplicitConversionChecker::CheckConditionalOperand(Expr *E, QualType T,
+ SourceLocation CC,
+ bool &ICContext) {
E = E->IgnoreParenImpCasts();
// Diagnose incomplete type for second or third operand in C.
if (!S.getLangOpts().CPlusPlus && E->getType()->isRecordType())
S.RequireCompleteExprType(E, diag::err_incomplete_type);
if (auto *CO = dyn_cast<AbstractConditionalOperator>(E))
- return CheckConditionalOperator(S, CO, CC, T);
+ return CheckConditionalOperator(CO, CC, T);
- AnalyzeImplicitConversions(S, E, CC);
+ AnalyzeImplicitConversions(E, CC, false, true);
if (E->getType() != T)
return CheckImplicitConversion(S, E, T, CC, &ICContext);
}
-static void CheckConditionalOperator(Sema &S, AbstractConditionalOperator *E,
- SourceLocation CC, QualType T) {
- AnalyzeImplicitConversions(S, E->getCond(), E->getQuestionLoc());
+void ImplicitConversionChecker::CheckConditionalOperator(
+ AbstractConditionalOperator *E, SourceLocation CC, QualType T) {
+ AnalyzeImplicitConversions(E->getCond(), E->getQuestionLoc(), false, true);
Expr *TrueExpr = E->getTrueExpr();
if (auto *BCO = dyn_cast<BinaryConditionalOperator>(E))
TrueExpr = BCO->getCommon();
bool Suspicious = false;
- CheckConditionalOperand(S, TrueExpr, T, CC, Suspicious);
- CheckConditionalOperand(S, E->getFalseExpr(), T, CC, Suspicious);
+ CheckConditionalOperand(TrueExpr, T, CC, Suspicious);
+ CheckConditionalOperand(E->getFalseExpr(), T, CC, Suspicious);
if (T->isBooleanType())
DiagnoseIntInBoolContext(S, E);
@@ -15340,21 +15412,15 @@ static void CheckBoolLikeConversion(Sema &S, Expr *E, SourceLocation CC) {
CheckImplicitConversion(S, E->IgnoreParenImpCasts(), S.Context.BoolTy, CC);
}
-namespace {
-struct AnalyzeImplicitConversionsWorkItem {
- Expr *E;
- SourceLocation CC;
- bool IsListInit;
-};
-}
-
/// Data recursive variant of AnalyzeImplicitConversions. Subexpressions
/// that should be visited are added to WorkList.
-static void AnalyzeImplicitConversions(
- Sema &S, AnalyzeImplicitConversionsWorkItem Item,
+void ImplicitConversionChecker::AnalyzeImplicitConversions(
+ AnalyzeImplicitConversionsWorkItem Item,
llvm::SmallVectorImpl<AnalyzeImplicitConversionsWorkItem> &WorkList) {
Expr *OrigE = Item.E;
SourceLocation CC = Item.CC;
+ Expr *TopLevel = Item.IsTopLevelExpr ? OrigE : TopLevelExpr;
+ llvm::SaveAndRestore TLE(TopLevelExpr, TopLevel);
QualType T = OrigE->getType();
Expr *E = OrigE->IgnoreParenImpCasts();
@@ -15401,7 +15467,7 @@ static void AnalyzeImplicitConversions(
// For conditional operators, we analyze the arguments as if they
// were being fed directly into the output.
if (auto *CO = dyn_cast<AbstractConditionalOperator>(SourceExpr)) {
- CheckConditionalOperator(S, CO, CC, T);
+ CheckConditionalOperator(CO, CC, T);
return;
}
@@ -15423,7 +15489,7 @@ static void AnalyzeImplicitConversions(
// FIXME: Use a more uniform representation for this.
for (auto *SE : POE->semantics())
if (auto *OVE = dyn_cast<OpaqueValueExpr>(SE))
- WorkList.push_back({OVE->getSourceExpr(), CC, IsListInit});
+ WorkList.push_back({OVE->getSourceExpr(), CC, IsListInit, false});
}
// Skip past explicit casts.
@@ -15431,21 +15497,21 @@ static void AnalyzeImplicitConversions(
E = CE->getSubExpr()->IgnoreParenImpCasts();
if (!CE->getType()->isVoidType() && E->getType()->isAtomicType())
S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst);
- WorkList.push_back({E, CC, IsListInit});
+ WorkList.push_back({E, CC, IsListInit, false});
return;
}
if (BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
// Do a somewhat different check with comparison operators.
if (BO->isComparisonOp())
- return AnalyzeComparison(S, BO);
+ return AnalyzeComparison(BO);
// And with simple assignments.
if (BO->getOpcode() == BO_Assign)
- return AnalyzeAssignment(S, BO);
+ return AnalyzeAssignment(BO);
// And with compound assignments.
if (BO->isAssignmentOp())
- return AnalyzeCompoundAssignment(S, BO);
+ return AnalyzeCompoundAssignment(BO);
}
// These break the otherwise-useful invariant below. Fortunately,
@@ -15478,7 +15544,7 @@ static void AnalyzeImplicitConversions(
// Ignore checking string literals that are in logical and operators.
// This is a common pattern for asserts.
continue;
- WorkList.push_back({ChildExpr, CC, IsListInit});
+ WorkList.push_back({ChildExpr, CC, IsListInit, false});
}
if (BO && BO->isLogicalOp()) {
@@ -15505,12 +15571,13 @@ static void AnalyzeImplicitConversions(
/// AnalyzeImplicitConversions - Find and report any interesting
/// implicit conversions in the given expression. There are a couple
/// of competing diagnostics here, -Wconversion and -Wsign-compare.
-static void AnalyzeImplicitConversions(Sema &S, Expr *OrigE, SourceLocation CC,
- bool IsListInit/*= false*/) {
+void ImplicitConversionChecker::AnalyzeImplicitConversions(
+ Expr *OrigE, SourceLocation CC, bool IsListInit /*= false*/,
+ bool IsTopLevelExpr /*= false */) {
llvm::SmallVector<AnalyzeImplicitConversionsWorkItem, 16> WorkList;
- WorkList.push_back({OrigE, CC, IsListInit});
+ WorkList.push_back({OrigE, CC, IsListInit, IsTopLevelExpr});
while (!WorkList.empty())
- AnalyzeImplicitConversions(S, WorkList.pop_back_val(), WorkList);
+ AnalyzeImplicitConversions(WorkList.pop_back_val(), WorkList);
}
/// Diagnose integer type and any valid implicit conversion to it.
@@ -15789,7 +15856,8 @@ void Sema::CheckImplicitConversions(Expr *E, SourceLocation CC) {
CheckArrayAccess(E);
// This is not the right CC for (e.g.) a variable initialization.
- AnalyzeImplicitConversions(*this, E, CC);
+ ImplicitConversionChecker(*this).AnalyzeImplicitConversions(E, CC, false,
+ true);
}
/// CheckBoolLikeConversion - Check conversion of given expression to boolean.
diff --git a/clang/test/FixIt/sign-comparison.c b/clang/test/FixIt/sign-comparison.c
new file mode 100644
index 000000000000000..a56bc3613dfd67a
--- /dev/null
+++ b/clang/test/FixIt/sign-comparison.c
@@ -0,0 +1,129 @@
+// RUN: %clang_cc1 -fsyntax-only -Wsign-compare -Wno-unused-comparison -Wno-empty-body -Wno-unused-value -verify %s
+// RUN: cp %s %t
+// RUN: %clang_cc1 -fsyntax-only -Wsign-compare -fixit -x c %t 2> /dev/null
+// RUN: grep -v CHECK %t | FileCheck %s
+
+unsigned Uf(void);
+int Sf(void);
+
+void test(signed S, unsigned U) {
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand from 'int' to 'unsigned int'}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ S < U;
+ // CHECK: S < 0 || (unsigned int)S < U;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ S <= U;
+ // CHECK: S < 0 || (unsigned int)S <= U;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ S == U;
+ // CHECK: S >= 0 && (unsigned int)S == U;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ S > U;
+ // CHECK: S >= 0 && (unsigned int)S > U;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ S >= U;
+ // CHECK: S >= 0 && (unsigned int)S >= U;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ S != U;
+ // CHECK: S >= 0 && (unsigned int)S != U;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts right-side operand from 'int' to 'unsigned int'}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ U < S;
+ // CHECK: S >= 0 && (unsigned int)U < S;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts right-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ U <= S;
+ // CHECK: S >= 0 && (unsigned int)U <= S;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts right-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ U == S;
+ // CHECK: S >= 0 && (unsigned int)U == S;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts right-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ U > S;
+ // CHECK: S < 0 || (unsigned int)U > S;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts right-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ U >= S;
+ // CHECK: S < 0 || (unsigned int)U >= S;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts right-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ U != S;
+ // CHECK: S >= 0 && (unsigned int)U != S;
+
+
+ // expected-warning at +6{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-warning at +5{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-warning at +4{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +3{{consider verifying that the signed value is non-negative}}
+ // expected-note at +2{{consider verifying that the signed value is non-negative}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ for (S < U; S < U; S < U) ;
+ // CHECK: for (S < 0 || (unsigned int)S < U; S < 0 || (unsigned int)S < U; S < 0 || (unsigned int)S < U) ;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ if (S < U) ;
+ // CHECK: if (S < 0 || (unsigned int)S < U) ;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ 1 && S < U;
+ // CHECK: 1 && (S < 0 || (unsigned int)S < U);
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ S < U + U;
+ // CHECK: S < 0 || (unsigned int)S < U + U;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ S + S < U;
+ // CHECK: S + S < 0 || (unsigned int)(S + S) < U
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ Sf() < U;
+ // CHECK: Sf() < U;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ S < Uf();
+ // CHECK: S < Uf();
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ S++ < U;
+ // CHECK: S++ < U;
+
+ // expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ S < U++;
+ // CHECK: S < U++;
+}
+
+typedef unsigned uint32_t;
+typedef int int32_t;
+
+void test2(int32_t S, uint32_t U) {
+ // expected-warning at +2{{comparison of integers of different signs}}
+ // expected-note at +1{{consider verifying that the signed value is non-negative}}
+ S < U;
+ // CHECK: S < 0 || (uint32_t)S < U;
+}
diff --git a/clang/test/Sema/compare.c b/clang/test/Sema/compare.c
index 17cf0351ef4f53f..c700627e18506d9 100644
--- a/clang/test/Sema/compare.c
+++ b/clang/test/Sema/compare.c
@@ -14,28 +14,28 @@ int ints(long a, unsigned long b) {
enum EnumC {C = 0x10000};
return
// (a,b)
- (a == (unsigned long) b) + // expected-warning {{comparison of integers of different signs}}
+ (a == (unsigned long) b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
(a == (unsigned int) b) +
(a == (unsigned short) b) +
(a == (unsigned char) b) +
- ((long) a == b) + // expected-warning {{comparison of integers of different signs}}
- ((int) a == b) + // expected-warning {{comparison of integers of different signs}}
- ((short) a == b) + // expected-warning {{comparison of integers of different signs}}
- ((signed char) a == b) + // expected-warning {{comparison of integers of different signs}}
- ((long) a == (unsigned long) b) + // expected-warning {{comparison of integers of different signs}}
- ((int) a == (unsigned int) b) + // expected-warning {{comparison of integers of different signs}}
+ ((long) a == b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((int) a == b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((short) a == b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((signed char) a == b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((long) a == (unsigned long) b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((int) a == (unsigned int) b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
((short) a == (unsigned short) b) +
((signed char) a == (unsigned char) b) +
- (a < (unsigned long) b) + // expected-warning {{comparison of integers of different signs}}
+ (a < (unsigned long) b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
(a < (unsigned int) b) +
(a < (unsigned short) b) +
(a < (unsigned char) b) +
- ((long) a < b) + // expected-warning {{comparison of integers of different signs}}
- ((int) a < b) + // expected-warning {{comparison of integers of different signs}}
- ((short) a < b) + // expected-warning {{comparison of integers of different signs}}
- ((signed char) a < b) + // expected-warning {{comparison of integers of different signs}}
- ((long) a < (unsigned long) b) + // expected-warning {{comparison of integers of different signs}}
- ((int) a < (unsigned int) b) + // expected-warning {{comparison of integers of different signs}}
+ ((long) a < b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((int) a < b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((short) a < b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((signed char) a < b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((long) a < (unsigned long) b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((int) a < (unsigned int) b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
((short) a < (unsigned short) b) +
((signed char) a < (unsigned char) b) +
@@ -130,7 +130,7 @@ int ints(long a, unsigned long b) {
((int) a == (unsigned int) C) +
((short) a == (unsigned short) C) +
((signed char) a == (unsigned char) C) +
- (a < (unsigned long) C) + // expected-warning {{comparison of integers of different signs}}
+ (a < (unsigned long) C) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
(a < (unsigned int) C) +
(a < (unsigned short) C) +
(a < (unsigned char) C) +
@@ -138,8 +138,8 @@ int ints(long a, unsigned long b) {
((int) a < C) +
((short) a < C) + // expected-warning {{comparison of constant 'C' (65536) with expression of type 'short' is always true}}
((signed char) a < C) + // expected-warning {{comparison of constant 'C' (65536) with expression of type 'signed char' is always true}}
- ((long) a < (unsigned long) C) + // expected-warning {{comparison of integers of different signs}}
- ((int) a < (unsigned int) C) + // expected-warning {{comparison of integers of different signs}}
+ ((long) a < (unsigned long) C) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((int) a < (unsigned int) C) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
((short) a < (unsigned short) C) +
((signed char) a < (unsigned char) C) +
@@ -182,7 +182,7 @@ int ints(long a, unsigned long b) {
((int) a == (unsigned int) 0x80000) +
((short) a == (unsigned short) 0x80000) +
((signed char) a == (unsigned char) 0x80000) +
- (a < (unsigned long) 0x80000) + // expected-warning {{comparison of integers of different signs}}
+ (a < (unsigned long) 0x80000) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
(a < (unsigned int) 0x80000) +
(a < (unsigned short) 0x80000) +
(a < (unsigned char) 0x80000) +
@@ -190,8 +190,8 @@ int ints(long a, unsigned long b) {
((int) a < 0x80000) +
((short) a < 0x80000) + // expected-warning {{comparison of constant 524288 with expression of type 'short' is always true}}
((signed char) a < 0x80000) + // expected-warning {{comparison of constant 524288 with expression of type 'signed char' is always true}}
- ((long) a < (unsigned long) 0x80000) + // expected-warning {{comparison of integers of different signs}}
- ((int) a < (unsigned int) 0x80000) + // expected-warning {{comparison of integers of different signs}}
+ ((long) a < (unsigned long) 0x80000) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((int) a < (unsigned int) 0x80000) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
((short) a < (unsigned short) 0x80000) +
((signed char) a < (unsigned char) 0x80000) +
@@ -250,11 +250,11 @@ int test2(int i32) {
return 0;
} else if (x->u31 == i32) { // comparison in int32, exact
return 1;
- } else if (x->u32 == i32) { // expected-warning {{comparison of integers of different signs}}
+ } else if (x->u32 == i32) { // expected-warning {{comparison of integers of different signs}} expected-note{{}}
return 2;
} else if (x->u63 == i32) { // comparison in uint64, exact because ==
return 3;
- } else if (x->u64 == i32) { // expected-warning {{comparison of integers of different signs}}
+ } else if (x->u64 == i32) { // expected-warning {{comparison of integers of different signs}} expected-note{{}}
return 4;
} else {
return 5;
@@ -273,7 +273,7 @@ void test3(void) {
extern char *ptr4;
void test4(void) {
long value;
- if (value < (unsigned long) &ptr4) // expected-warning {{comparison of integers of different signs}}
+ if (value < (unsigned long) &ptr4) // expected-warning {{comparison of integers of different signs}} expected-note{{}}
return;
}
@@ -381,7 +381,7 @@ int rdar8511238(void) {
// PR10336
int test9(int sv, unsigned uv, long slv) {
- return sv == (uv ^= slv); // expected-warning {{comparison of integers of different signs: 'int' and 'unsigned int'}}
+ return sv == (uv ^= slv); // expected-warning {{comparison of integers of different signs}} expected-note{{}}
}
void test10(void) {
@@ -390,7 +390,7 @@ void test10(void) {
long sl;
_Bool b;
- b = (si == (ui = sl)); // expected-warning {{comparison of integers of different signs: 'int' and 'unsigned int'}}
+ b = (si == (ui = sl)); // expected-warning {{comparison of integers of different signs}} expected-note{{}}
b = (si == (ui = sl&15));
}
diff --git a/clang/test/SemaCXX/compare.cpp b/clang/test/SemaCXX/compare.cpp
index cfddf2142f30839..82098b5c4a81294 100644
--- a/clang/test/SemaCXX/compare.cpp
+++ b/clang/test/SemaCXX/compare.cpp
@@ -10,28 +10,28 @@ int test0(long a, unsigned long b) {
enum EnumC {C = 0x10000};
return
// (a,b)
- (a == (unsigned long) b) + // expected-warning {{comparison of integers of different signs}}
+ (a == (unsigned long) b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
(a == (unsigned int) b) +
(a == (unsigned short) b) +
(a == (unsigned char) b) +
- ((long) a == b) + // expected-warning {{comparison of integers of different signs}}
- ((int) a == b) + // expected-warning {{comparison of integers of different signs}}
- ((short) a == b) + // expected-warning {{comparison of integers of different signs}}
- ((signed char) a == b) + // expected-warning {{comparison of integers of different signs}}
- ((long) a == (unsigned long) b) + // expected-warning {{comparison of integers of different signs}}
- ((int) a == (unsigned int) b) + // expected-warning {{comparison of integers of different signs}}
+ ((long) a == b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((int) a == b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((short) a == b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((signed char) a == b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((long) a == (unsigned long) b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((int) a == (unsigned int) b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
((short) a == (unsigned short) b) +
((signed char) a == (unsigned char) b) +
- (a < (unsigned long) b) + // expected-warning {{comparison of integers of different signs}}
+ (a < (unsigned long) b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
(a < (unsigned int) b) +
(a < (unsigned short) b) +
(a < (unsigned char) b) +
- ((long) a < b) + // expected-warning {{comparison of integers of different signs}}
- ((int) a < b) + // expected-warning {{comparison of integers of different signs}}
- ((short) a < b) + // expected-warning {{comparison of integers of different signs}}
- ((signed char) a < b) + // expected-warning {{comparison of integers of different signs}}
- ((long) a < (unsigned long) b) + // expected-warning {{comparison of integers of different signs}}
- ((int) a < (unsigned int) b) + // expected-warning {{comparison of integers of different signs}}
+ ((long) a < b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((int) a < b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((short) a < b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((signed char) a < b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((long) a < (unsigned long) b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((int) a < (unsigned int) b) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
((short) a < (unsigned short) b) +
((signed char) a < (unsigned char) b) +
@@ -126,7 +126,7 @@ int test0(long a, unsigned long b) {
((int) a == (unsigned int) C) +
((short) a == (unsigned short) C) +
((signed char) a == (unsigned char) C) +
- (a < (unsigned long) C) + // expected-warning {{comparison of integers of different signs}}
+ (a < (unsigned long) C) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
(a < (unsigned int) C) +
(a < (unsigned short) C) +
(a < (unsigned char) C) +
@@ -134,8 +134,8 @@ int test0(long a, unsigned long b) {
((int) a < C) +
((short) a < C) + // expected-warning {{comparison of constant 'C' (65536) with expression of type 'short' is always true}}
((signed char) a < C) + // expected-warning {{comparison of constant 'C' (65536) with expression of type 'signed char' is always true}}
- ((long) a < (unsigned long) C) + // expected-warning {{comparison of integers of different signs}}
- ((int) a < (unsigned int) C) + // expected-warning {{comparison of integers of different signs}}
+ ((long) a < (unsigned long) C) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((int) a < (unsigned int) C) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
((short) a < (unsigned short) C) +
((signed char) a < (unsigned char) C) +
@@ -178,7 +178,7 @@ int test0(long a, unsigned long b) {
((int) a == (unsigned int) 0x80000) +
((short) a == (unsigned short) 0x80000) +
((signed char) a == (unsigned char) 0x80000) +
- (a < (unsigned long) 0x80000) + // expected-warning {{comparison of integers of different signs}}
+ (a < (unsigned long) 0x80000) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
(a < (unsigned int) 0x80000) +
(a < (unsigned short) 0x80000) +
(a < (unsigned char) 0x80000) +
@@ -186,8 +186,8 @@ int test0(long a, unsigned long b) {
((int) a < 0x80000) +
((short) a < 0x80000) + // expected-warning {{comparison of constant 524288 with expression of type 'short' is always true}}
((signed char) a < 0x80000) + // expected-warning {{comparison of constant 524288 with expression of type 'signed char' is always true}}
- ((long) a < (unsigned long) 0x80000) + // expected-warning {{comparison of integers of different signs}}
- ((int) a < (unsigned int) 0x80000) + // expected-warning {{comparison of integers of different signs}}
+ ((long) a < (unsigned long) 0x80000) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
+ ((int) a < (unsigned int) 0x80000) + // expected-warning {{comparison of integers of different signs}} expected-note{{}}
((short) a < (unsigned short) 0x80000) +
((signed char) a < (unsigned char) 0x80000) +
@@ -233,10 +233,10 @@ void test4(short s) {
// All negative shorts are cast towards the max unsigned range. Relation
// comparisons are possible, but equality comparisons are tautological.
const unsigned A = 32768;
- void (s < A); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
- void (s > A); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
- void (s <= A); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
- void (s >= A); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
+ void (s < A); // expected-warning{{comparison of integers of different signs}} expected-note{{}}
+ void (s > A); // expected-warning{{comparison of integers of different signs}} expected-note{{}}
+ void (s <= A); // expected-warning{{comparison of integers of different signs}} expected-note{{}}
+ void (s >= A); // expected-warning{{comparison of integers of different signs}} expected-note{{}}
void (s == A); // expected-warning{{comparison of constant 32768 with expression of type 'short' is always false}}
void (s != A); // expected-warning{{comparison of constant 32768 with expression of type 'short' is always true}}
@@ -245,12 +245,12 @@ void test4(short s) {
// unsigned. Likewise, a negative one short can also be converted to max
// unsigned.
const unsigned B = -1;
- void (s < B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
+ void (s < B); // expected-warning{{comparison of integers of different signs}} expected-note{{}}
void (s > B); // expected-warning{{comparison 'short' > 4294967295 is always false}}
void (s <= B); // expected-warning{{comparison 'short' <= 4294967295 is always true}}
- void (s >= B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
- void (s == B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
- void (s != B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
+ void (s >= B); // expected-warning{{comparison of integers of different signs}} expected-note{{}}
+ void (s == B); // expected-warning{{comparison of integers of different signs}} expected-note{{}}
+ void (s != B); // expected-warning{{comparison of integers of different signs}} expected-note{{}}
}
@@ -309,14 +309,14 @@ void test7(unsigned long other) {
(void)((unsigned long)other != (unsigned)(0xffffffff));
// Common unsigned, other signed, constant unsigned
- (void)((int)other != (unsigned long)(0xffffffffffffffff)); // expected-warning{{different signs}}
+ (void)((int)other != (unsigned long)(0xffffffffffffffff)); // expected-warning{{different signs}} expected-note{{}}
(void)((int)other != (unsigned long)(0x00000000ffffffff)); // expected-warning{{true}}
(void)((int)other != (unsigned long)(0x000000000fffffff));
- (void)((int)other < (unsigned long)(0x00000000ffffffff)); // expected-warning{{different signs}}
+ (void)((int)other < (unsigned long)(0x00000000ffffffff)); // expected-warning{{different signs}} expected-note{{}}
(void)((int)other == (unsigned)(0x800000000));
// Common unsigned, other unsigned, constant signed
- (void)((unsigned long)other != (int)(0xffffffff)); // expected-warning{{different signs}}
+ (void)((unsigned long)other != (int)(0xffffffff)); // expected-warning{{different signs}} expected-note{{}}
// Common unsigned, other signed, constant signed
// Should not be possible as the common type should also be signed.
>From 0835a85b2c4593133d83a1d04558c87ede8d4588 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix=20Cloutier?= <fcloutier at apple.com>
Date: Fri, 8 Sep 2023 14:09:04 -0400
Subject: [PATCH 2/2] Address review feedback
---
clang/lib/Sema/SemaChecking.cpp | 52 ++++++++++++++++---------
clang/test/FixIt/sign-comparison.c | 34 +++++++++++++++-
clang/test/Frontend/Weverything.c | 2 +-
clang/test/Frontend/Wno-everything.c | 5 ++-
clang/test/Frontend/warning-mapping-5.c | 2 +-
clang/test/Sema/sign-compare-enum.c | 26 ++++++++-----
6 files changed, 87 insertions(+), 34 deletions(-)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 0f560282deb4cfc..e039c390ab5383d 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -14075,8 +14075,10 @@ class ImplicitConversionChecker {
/// Analyze the operands of the given comparison. Implements the
/// fallback case from AnalyzeComparison.
void ImplicitConversionChecker::AnalyzeImpConvsInComparison(BinaryOperator *E) {
- AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(), false, false);
- AnalyzeImplicitConversions(E->getRHS(), E->getOperatorLoc(), false, false);
+ AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(),
+ /*IsInitList=*/false, /*IsTopLevelExpr=*/false);
+ AnalyzeImplicitConversions(E->getRHS(), E->getOperatorLoc(),
+ /*IsInitList=*/false, /*IsTopLevelExpr=*/false);
}
/// Implements -Wsign-compare.
@@ -14164,8 +14166,10 @@ void ImplicitConversionChecker::AnalyzeComparison(BinaryOperator *E) {
// Go ahead and analyze implicit conversions in the operands. Note
// that we skip the implicit conversions on both sides.
- AnalyzeImplicitConversions(LHS, E->getOperatorLoc(), false, false);
- AnalyzeImplicitConversions(RHS, E->getOperatorLoc(), false, false);
+ AnalyzeImplicitConversions(LHS, E->getOperatorLoc(), /*IsInitList=*/false,
+ /*IsTopLevelExpr=*/false);
+ AnalyzeImplicitConversions(RHS, E->getOperatorLoc(), /*IsInitList=*/false,
+ /*IsTopLevelExpr=*/false);
// If the signed range is non-negative, -Wsign-compare won't fire.
if (signedRange.NonNegative)
@@ -14209,18 +14213,21 @@ void ImplicitConversionChecker::AnalyzeComparison(BinaryOperator *E) {
PrefixS << Lexer::getSourceText(TokRange, S.SourceMgr, S.getLangOpts(),
&Invalid);
if (!Invalid) {
- bool ParenthesizeForCast = isa<BinaryOperator>(signedOperand) ||
+ bool ParenthesizeForCast = S.getLangOpts().CPlusPlus ||
+ isa<BinaryOperator>(signedOperand) ||
isa<ConditionalOperator>(signedOperand);
bool IsSignedLHS = signedOperand == LHS;
bool IsLTOrLE = E->getOpcode() == BO_LT || E->getOpcode() == BO_LE;
bool IsGTOrGE = E->getOpcode() == BO_GT || E->getOpcode() == BO_GE;
if ((IsSignedLHS && IsLTOrLE) || (!IsSignedLHS && IsGTOrGE)) {
- PrefixS << " < 0 || (";
+ PrefixS << " < 0 || ";
} else {
- PrefixS << " >= 0 && (";
+ PrefixS << " >= 0 && ";
}
- PrefixS << T.getAsString(S.getLangOpts());
- PrefixS << ")";
+ if (S.getLangOpts().CPlusPlus)
+ PrefixS << "static_cast<" << T.getAsString(S.getLangOpts()) << ">";
+ else
+ PrefixS << "(" << T.getAsString(S.getLangOpts()) << ")";
if (ParenthesizeForCast)
PrefixS << "(";
PrefixS.flush();
@@ -14384,7 +14391,8 @@ static bool AnalyzeBitFieldAssignment(Sema &S, FieldDecl *Bitfield, Expr *Init,
/// operations.
void ImplicitConversionChecker::AnalyzeAssignment(BinaryOperator *E) {
// Just recurse on the LHS.
- AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(), false, true);
+ AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(),
+ /*IsInitList=*/false, /*IsTopLevelExpr=*/true);
// We want to recurse on the RHS as normal unless we're assigning to
// a bitfield.
@@ -14392,8 +14400,9 @@ void ImplicitConversionChecker::AnalyzeAssignment(BinaryOperator *E) {
if (AnalyzeBitFieldAssignment(S, Bitfield, E->getRHS(),
E->getOperatorLoc())) {
// Recurse, ignoring any implicit conversions on the RHS.
- return AnalyzeImplicitConversions(E->getRHS()->IgnoreParenImpCasts(),
- E->getOperatorLoc(), false, true);
+ return AnalyzeImplicitConversions(
+ E->getRHS()->IgnoreParenImpCasts(), E->getOperatorLoc(),
+ /*IsInitList=*/false, /*IsTopLevelExpr=*/true);
}
}
@@ -14568,8 +14577,10 @@ void ImplicitConversionChecker::AnalyzeCompoundAssignment(BinaryOperator *E) {
assert(isa<CompoundAssignOperator>(E) &&
"Must be compound assignment operation");
// Recurse on the LHS and RHS in here
- AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(), false, true);
- AnalyzeImplicitConversions(E->getRHS(), E->getOperatorLoc(), false, true);
+ AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(),
+ /*IsInitList=*/false, /*IsTopLevelExpr=*/true);
+ AnalyzeImplicitConversions(E->getRHS(), E->getOperatorLoc(),
+ /*IsInitList=*/false, /*IsTopLevelExpr=*/true);
if (E->getLHS()->getType()->isAtomicType())
S.Diag(E->getOperatorLoc(), diag::warn_atomic_implicit_seq_cst);
@@ -15362,14 +15373,16 @@ void ImplicitConversionChecker::CheckConditionalOperand(Expr *E, QualType T,
if (auto *CO = dyn_cast<AbstractConditionalOperator>(E))
return CheckConditionalOperator(CO, CC, T);
- AnalyzeImplicitConversions(E, CC, false, true);
+ AnalyzeImplicitConversions(E, CC, /*IsInitList=*/false,
+ /*IsTopLevelExpr=*/true);
if (E->getType() != T)
return CheckImplicitConversion(S, E, T, CC, &ICContext);
}
void ImplicitConversionChecker::CheckConditionalOperator(
AbstractConditionalOperator *E, SourceLocation CC, QualType T) {
- AnalyzeImplicitConversions(E->getCond(), E->getQuestionLoc(), false, true);
+ AnalyzeImplicitConversions(E->getCond(), E->getQuestionLoc(),
+ /*IsInitList=*/false, /*IsTopLevelExpr=*/true);
Expr *TrueExpr = E->getTrueExpr();
if (auto *BCO = dyn_cast<BinaryConditionalOperator>(E))
@@ -15489,7 +15502,8 @@ void ImplicitConversionChecker::AnalyzeImplicitConversions(
// FIXME: Use a more uniform representation for this.
for (auto *SE : POE->semantics())
if (auto *OVE = dyn_cast<OpaqueValueExpr>(SE))
- WorkList.push_back({OVE->getSourceExpr(), CC, IsListInit, false});
+ WorkList.push_back(
+ {OVE->getSourceExpr(), CC, IsListInit, /*IsTopLevelExpr=*/false});
}
// Skip past explicit casts.
@@ -15497,7 +15511,7 @@ void ImplicitConversionChecker::AnalyzeImplicitConversions(
E = CE->getSubExpr()->IgnoreParenImpCasts();
if (!CE->getType()->isVoidType() && E->getType()->isAtomicType())
S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst);
- WorkList.push_back({E, CC, IsListInit, false});
+ WorkList.push_back({E, CC, IsListInit, /*IsTopLevelExpr=*/false});
return;
}
@@ -15544,7 +15558,7 @@ void ImplicitConversionChecker::AnalyzeImplicitConversions(
// Ignore checking string literals that are in logical and operators.
// This is a common pattern for asserts.
continue;
- WorkList.push_back({ChildExpr, CC, IsListInit, false});
+ WorkList.push_back({ChildExpr, CC, IsListInit, /*IsTopLevelExpr=*/false});
}
if (BO && BO->isLogicalOp()) {
diff --git a/clang/test/FixIt/sign-comparison.c b/clang/test/FixIt/sign-comparison.c
index a56bc3613dfd67a..3150c1f7714307d 100644
--- a/clang/test/FixIt/sign-comparison.c
+++ b/clang/test/FixIt/sign-comparison.c
@@ -1,7 +1,11 @@
-// RUN: %clang_cc1 -fsyntax-only -Wsign-compare -Wno-unused-comparison -Wno-empty-body -Wno-unused-value -verify %s
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin -fsyntax-only -Wsign-compare -Wno-unused-comparison -Wno-empty-body -Wno-unused-value -verify %s
+// RUN: %clang_cc1 -triple=armv7-apple-darwin -fsyntax-only -Wsign-compare -Wno-unused-comparison -Wno-empty-body -Wno-unused-value -verify %s
// RUN: cp %s %t
// RUN: %clang_cc1 -fsyntax-only -Wsign-compare -fixit -x c %t 2> /dev/null
// RUN: grep -v CHECK %t | FileCheck %s
+// RUN: cp %s %t
+// RUN: %clang_cc1 -fsyntax-only -Wsign-compare -fixit -x c++ %t 2> /dev/null
+// RUN: grep -v CHECK %t | FileCheck --check-prefix=CHECKXX %s
unsigned Uf(void);
int Sf(void);
@@ -11,61 +15,73 @@ void test(signed S, unsigned U) {
// expected-note at +1{{consider verifying that the signed value is non-negative}}
S < U;
// CHECK: S < 0 || (unsigned int)S < U;
+ // CHECKXX: S < 0 || static_cast<unsigned int>(S) < U;
// expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
S <= U;
// CHECK: S < 0 || (unsigned int)S <= U;
+ // CHECKXX: S < 0 || static_cast<unsigned int>(S) <= U;
// expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
S == U;
// CHECK: S >= 0 && (unsigned int)S == U;
+ // CHECKXX: S >= 0 && static_cast<unsigned int>(S) == U;
// expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
S > U;
// CHECK: S >= 0 && (unsigned int)S > U;
+ // CHECKXX: S >= 0 && static_cast<unsigned int>(S) > U;
// expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
S >= U;
// CHECK: S >= 0 && (unsigned int)S >= U;
+ // CHECKXX: S >= 0 && static_cast<unsigned int>(S) >= U;
// expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
S != U;
// CHECK: S >= 0 && (unsigned int)S != U;
+ // CHECKXX: S >= 0 && static_cast<unsigned int>(S) != U;
// expected-warning at +2{{comparison of integers of different signs implicitly casts right-side operand from 'int' to 'unsigned int'}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
U < S;
// CHECK: S >= 0 && (unsigned int)U < S;
+ // CHECKXX: S >= 0 && static_cast<unsigned int>(U) < S;
// expected-warning at +2{{comparison of integers of different signs implicitly casts right-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
U <= S;
// CHECK: S >= 0 && (unsigned int)U <= S;
+ // CHECKXX: S >= 0 && static_cast<unsigned int>(U) <= S;
// expected-warning at +2{{comparison of integers of different signs implicitly casts right-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
U == S;
// CHECK: S >= 0 && (unsigned int)U == S;
+ // CHECKXX: S >= 0 && static_cast<unsigned int>(U) == S;
// expected-warning at +2{{comparison of integers of different signs implicitly casts right-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
U > S;
// CHECK: S < 0 || (unsigned int)U > S;
+ // CHECKXX: S < 0 || static_cast<unsigned int>(U) > S;
// expected-warning at +2{{comparison of integers of different signs implicitly casts right-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
U >= S;
// CHECK: S < 0 || (unsigned int)U >= S;
+ // CHECKXX: S < 0 || static_cast<unsigned int>(U) >= S;
// expected-warning at +2{{comparison of integers of different signs implicitly casts right-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
U != S;
// CHECK: S >= 0 && (unsigned int)U != S;
+ // CHECKXX: S >= 0 && static_cast<unsigned int>(U) != S;
// expected-warning at +6{{comparison of integers of different signs implicitly casts left-side operand}}
@@ -76,46 +92,61 @@ void test(signed S, unsigned U) {
// expected-note at +1{{consider verifying that the signed value is non-negative}}
for (S < U; S < U; S < U) ;
// CHECK: for (S < 0 || (unsigned int)S < U; S < 0 || (unsigned int)S < U; S < 0 || (unsigned int)S < U) ;
+ // CHECKXX: for (S < 0 || static_cast<unsigned int>(S) < U; S < 0 || static_cast<unsigned int>(S) < U; S < 0 || static_cast<unsigned int>(S) < U) ;
// expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
if (S < U) ;
// CHECK: if (S < 0 || (unsigned int)S < U) ;
+ // CHECKXX: if (S < 0 || static_cast<unsigned int>(S) < U) ;
// expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
1 && S < U;
// CHECK: 1 && (S < 0 || (unsigned int)S < U);
+ // CHECKXX: 1 && (S < 0 || static_cast<unsigned int>(S) < U);
// expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
S < U + U;
// CHECK: S < 0 || (unsigned int)S < U + U;
+ // CHECKXX: S < 0 || static_cast<unsigned int>(S) < U + U;
// expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
S + S < U;
// CHECK: S + S < 0 || (unsigned int)(S + S) < U
+ // CHECKXX: S + S < 0 || static_cast<unsigned int>(S + S) < U
// expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
Sf() < U;
// CHECK: Sf() < U;
+ // CHECKXX: Sf() < U;
// expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
S < Uf();
// CHECK: S < Uf();
+ // CHECKXX: S < Uf();
// expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
S++ < U;
// CHECK: S++ < U;
+ // CHECKXX: S++ < U;
// expected-warning at +2{{comparison of integers of different signs implicitly casts left-side operand}}
// expected-note at +1{{consider verifying that the signed value is non-negative}}
S < U++;
// CHECK: S < U++;
+ // CHECKXX: S < U++;
+
+#if !__LP64__
+ // expected-warning at +3{{comparison of integers of different signs implicitly casts left-side operand}}
+ // expected-note at +2{{consider verifying that the signed value is non-negative}}
+#endif
+ (long)S < U;
}
typedef unsigned uint32_t;
@@ -126,4 +157,5 @@ void test2(int32_t S, uint32_t U) {
// expected-note at +1{{consider verifying that the signed value is non-negative}}
S < U;
// CHECK: S < 0 || (uint32_t)S < U;
+ // CHECKXX: S < 0 || static_cast<uint32_t>(S) < U;
}
diff --git a/clang/test/Frontend/Weverything.c b/clang/test/Frontend/Weverything.c
index 32f314720b24841..bd16016638c87fb 100644
--- a/clang/test/Frontend/Weverything.c
+++ b/clang/test/Frontend/Weverything.c
@@ -5,5 +5,5 @@
int f0(int, unsigned);
int f0(int x, unsigned y) {
- return x < y; // expected-warning {{comparison of integers}}
+ return x < y; // expected-warning {{comparison of integers}} expected-note{{}}
}
diff --git a/clang/test/Frontend/Wno-everything.c b/clang/test/Frontend/Wno-everything.c
index ca70ca4c6429d9b..fb6d1e87f7b4e9b 100644
--- a/clang/test/Frontend/Wno-everything.c
+++ b/clang/test/Frontend/Wno-everything.c
@@ -2,6 +2,7 @@
int f0(int, unsigned);
int f0(int x, unsigned y) {
- if (x=3);
- return x < y; // expected-warning {{comparison of integers}}
+ if (x = 3)
+ ;
+ return x < y; // expected-warning {{comparison of integers}} expected-note{{}}
}
diff --git a/clang/test/Frontend/warning-mapping-5.c b/clang/test/Frontend/warning-mapping-5.c
index 84efd8010d0fd12..bd2d920aee918d1 100644
--- a/clang/test/Frontend/warning-mapping-5.c
+++ b/clang/test/Frontend/warning-mapping-5.c
@@ -4,5 +4,5 @@
#pragma clang diagnostic warning "-Wsign-compare"
int f0(int x, unsigned y) {
- return x < y; // expected-warning {{comparison of integers}}
+ return x < y; // expected-warning {{comparison of integers}} expected-note{{}}
}
diff --git a/clang/test/Sema/sign-compare-enum.c b/clang/test/Sema/sign-compare-enum.c
index fb67e323f1d128e..36a706900417dca 100644
--- a/clang/test/Sema/sign-compare-enum.c
+++ b/clang/test/Sema/sign-compare-enum.c
@@ -22,25 +22,28 @@ int test_pos(enum PosEnum a) {
if (a < 2)
return 0;
- // No warning, except in Windows C mode, where PosEnum is 'int' and it can
- // take on any value according to the C standard.
+ // No warning, except in Windows C mode, where PosEnum is 'int' and it can
+ // take on any value according to the C standard.
#if !defined(SILENCE) && defined(_WIN32) && !defined(__cplusplus)
- // expected-warning at +2 {{comparison of integers of different signs}}
+ // expected-warning at +3 {{comparison of integers of different signs}}
+ // expected-note at +2{{}}
#endif
if (a < 2U)
return 0;
unsigned uv = 2;
#if !defined(SILENCE) && defined(_WIN32) && !defined(__cplusplus)
- // expected-warning at +2 {{comparison of integers of different signs}}
+ // expected-warning at +3 {{comparison of integers of different signs}}
+ // expected-note at +2{{}}
#endif
if (a < uv)
return 1;
#if !defined(SILENCE) && defined(_WIN32) && !defined(__cplusplus)
- // expected-warning at +2 {{comparison of integers of different signs}}
+ // expected-warning at +3 {{comparison of integers of different signs}}
+ // expected-note at +2{{}}
#endif
- if (a < sizeof(message)/sizeof(message[0]))
+ if (a < sizeof(message) / sizeof(message[0]))
return 0;
return 4;
}
@@ -56,22 +59,25 @@ int test_neg(enum NegEnum a) {
return 0;
#ifndef SILENCE
- // expected-warning at +2 {{comparison of integers of different signs}}
+ // expected-warning at +3 {{comparison of integers of different signs}}
+ // expected-note at +2{{}}
#endif
if (a < 2U)
return 0;
unsigned uv = 2;
#ifndef SILENCE
- // expected-warning at +2 {{comparison of integers of different signs}}
+ // expected-warning at +3 {{comparison of integers of different signs}}
+ // expected-note at +2{{}}
#endif
if (a < uv)
return 1;
#ifndef SILENCE
- // expected-warning at +2 {{comparison of integers of different signs}}
+ // expected-warning at +3 {{comparison of integers of different signs}}
+ // expected-note at +2{{}}
#endif
- if (a < sizeof(message)/sizeof(message[0]))
+ if (a < sizeof(message) / sizeof(message[0]))
return 0;
return 4;
}
More information about the cfe-commits
mailing list