[clang] [alpha.webkit.UncountedLocalVarsChecker] Allow uncounted object references within trivial statements (PR #82229)

[Artem Dergachev via cfe-commits]

================
@@ -474,4 +504,22 @@ bool TrivialFunctionAnalysis::isTrivialImpl(
   return Result;
 }
 
+bool TrivialFunctionAnalysis::isTrivialImpl(
+    const Stmt *S, TrivialFunctionAnalysis::CacheTy &Cache) {
+  // If the statement isn't in the cache, conservatively assume that
+  // it's not trivial until analysis completes. Unlike a function case,
+  // we don't insert an entry into the cache until Visit returns
+  // since Visit* functions themselves make use of the cache.
----------------
haoNoQ wrote:

>  ```
> // If the statement isn't in the cache, conservatively assume that
> // it's not trivial until analysis completes.
> ```

This comment should probably be moved to `WithCachedResult()` where the false-by-default insertion actually takes place. (Not sure if it was clear that my original comment was talking about that insertion specifically. If it wasn't clear, it was probably a bad comment that needed to be phrased differently in the other function as well.)

> ```
> //                                            Unlike a function case,
> // we don't insert an entry into the cache until Visit returns
> // since Visit* functions themselves make use of the cache.
> ```

We can probably avoid double insert/lookup by skipping this insert/lookup entirely. Just rely on `Visit()` to do the bookkeeping.

But at the same time, it's nice that your code guarantees *some* caching even when we forget to cache a specific statement. It's a property we'd lose if we simply remove this global insert-lookup.

So I wonder if you can replace it with an *assertion* instead? Something like:
```
bool Result = V.Visit(S);
assert(Cache.contains(S) && "Top-level statement not properly cached!");
return Result;
```
This may actively help us find forgotten `WithCachedResult()` calls instead of silently accepting them.

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