Skip to content

Commit 773e88f

Browse files
authored
[Clang] Force expressions with UO_Not to not be non-negative (#126846)
This PR addresses the bug of not throwing warnings for the following code: ```c++ int test13(unsigned a, int *b) { return a > ~(95 != *b); // expected-warning {{comparison of integers of different signs}} } ``` However, in the original issue, a comment mentioned that negation, pre-increment, and pre-decrement operators are also incorrect in this case. Fixes #18878
1 parent a1a6a83 commit 773e88f

File tree

3 files changed

+103
-2
lines changed

3 files changed

+103
-2
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,9 @@ Improvements to Clang's diagnostics
238238
as function arguments or return value respectively. Note that
239239
:doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The
240240
feature will be default-enabled with ``-Wthread-safety`` in a future release.
241+
- The ``-Wsign-compare`` warning now treats expressions with bitwise not(~) and minus(-) as signed integers
242+
except for the case where the operand is an unsigned integer
243+
and throws warning if they are compared with unsigned integers (##18878).
241244

242245
- Improve the diagnostics for chained comparisons to report actual expressions and operators (#GH129069).
243246

clang/lib/Sema/SemaChecking.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10620,6 +10620,42 @@ static std::optional<IntRange> TryGetExprRange(ASTContext &C, const Expr *E,
1062010620
case UO_AddrOf: // should be impossible
1062110621
return IntRange::forValueOfType(C, GetExprType(E));
1062210622

10623+
case UO_Minus: {
10624+
if (E->getType()->isUnsignedIntegerType()) {
10625+
return TryGetExprRange(C, UO->getSubExpr(), MaxWidth, InConstantContext,
10626+
Approximate);
10627+
}
10628+
10629+
std::optional<IntRange> SubRange = TryGetExprRange(
10630+
C, UO->getSubExpr(), MaxWidth, InConstantContext, Approximate);
10631+
10632+
if (!SubRange)
10633+
return std::nullopt;
10634+
10635+
// If the range was previously non-negative, we need an extra bit for the
10636+
// sign bit. If the range was not non-negative, we need an extra bit
10637+
// because the negation of the most-negative value is one bit wider than
10638+
// that value.
10639+
return IntRange(SubRange->Width + 1, false);
10640+
}
10641+
10642+
case UO_Not: {
10643+
if (E->getType()->isUnsignedIntegerType()) {
10644+
return TryGetExprRange(C, UO->getSubExpr(), MaxWidth, InConstantContext,
10645+
Approximate);
10646+
}
10647+
10648+
std::optional<IntRange> SubRange = TryGetExprRange(
10649+
C, UO->getSubExpr(), MaxWidth, InConstantContext, Approximate);
10650+
10651+
if (!SubRange)
10652+
return std::nullopt;
10653+
10654+
// The width increments by 1 if the sub-expression cannot be negative
10655+
// since it now can be.
10656+
return IntRange(SubRange->Width + (int)SubRange->NonNegative, false);
10657+
}
10658+
1062310659
default:
1062410660
return TryGetExprRange(C, UO->getSubExpr(), MaxWidth, InConstantContext,
1062510661
Approximate);

clang/test/Sema/compare.c

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare -Wtautological-constant-in-range-compare %s -Wno-unreachable-code
2-
// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare -Wtype-limits %s -Wno-unreachable-code
1+
// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare -Wtautological-constant-in-range-compare %s -Wno-unreachable-code -DTEST=1
2+
// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare -Wtype-limits %s -Wno-unreachable-code -DTEST=2
33

44
int test(char *C) { // nothing here should warn.
55
return C != ((void*)0);
@@ -419,3 +419,65 @@ void pr36008(enum PR36008EnumTest lhs) {
419419
if (x == y) x = y; // no warning
420420
if (y == x) y = x; // no warning
421421
}
422+
423+
int test13(unsigned a, int b) {
424+
return a > ~(95 != b); // expected-warning {{comparison of integers of different signs}}
425+
}
426+
427+
int test14(unsigned a, unsigned b) {
428+
return a > ~b; // no-warning
429+
}
430+
431+
int test15(unsigned a, int b) {
432+
return a > -(95 != b); // expected-warning {{comparison of integers of different signs}}
433+
}
434+
435+
int test16(unsigned a, unsigned b) {
436+
return a > -b; // no-warning
437+
}
438+
439+
int test17(int a, unsigned b) {
440+
return a > -(-b); // expected-warning {{comparison of integers of different signs}}
441+
}
442+
443+
int test18(int a) {
444+
return a == -(-2147483648); // expected-warning {{result of comparison of constant 2147483648 with expression of type 'int' is always false}}
445+
}
446+
447+
int test19(int n) {
448+
return -(n & 15) <= -15; // no-warning
449+
}
450+
451+
#if TEST == 1
452+
int test20(int n) {
453+
return -(n & 15) <= -17; // expected-warning {{result of comparison of 5-bit signed value <= -17 is always false}}
454+
}
455+
#endif
456+
457+
int test21(short n) {
458+
return -n == 32768; // no-warning
459+
}
460+
461+
#if TEST == 1
462+
int test22(short n) {
463+
return -n == 65536; // expected-warning {{result of comparison of 17-bit signed value == 65536 is always false}}
464+
}
465+
#endif
466+
467+
int test23(unsigned short n) {
468+
return ~n == 32768; // no-warning
469+
}
470+
471+
int test24(short n) {
472+
return ~n == 32767; // no-warning
473+
}
474+
475+
#if TEST == 1
476+
int test25(unsigned short n) {
477+
return ~n == 65536; // expected-warning {{result of comparison of 17-bit signed value == 65536 is always false}}
478+
}
479+
480+
int test26(short n) {
481+
return ~n == 32768; // expected-warning {{result of comparison of 16-bit signed value == 32768 is always false}}
482+
}
483+
#endif

0 commit comments

Comments
 (0)