NoQ accepted this revision.
NoQ added inline comments.
This revision is now accepted and ready to land.

================
Comment at: 
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp:636
     if (isa<NonParamVarRegion>(R))
-      Result.push_back(R);
+      Result.emplace_back(R, Val);
 
----------------
vsavchenko wrote:
> NoQ wrote:
> > Because we already know that `Val.getAsLocSymbol()` is equal to `Sym`, we 
> > can be certain that `Val` is either a `&SymRegion{Sym}` (i.e., literally 
> > this symbol in its pristine representation) or, in some rare cases, a 
> > `LocAsInteger` over that (which is a case i'm not sure we even want to 
> > handle). I dunno if we really need to return it here. Maybe it's easier to 
> > re-construct it in place as `SValBuilder.makeLoc(Sym)`.
> Oh, that was my original solution, but it doesn't work for dynamic casts.
> This check compares symbols, but `FindLastStore` compares `SVal`s.  
> `&Derived{&SymRegion}` is not the same as `&SymRegion` and we fail.
> So, instead of hacking on the result from `SValBuilder` trying to make 
> something that will indeed match the last stored value, I save that last 
> stored value here, so we're guaranteed that `FindLastStore` will work as 
> intended.
Oh, C++. Indeed. Our pointer cast representation strikes again. Ok then!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100852/new/

https://reviews.llvm.org/D100852

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D100852: [analyz... Artem Dergachev via Phabricator via cfe-commits

Reply via email to