[clang] [clang][dataflow] Model conditional operator correctly. (PR #89213)

[via cfe-commits]

================
@@ -657,17 +658,22 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
   }
 
   void VisitConditionalOperator(const ConditionalOperator *S) {
-    // FIXME: Revisit this once flow conditions are added to the framework. For
-    // `a = b ? c : d` we can add `b => a == c && !b => a == d` to the flow
-    // condition.
-    // When we do this, we will need to retrieve the values of the operands from
-    // the environments for the basic blocks they are computed in, in a similar
-    // way to how this is done for short-circuited logical operators in
-    // `getLogicOperatorSubExprValue()`.
-    if (S->isGLValue())
-      Env.setStorageLocation(*S, Env.createObject(S->getType()));
-    else if (!S->getType()->isRecordType()) {
-      if (Value *Val = Env.createValue(S->getType()))
+    const Environment *TrueEnv = StmtToEnv.getEnvironment(*S->getTrueExpr());
+    const Environment *FalseEnv = StmtToEnv.getEnvironment(*S->getFalseExpr());
+
+    if (TrueEnv == nullptr || FalseEnv == nullptr)
+      return;
+
+    if (S->isGLValue()) {
+      StorageLocation *TrueLoc = TrueEnv->getStorageLocation(*S->getTrueExpr());
+      StorageLocation *FalseLoc =
+          FalseEnv->getStorageLocation(*S->getFalseExpr());
+      if (TrueLoc == FalseLoc && TrueLoc != nullptr)
+        Env.setStorageLocation(*S, *TrueLoc);
+    } else if (!S->getType()->isRecordType()) {
+      if (Value *Val = Environment::joinValues(
----------------
martinboehme wrote:

> it might be worth commenting why we need a join here. IIUC, we lack any concept of a "result expression" of a basic block.

Just to make sure I understand: The idea of such a concept would be that each of the branches of the conditional operator sets its result expression, and then when we perform the join for those two branches, we join the values of the result expressions?

This would work for the case of the conditional operator -- but it doesn't work for `&&` and `||`, where we also have two basic blocks with "result expressions", but at the place where the branches join, we don't want to join the result expressions but rather combine them using logical "and" or "or".

So I don't think that a concept like this would add much of value.

> So, when the environments were joined in the normal basic-block processing algorithm, these two expressions would have been simply dropped (existing at respectively different locations in LocToVal).

(I think you mean `ExprToVal`?)

Yes, in the joined environment these values don't exist -- so we have to extract them from the environments for the two branches (which is exactly what we do for `&&` and `||` too). We then combine the values using a join because that's the operation that happens to be appropriate for the conditional operator (whereas for the logical operators we combine them using a logical operation).

I've added a short comment that hopefully makes this a bit clearer -- WDYT?


https://github.com/llvm/llvm-project/pull/89213