[PATCH] D146514: [clang][dataflow] Fix crash when RHS of `&&` or `||` calls `noreturn` func.

[Martin Böhme via Phabricator via cfe-commits]

mboehme added a comment.

In D146514#4209931 <https://reviews.llvm.org/D146514#4209931>, @xazax.hun wrote:

> This fix looks good for me for this particular problem, but I wonder whether the solution is general enough. In case the analysis figures out that a call would not return (e.g., the `value` method is called on a provably empty optional, and it would throw instead of returning), would this approach still work? Would the analysis update the `BlockReachable` bitvector on demand?

No, I think this is a different case.

`BlockReachable` is intended to encode reachability solely as defined by the structure of the CFG. The whole problem we're trying to solve in this patch is that the CFG understands that `noreturn` functions don't return and therefore eliminates any successor edges from calls to `noreturn` functions. This means that some of the CFG nodes become unreachable; specifically, in our case, the RHS of the `&&` and `||` become unreachable, and so the dataflow analysis never computes a value for them. We therefore need to make sure that we don't try to query the value of the RHS in this case.

The case of calling `value` on a provably empty optional is different: The CFG does not understand that such a call does not return and therefore generates successor edges as it would for any other function call.

A side effect of the logic that this patch adds is that we get some neat inference results, as demonstrated in the newly added test. However, generating these results isn't the main goal of this patch; the patch is merely intended to ensure that we can process CFGs containing unreachable nodes caused by `noreturn` functions.

It would certainly be nice to generate similar inference results in the example that you mention (`value` on a provably empty optional), but that would require more work and a different approach.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146514/new/

https://reviews.llvm.org/D146514