xazax.hun added inline comments.
================ Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:516 + // assigned to it. + Visit(&SubExpr); + if (auto *Val = dyn_cast_or_null<BoolValue>( ---------------- sgatev wrote: > xazax.hun wrote: > > Could you elaborate on when would this happen? I'd expect the traversal to > > always visit the predecessor basic blocks first and within a basic block > > always visit subexpressions first. So I'd be quite surprised if there is a > > subexpression we did not visit. > From what I've seen, logic operators influence the structure of the CFG > through additional basic blocks and terminators, but their sub-expression > operators are not added directly in the basic blocks. > > For example: > ``` > void test(bool a, bool b, bool c) { > bool d = a && (b || c); > } > ``` > > results in: > ``` > > void test(bool a, bool b, bool c) > [B5 (ENTRY)] > Succs (1): B4 > > [B1] > 1: [B4.2] && ([B3.2] || [B2.2]) > 2: bool d = a && (b || c); > Preds (3): B2 B3 B4 > Succs (1): B0 > > [B2] > 1: c > 2: [B2.1] (ImplicitCastExpr, LValueToRValue, _Bool) > Preds (1): B3 > Succs (1): B1 > > [B3] > 1: b > 2: [B3.1] (ImplicitCastExpr, LValueToRValue, _Bool) > T: [B3.2] || ... > Preds (1): B4 > Succs (2): B1 B2 > > [B4] > 1: a > 2: [B4.1] (ImplicitCastExpr, LValueToRValue, _Bool) > T: [B4.2] && ... > Preds (1): B5 > Succs (2): B3 B1 > > [B0 (EXIT)] > Preds (1): B1 > ``` > > So, when we evaluate `a && (b || c)` in `B1`, the sub-expression `b || c` has > not been evaluated yet. I updated the comment in the code to make that more > clear. I understand the structure of the CFG and also understand that certain subexpressions are in different basic blocks. But I still don't understand why would `b || c` be not evaluated when we evaluate `a && (b || c)`. The operator `&&` in your example is evaluated in `B1`. The operator `||` is evaluated in `B3`. `B3` is a predecessor of `B1`, so if we process the CFG in reverse-post order, we should visit `B3` before `B1`. I am pretty sure if the traversal is well-written we should have the `||` evaluated before we visit `B1`. I suspect that something different might be going on. Is it possible that you want to evaluate the `&&` in `B4`? Note that `&&` is a terminator there because of the short-circuiting. So at that point we should NOT ask for the value of `||`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121455/new/ https://reviews.llvm.org/D121455 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits