Xazax-hun wrote:

Thanks for working on this! I did some thinking and I believe this is going in 
the right direction but it still matches on the AST, which is often not a good 
idea if we want full generosity.

Consider more complex examples like:
```
 int *p = cond ? q : ((cond2 ? throw 5 : throw 3), nullptr);
```

Here, if we check the false branch of the outer ternary, it won't be a single 
throw expression, yet we should not have a flow from the false branch to the 
result.

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. 

An alternative, cheaper (but less precise) thing to do could be:
* Get the CFG node for the full ternary and for each of the branches.
* Check if there is an edge between the CFG node from a branch to the ternary 
(and the branch has no noreturn node).
* Do the flow if that is the case.

This avoids matching the AST so it is a bit more robust, it avoids the 
expensive `isInevitablySinking` call but it does not cover all the complex 
scenarios.

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.

https://github.com/llvm/llvm-project/pull/190345
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to