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

[Artem Dergachev via cfe-commits]

================
@@ -305,19 +337,21 @@ class TrivialFunctionAnalysisVisitor
   }
 
   bool VisitDeclRefExpr(const DeclRefExpr *DRE) {
-    if (auto *decl = DRE->getDecl()) {
-      if (isa<ParmVarDecl>(decl))
-        return true;
-      if (isa<EnumConstantDecl>(decl))
-        return true;
-      if (auto *VD = dyn_cast<VarDecl>(decl)) {
-        if (VD->hasConstantInitialization() && VD->getEvaluatedValue())
+    return WithCachedResult(DRE, [&]() {
+      if (auto *decl = DRE->getDecl()) {
+        if (isa<ParmVarDecl>(decl))
           return true;
-        auto *Init = VD->getInit();
-        return !Init || Visit(Init);
+        if (isa<EnumConstantDecl>(decl))
+          return true;
+        if (auto *VD = dyn_cast<VarDecl>(decl)) {
+          if (VD->hasConstantInitialization() && VD->getEvaluatedValue())
+            return true;
+          auto *Init = VD->getInit();
+          return !Init || Visit(Init);
----------------
haoNoQ wrote:

> Actually, we need to cache the results for `VisitDeclRefExpr` as well to avoid infinite recursion.

Wait, hmmm, this looks unusual. Why are we visiting the initializer whenever the variable is *accessed*? Isn't it sufficient to visit the initializer whenever the object is *initialized*? The initializer isn't evaluated at use site and it doesn't necessarily represent the contents of the variable anymore. Why do we care what the variable contained "initially"?

It's good that we aren't doing it *every* time the variable is used (so we can treat it as an unrelated problem for another patch) but we probably weren't doing it at the right time in the first place.

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