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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
