[PATCH] D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions

[Marcin Twardak via Phabricator via cfe-commits]

twardakm marked 9 inline comments as done.
twardakm added inline comments.


================
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));
+}
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69855