[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D84316#2168462 <https://reviews.llvm.org/D84316#2168462>, @NoQ wrote:

> Shared accessors look amazing.
>
> [...] you're splitting up the part which performs boring bookkeeping for the state trait from the part which models `strlen()` and other various functions.


Exactly.

> Such separation also looks amazing because ultimately the first part can be made checker-inspecific (i.e., a reusable half-baked trait that can be instantiated multiple times to track various things regardless of their meaning).

Could you elaborate on the latter part? (instantiated multiple times...)

> I don't think i understand having `unix.cstring.CStringChecker` as one more entry in `Checkers.td`. Do you expect there to be a situation when enabling `CStringModeling` without `CStringChecker` actually makes sense?

You seem to be right.
Enabling only the cstring modeling part does not make much sense without enabling //at least// the CStringChecker to model the cstring manipulation functions - even if the reporting is disabled of such functions.

> If not, why not keep them agglutinated?

I wanted to have a separate class for bookkeeping while minimalizing the necessary changes.
What do you think would be the best way to organize this separation?

Few notes:

- Checkers are implemented in the anonymous namespace, so only the given file has access to them.
- I wanted to separate the bookkeeping logic from the reporting/function modeling logic in different files.
- I like the fact that after the change the CStringChecker implements only the evalCall checker callback.

Let me know if I misunderstood something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84316