aaron.ballman accepted this revision.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM!
================
Comment at:
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:83
+ const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP)
{
+ PP->addPPCallbacks(std::make_unique<NamespaceCommentPPCallbacks>(PP, this));
+}
----------------
twardakm wrote:
> aaron.ballman wrote:
> > twardakm wrote:
> > > aaron.ballman wrote:
> > > > Rather than making an entire new object for PP callbacks, why not make
> > > > `NamespaceCommentCheck` inherit from `PPCallbacks`? It seems like it
> > > > would simplify the interface somewhat.
> > > I think the check hast to inherit from ClangTidyCheck?
> > >
> > > I duplicated the solution from other checks (e.g.
> > > IdentifierNamingCheck.cpp). Could you please point to some existing check
> > > which implements your idea?
> > I don't know if we have other checks doing this -- I was thinking of using
> > multiple inheritance in this case, because the callbacks are effectively a
> > mixin.
> >
> > That said, I don't insist on this change.
> The problem which I see is that addPPCallbacks takes ownership of the object
> passed to it. I couldn't find any other way of invoking PP callbacks, so I'll
> leave it as is.
Ah, that makes sense. Thank you for investigating it. :-)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69855/new/
https://reviews.llvm.org/D69855
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
