[clang] [NFC][analyzer] Spread use of 'Expr*' instead of 'Stmt*' (PR #188319)

[Donát Nagy via cfe-commits]

NagyDonat wrote:

@steakhal Thanks for the warning!

> The engine gives (or used to give) some special treatment to objective C loops and bound SVal to the loop statement. If I recall Husi tried to get rid of them exactly for the same reason you are making this change.

The hack related to objective-C `for` loops was eliminated by Husi in commit https://github.com/llvm/llvm-project/commit/b9bca883c970d36f408db80df21838c713c326db (in 2020). By the way, @Szelethus I'm very grateful for this patch, it did the "heavy lifting" for this cleanup :sweat_smile: 

In the commit message of that commit Husi writes that:
> Turns out, we make an exception for ReturnStmt (which we'll leave for another time) and ObjCForCollectionStmt.

and he also verifies this with the assertion
```c++
assert((isa<Expr, ReturnStmt>(S)) &&
       "Environment can only argue about Exprs, since only they express "
       "a value! Any non-expression statement stored in Environment is a "
       "result of a hack!");
```
in `Environment::getSVal`. 

However, it turns out that the `ReturnStmt` hack is mild and shallow compared to the `ObjCForCollectionStmt` hack: `Environment::getSVal` accepts an `EnvironmentEntry` that holds a `ReturnStmt` (and there is indeed a call in `ExprEngine::processCallExit` that passes an `EnvironmentEntry` holding a `ReturnStmt`), but this case is handled by the block
```c++
case Stmt::ReturnStmtClass: {                               
  const auto *RS = cast<ReturnStmt>(S);                     
  if (const Expr *RE = RS->getRetValue())                   
    return getSVal(EnvironmentEntry(RE, LCtx), svalBuilder);
  return UndefinedVal();                                    
}                                                           
```
which looks up and returns the value bound to the `RetValue` subexpression of the return statement.

The value is bound to this subexpression by the natural "bind value to each evaluated expression" process; there is no code that actually binds values to `ReturnStmt`s in the `Environment`. (Unlike the `ObjCForCollectionStmt` hack where the environment actually contained entries that bound value to the loop statement.)

In fact this behavior is unchanged since commit https://github.com/llvm/llvm-project/commit/3f406ba4bf6090d26bf59e20c9536e81bc9e01b6 (from 2012) which introduced this ill-advised and confusing "let's look up the return statement in the `Environment`" hack, so I'm pretty sure that there was never any code that actually bound values to return statements in the environment.

Based on this I'm still pretty confident that this PR is correct (there is no code that actually binds values to non-expression statements). However, it is very useful that you highlighted this, because I will need to take this into consideration when I clean up the "other side" (_looking up_ non-expression statements). 


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