[PATCH] [analyzer] Refactor conditional expression evaluating code

[Jordan Rose]

  This is looking a lot more reasonable now! Still a few comments, and I'd like to make sure Ted gives a final okay before commit, but I'm a lot more confident in this code than I was a few iterations ago. Thanks, Pavel.

  Oh, and nitpick: new function names should follow the LLVM convention to start with a lowercase letter, even though there's a lot of analyzer code that's the other way.


================
Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:504-509
@@ -503,1 +503,8 @@
 
+static bool GetTruthValue(SVal X) {
+  // Value should be either a nonloc::ConcreteInt or a loc::ConcreteInt
+  if (Optional<nonloc::ConcreteInt> Val = X.getAs<nonloc::ConcreteInt>())
+    return Val->getValue() != 0;
+  return X.castAs<loc::ConcreteInt>().getValue() != 0;
+}
+
----------------
This can be replaced with "assert(X.isConstant()); return X.isZeroConstant();".

================
Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:553-556
@@ -567,1 +552,6 @@
+    if (StTrue) {
+      if (!StFalse) {
+        // The value is known to be true.
+        X = getSValBuilder().makeIntVal(1, B->getType());
       }
+    } else {
----------------
Please include something about the StTrue && StFalse case here, just to make it clear you didn't forget it.

================
Comment at: lib/Analysis/LiveVariables.cpp:381
@@ +380,3 @@
+
+void TransferFunctions::MarkLogicalExprLeafs(const Expr *E) {
+  const BinaryOperator *B = dyn_cast<BinaryOperator>(E);
----------------
Usual English plural is "leaves", even when applying to a tree data structure instead of a real tree.

================
Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:534
@@ +533,3 @@
+  SValBuilder &SVB = State->getStateManager().getSValBuilder();
+  return State->BindExpr(E, LC, SVB.evalCast(X, B->getType(), XType));
+}
----------------
Rather than have this bind anything, why not just return the properly-casted value?


http://llvm-reviews.chandlerc.com/D1340