jordan_rose added a comment.
I guess the regular pings didn't work, so it was worth trying the gentle one?
Sorry!
This seems mostly ready to me, but I still have a few comments.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1646-1650
@@ -1644,2 +1645,7 @@
DefinedOrUnknownSVal CondV = CondV_untested.castAs<DefinedOrUnknownSVal>();
+ if (CondV.isUnknown()) {
+ CondV = state->getStateManager().getSValBuilder().conjureSymbolVal(nullptr,
+ CondE, LCtx, builder.getBuilderContext()->blockCount());
+ state = state->BindExpr(CondE, LCtx, CondV);
+ }
----------------
What's the purpose of this? As I understand it, a switch condition value will
never be reused in later states, so there's no point in adding constraints to
it unless it's already symbolic.
(And not bothering to do this would remove the need to pass the
NodeBuilderContext through.)
================
Comment at: lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp:199-200
@@ +198,4 @@
+ // Just add the constraint to the expression without trying to simplify.
+ SymbolRef Sym = Value.getAsSymExpr();
+ return assumeSymWithinInclusiveRange(State, Sym, From, To, InRange);
+ }
----------------
Will this ever return a null symbol? Maybe add an assertion?
================
Comment at: lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp:215-220
@@ +214,8 @@
+ return assumeSymWithinInclusiveRange(State, Sym, From, To, InRange);
+ return State;
+ } // end switch
+
+ case nonloc::ConcreteIntKind: {
+ const llvm::APSInt &IntVal =
Value.castAs<nonloc::ConcreteInt>().getValue();
+ bool IsInRange = IntVal >= From && IntVal <= To;
+ bool isFeasible = (IsInRange == InRange);
----------------
This is still relevant.
================
Comment at: test/Analysis/switch-case.c:1
@@ +1,2 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.ExprInspection -verify %s
+
----------------
All tests should include the "core" checkers. At some point we'll probably make
that implicit, but for now please add that to the -analyzer-checker list.
================
Comment at: test/Analysis/switch-case.c:24-25
@@ +23,4 @@
+ case 3 ... 10:
+ clang_analyzer_eval(t > 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(t + 2 <= 11); // expected-warning{{TRUE}}
+ clang_analyzer_eval(t + 1 == 3); // expected-warning{{UNKNOWN}}
----------------
Can you include at least one more check here: `clang_analyzer_eval(t > 2)`?
(Which should be unknown.)
================
Comment at: test/Analysis/switch-case.c:122
@@ +121,3 @@
+
+void testDefaultBrachRange(int arg) {
+ switch (arg) {
----------------
Typo: "Brach"
http://reviews.llvm.org/D5102
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
