[clang] [clang][dataflow] Discard unneeded `ExprToLoc` and `ExprToVal` entries. (PR #72850)

[via cfe-commits]

martinboehme wrote:

I'm intentionally not responding to comments in the code for now -- wanted to wrap up this high-level discussion first.

> Overall seemed good (mostly just piping), but I think we need more explanation (on the review thread and somewhere appropriate in the code) of what exactly determines whether an expression is "needed".
> 
> I was wondering, when reading the code, why _any_ expression is needed once we've finished processing the block. My recollection is that ternary expressions are responsible for this, but in that case shouldn't we be looking at them directly? If not, it seems worth explaining.

Yes, it's the ternaries that cause us to need to access an expression after we've finished processing the block. Here's a simple example:

https://godbolt.org/z/Ko7K86GKW

Essentially, this comes up whenever we have control flow within a *full-expression*. I believe the only cases where this can happen is for the ternary / conditional operator and for the short-circuiting logical operators.

The short-circuiting logical operators already have special treatment in the dataflow framework to retrieve their operands from the environment for the block in which those operands are computed (see also the comments added in this patch). So this leaves only the conditional operator as the other case.

The reason I went with a more conservative approach in this patch are:

*  I wasn't sure whether there was maybe some other case I wasn't thinking of.
* The approach of checking whether an expression is "consumed" by a block seemed more generic than singling out the conditional operator.

But maybe the right thing to do instead would be to be more bold:

*  Treat the operands of the conditional operator in the same way that we already treat the operands of the short-circuiting logical operators (see above), i.e. retrieve them from the environment for the block in which those operands are computed.
*  Clear out `ExprToLoc` and `ExprToVal` entirely when starting a new block.

This would be less complex to implement than what I've got in this patch, and it would require less computation. The main question mark I see is whether there's any cases I've overlooked where control flow happens within a full expression. But if we can't think of them, then presumably they're rare, and they should be easy to fix if we discover them?

So I think I'm leaning towards implementing this simpler, but bolder approach. Would appreciate input.

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