[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtr.h:36
+
+class FindWhereConstrained : public BugReporterVisitor {
+private:
----------------
RedDocMD wrote:
> vsavchenko wrote:
> > vsavchenko wrote:
> > > nit: class name should be a noun (functions and methods are verbs)
> > It is again some sort of verb. I don't know how to read it except for "emit note on get"-visitor. What about something like `GetNoteEmitter`? I mean do we really need to put `Visitor` in the name?
> I put in `Visitor` because all the visitors that I have come across end with *Visitor*. But then again, I am sure that if we look, we'll find counter-examples. So your call here ...
That's true, but it doesn't mean that we shouldn't care about the names and how they read.  You cannot say that you visit "emit note on get", so the name doesn't help to understand what this class does. If you want to name it `GetVisitor` - no problem because it does visit `get`s.  But if you want to state in the name that it emits notes on gets, the name should say `Emitter`, and that second name is more specific.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183