r372448 - Improve -Wtautological-overlap-compare
Richard Trieu via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 20 19:37:11 PDT 2019
Author: rtrieu
Date: Fri Sep 20 19:37:10 2019
New Revision: 372448
URL: http://llvm.org/viewvc/llvm-project?rev=372448&view=rev
Log:
Improve -Wtautological-overlap-compare
Allow this warning to detect a larger number of constant values, including
negative numbers, and handle non-int types better.
Differential Revision: https://reviews.llvm.org/D66044
Modified:
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/lib/Analysis/ReachableCode.cpp
cfe/trunk/test/Analysis/cfg.cpp
cfe/trunk/test/Sema/warn-overlap.c
cfe/trunk/test/Sema/warn-unreachable.c
cfe/trunk/test/SemaCXX/warn-unreachable.cpp
Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=372448&r1=372447&r2=372448&view=diff
==============================================================================
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Fri Sep 20 19:37:10 2019
@@ -51,7 +51,8 @@ Major New Features
Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-- ...
+- -Wtautological-overlap-compare will warn on negative numbers and non-int
+ types.
Non-comprehensive list of changes in this release
-------------------------------------------------
Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=372448&r1=372447&r2=372448&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Fri Sep 20 19:37:10 2019
@@ -70,11 +70,35 @@ static SourceLocation GetEndLoc(Decl *D)
return D->getLocation();
}
+/// Returns true on constant values based around a single IntegerLiteral.
+/// Allow for use of parentheses, integer casts, and negative signs.
+static bool IsIntegerLiteralConstantExpr(const Expr *E) {
+ // Allow parentheses
+ E = E->IgnoreParens();
+
+ // Allow conversions to different integer kind.
+ if (const auto *CE = dyn_cast<CastExpr>(E)) {
+ if (CE->getCastKind() != CK_IntegralCast)
+ return false;
+ E = CE->getSubExpr();
+ }
+
+ // Allow negative numbers.
+ if (const auto *UO = dyn_cast<UnaryOperator>(E)) {
+ if (UO->getOpcode() != UO_Minus)
+ return false;
+ E = UO->getSubExpr();
+ }
+
+ return isa<IntegerLiteral>(E);
+}
+
/// Helper for tryNormalizeBinaryOperator. Attempts to extract an IntegerLiteral
-/// or EnumConstantDecl from the given Expr. If it fails, returns nullptr.
+/// constant expression or EnumConstantDecl from the given Expr. If it fails,
+/// returns nullptr.
static const Expr *tryTransformToIntOrEnumConstant(const Expr *E) {
E = E->IgnoreParens();
- if (isa<IntegerLiteral>(E))
+ if (IsIntegerLiteralConstantExpr(E))
return E;
if (auto *DR = dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts()))
return isa<EnumConstantDecl>(DR->getDecl()) ? DR : nullptr;
@@ -121,11 +145,11 @@ tryNormalizeBinaryOperator(const BinaryO
static bool areExprTypesCompatible(const Expr *E1, const Expr *E2) {
// User intent isn't clear if they're mixing int literals with enum
// constants.
- if (isa<IntegerLiteral>(E1) != isa<IntegerLiteral>(E2))
+ if (isa<DeclRefExpr>(E1) != isa<DeclRefExpr>(E2))
return false;
// Integer literal comparisons, regardless of literal type, are acceptable.
- if (isa<IntegerLiteral>(E1))
+ if (!isa<DeclRefExpr>(E1))
return true;
// IntegerLiterals are handled above and only EnumConstantDecls are expected
@@ -1081,6 +1105,10 @@ private:
// * Variable x is equal to the largest literal.
// * Variable x is greater than largest literal.
bool AlwaysTrue = true, AlwaysFalse = true;
+ // Track value of both subexpressions. If either side is always
+ // true/false, another warning should have already been emitted.
+ bool LHSAlwaysTrue = true, LHSAlwaysFalse = true;
+ bool RHSAlwaysTrue = true, RHSAlwaysFalse = true;
for (const llvm::APSInt &Value : Values) {
TryResult Res1, Res2;
Res1 = analyzeLogicOperatorCondition(BO1, Value, L1);
@@ -1096,10 +1124,16 @@ private:
AlwaysTrue &= (Res1.isTrue() || Res2.isTrue());
AlwaysFalse &= !(Res1.isTrue() || Res2.isTrue());
}
+
+ LHSAlwaysTrue &= Res1.isTrue();
+ LHSAlwaysFalse &= Res1.isFalse();
+ RHSAlwaysTrue &= Res2.isTrue();
+ RHSAlwaysFalse &= Res2.isFalse();
}
if (AlwaysTrue || AlwaysFalse) {
- if (BuildOpts.Observer)
+ if (!LHSAlwaysTrue && !LHSAlwaysFalse && !RHSAlwaysTrue &&
+ !RHSAlwaysFalse && BuildOpts.Observer)
BuildOpts.Observer->compareAlwaysTrue(B, AlwaysTrue);
return TryResult(AlwaysTrue);
}
Modified: cfe/trunk/lib/Analysis/ReachableCode.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ReachableCode.cpp?rev=372448&r1=372447&r2=372448&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ReachableCode.cpp (original)
+++ cfe/trunk/lib/Analysis/ReachableCode.cpp Fri Sep 20 19:37:10 2019
@@ -247,7 +247,7 @@ static bool isConfigurationValue(const S
}
case Stmt::UnaryOperatorClass: {
const UnaryOperator *UO = cast<UnaryOperator>(S);
- if (UO->getOpcode() != UO_LNot)
+ if (UO->getOpcode() != UO_LNot && UO->getOpcode() != UO_Minus)
return false;
bool SilenceableCondValNotSet =
SilenceableCondVal && SilenceableCondVal->getBegin().isInvalid();
Modified: cfe/trunk/test/Analysis/cfg.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cfg.cpp?rev=372448&r1=372447&r2=372448&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/cfg.cpp (original)
+++ cfe/trunk/test/Analysis/cfg.cpp Fri Sep 20 19:37:10 2019
@@ -547,6 +547,27 @@ int foo() {
}
} // namespace statement_expression_in_return
+// CHECK-LABEL: int overlap_compare(int x)
+// CHECK: [B2]
+// CHECK-NEXT: 1: 1
+// CHECK-NEXT: 2: return [B2.1];
+// CHECK-NEXT: Preds (1): B3(Unreachable)
+// CHECK-NEXT: Succs (1): B0
+// CHECK: [B3]
+// CHECK-NEXT: 1: x
+// CHECK-NEXT: 2: [B3.1] (ImplicitCastExpr, LValueToRValue, int)
+// CHECK-NEXT: 3: 5
+// CHECK-NEXT: 4: [B3.2] > [B3.3]
+// CHECK-NEXT: T: if [B4.5] && [B3.4]
+// CHECK-NEXT: Preds (1): B4
+// CHECK-NEXT: Succs (2): B2(Unreachable) B1
+int overlap_compare(int x) {
+ if (x == -1 && x > 5)
+ return 1;
+
+ return 2;
+}
+
// CHECK-LABEL: template<> int *PR18472<int>()
// CHECK: [B2 (ENTRY)]
// CHECK-NEXT: Succs (1): B1
Modified: cfe/trunk/test/Sema/warn-overlap.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-overlap.c?rev=372448&r1=372447&r2=372448&view=diff
==============================================================================
--- cfe/trunk/test/Sema/warn-overlap.c (original)
+++ cfe/trunk/test/Sema/warn-overlap.c Fri Sep 20 19:37:10 2019
@@ -141,3 +141,20 @@ int returns(int x) {
return x < 1 || x != 0;
// expected-warning at -1{{overlapping comparisons always evaluate to true}}
}
+
+int integer_conversion(unsigned x, int y) {
+ return x > 4 || x < 10;
+ // expected-warning at -1{{overlapping comparisons always evaluate to true}}
+ return y > 4u || y < 10u;
+ // expected-warning at -1{{overlapping comparisons always evaluate to true}}
+}
+
+int negative_compare(int x) {
+ return x > -1 || x < 1;
+ // expected-warning at -1{{overlapping comparisons always evaluate to true}}
+}
+
+int no_warning(unsigned x) {
+ return x >= 0 || x == 1;
+ // no warning since "x >= 0" is caught by a different tautological warning.
+}
Modified: cfe/trunk/test/Sema/warn-unreachable.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-unreachable.c?rev=372448&r1=372447&r2=372448&view=diff
==============================================================================
--- cfe/trunk/test/Sema/warn-unreachable.c (original)
+++ cfe/trunk/test/Sema/warn-unreachable.c Fri Sep 20 19:37:10 2019
@@ -433,7 +433,7 @@ void wrapOneInFixit(struct StructWithPoi
}
void unaryOpNoFixit() {
- if (- 1)
+ if (~ 1)
return; // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
unaryOpNoFixit(); // expected-warning {{code will never be executed}}
}
Modified: cfe/trunk/test/SemaCXX/warn-unreachable.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unreachable.cpp?rev=372448&r1=372447&r2=372448&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-unreachable.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-unreachable.cpp Fri Sep 20 19:37:10 2019
@@ -393,6 +393,9 @@ void tautological_compare(bool x, int y)
else
calledFun(); // expected-warning {{will never be executed}}
+ if (y == -1 && y != -1) // expected-note {{silence}}
+ calledFun(); // expected-warning {{will never be executed}}
+
// TODO: Extend warning to the following code:
if (x < -1)
calledFun();
@@ -408,6 +411,4 @@ void tautological_compare(bool x, int y)
else
calledFun();
- if (y == -1 && y != -1)
- calledFun();
}
More information about the cfe-commits
mailing list