[clang] [LifetimeSafety] Fix crash on ternary operator when one of the expressions is `throw` (PR #190345)

[Utkarsh Saxena via cfe-commits]

usx95 wrote:

> We should not have flow from r to p but we do. Why? Because r is a subexpression of the ternary and the fact generation will only look at the subexpressions of the ternary without consulting the CFG whether we actually have an edge between those AST nodes in the control flow.

Thanks for clarification. Sounds good to me then.

> I'm wondering if the cleanest solution would be something like:
> 
> * Get the Cfg node corresponding to evaluating the condition. The successors of this node are the branches.
> * Check if those branches are sink nodes, as in they never reach the end of the function call. You could use `isInevitablySinking` and do the flow only if this API returns false.
> 
> I think this might handle all of the complex cases more robustly.
> 
> That being said, `isInevitablySinking` looks like a relatively expensive API to call and calling it on multiple disparate CFGNodes seem to do some redundant work as the subtasks for the commonly reachable nodes are not cached. So I am not 100% sure if we actually need this precision at the cost of too much computation.

> The best solution might be to make `isInevitablySinking` a bit cheaper and use it for the flow but that might be a bit more work.

I like this. While we are at it, I think it might be worth pursuing. It looks like that it would be relatively simpler to introduce a cache for the DFS in `isInevitablySinking`, e.g. a 2-bit `SinkingCache` in `CFGBlock` to memoize the reachability results (uncomputed, sinking, or not sinking). If there is any successor which is not-sinking, current block is not sinking.


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