rtrieu updated this revision to Diff 215021.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66044/new/
https://reviews.llvm.org/D66044
Files:
lib/Analysis/CFG.cpp
lib/Analysis/ReachableCode.cpp
test/Analysis/cfg.cpp
test/Sema/warn-overlap.c
test/Sema/warn-unreachable.c
test/SemaCXX/warn-unreachable.cpp
Index: test/SemaCXX/warn-unreachable.cpp
===================================================================
--- test/SemaCXX/warn-unreachable.cpp
+++ test/SemaCXX/warn-unreachable.cpp
@@ -393,6 +393,9 @@
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 @@
else
calledFun();
- if (y == -1 && y != -1)
- calledFun();
}
Index: test/Sema/warn-unreachable.c
===================================================================
--- test/Sema/warn-unreachable.c
+++ test/Sema/warn-unreachable.c
@@ -433,7 +433,7 @@
}
void unaryOpNoFixit() {
- if (- 1)
+ if (~ 1)
return; // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
unaryOpNoFixit(); // expected-warning {{code will never be executed}}
}
Index: test/Sema/warn-overlap.c
===================================================================
--- test/Sema/warn-overlap.c
+++ test/Sema/warn-overlap.c
@@ -141,3 +141,20 @@
return x < 1 || x != 0;
// expected-warning@-1{{overlapping comparisons always evaluate to true}}
}
+
+int integer_conversion(unsigned x, int y) {
+ return x > 4 || x < 10;
+ // expected-warning@-1{{overlapping comparisons always evaluate to true}}
+ return y > 4u || y < 10u;
+ // expected-warning@-1{{overlapping comparisons always evaluate to true}}
+}
+
+int negative_compare(int x) {
+ return x > -1 || x < 1;
+ // expected-warning@-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.
+}
Index: test/Analysis/cfg.cpp
===================================================================
--- test/Analysis/cfg.cpp
+++ test/Analysis/cfg.cpp
@@ -498,6 +498,26 @@
}
} // end namespace pr37688_deleted_union_destructor
+// 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)]
@@ -522,4 +542,3 @@
void PR18472_helper() {
PR18472<int>();
}
-
Index: lib/Analysis/ReachableCode.cpp
===================================================================
--- lib/Analysis/ReachableCode.cpp
+++ lib/Analysis/ReachableCode.cpp
@@ -247,7 +247,7 @@
}
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();
Index: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -70,11 +70,35 @@
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 @@
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
@@ -1077,6 +1101,10 @@
// * 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);
@@ -1092,10 +1120,16 @@
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);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
