[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 3 22:42:43 PDT 2019


NoQ added inline comments.


================
Comment at: cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:133
   std::unique_ptr<llvm::DenseSet<const VarDecl *>> InEH;
+  const bool WarnForDeadNestedAssignments;
 
----------------
Szelethus wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > I suggest we adopt the idiom of passing the `Checker` object around and asking it about its options instead of passing each option around separately. This is easier and i don't see any downsides. Moreover, we already pass the `Checker` around (just type-erased for no good reason). If you don't mind, i'll remove this field as part of resolving merge conflicts in D65182.
> > What about subcheckers? In any case, feel free to remove it for now.
> What type erasure do you talk about specifically? In any case, it might happen because of our library layout, I had a bad time with that when I did the checker registration thingie.
I meant `const DeadStoresChecker *` -> `const CheckerBase *`.

> What about subcheckers?

As long as they're represented by the same checker object that's defined within the same translation unit, this isn't a problem. When they're multiple checker objects - dunno, i've honestly never seen that happen (:

In any case, i don't think it's a hard requirement, just an always-easier-thing-to-implement.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66733





More information about the cfe-commits mailing list