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

[Gábor Horváth 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(
----------------
Xazax-hun wrote:

Thanks for the explanation, now this makes perfect sense. I think this is a sign that our CFGs are not the best abstraction for dataflow analysis in their current form, and we might be able to come up with a better representation for ternaries that would not have this problem (but that representation would probably be a departure from the AST). 

That being said, I think this is not too bad to explain. I am happy with the PR now. I wonder if it is worth to extend the comment why the join here is inherently part of the transfer in case it is possible to do it in a couple of concise sentences. If that is not the case, I am ok with merging as is.  

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