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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to