[PATCH] D105552: [analyzer][NFC] NoStoreFuncVisitor: Compact parameters for region pretty printing into a class

Kristóf Umann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 8 03:25:17 PDT 2021


Szelethus added a comment.

Actually, one thing might be worth considering. The grand idea is to split up `NoStoreFuncVisitor` so that its clients only need to define these 3 (quoting from D105553 <https://reviews.llvm.org/D105553>):

> - What constitutes as a change to an entity
> - What the diagnostic message should be
> - What constitutes as the entity of interest being passed into the function

In the end, in my eye, it would look like this:

                       <--- NoStateChangeToRegionVisitor <--- NoStoreFuncVisitor
                      /
  NoStateChangeVisitor                                   <--- NoDynamicMemoryOwnershipChangeVisitor
                      \                                 /
                       <--- NoStateChangeToSymbolVisitor
                                                        \
                                                         <--- NoStreamOwnershipChangeVisitor

- `NoStateChangeVisitor` will ask its clients the above mentioned 3 questions.
- `NoStateChangeToSymbolVisitor` and `NoStateChangeToRegionVisitor` would answer whether the entity of interest was actually passed into the function, and propagate the rest.
- Visitors on the bottom would take care of what remains.

With this, or a similar change, visitors handling state changes to regions would need to know even less about the job that `NoStateChangeToRegionVisitor` supposedly took over already.

With that said, I don't have another `NoStateChangeToRegionVisitor` in my mind for the short term future, and even if I did, such a cosmetic encapsulation could wait for a better thought out solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105552



More information about the cfe-commits mailing list