aaron.ballman 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));
+}
----------------
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.
================
Comment at:
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:101-104
+ if (!IsNamespaceMacroExpansion)
+ Fix.append(" ").append(ND->getNameAsString());
+ else
+ Fix.append(" ").append(MacroDefinition);
----------------
Would this work instead `Fix.append(" ").append(IsNamespaceMacroExpansion ?
MacroDefinition : ND->getName());`?
(You may have to check the behavioral difference of `getName()` vs
`getNameAsString()` to be sure.)
================
Comment at:
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:135
+ const StringRef NameSpaceName) {
+ const auto &MacroIt = std::find_if(std::begin(Macros), std::end(Macros),
+ [&NameSpaceName](const auto &Macro) {
----------------
`llvm::find_if(Macros, ...);`
================
Comment at:
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:247
+ // Allow using macro definitions in closing comment.
+ if (isNamespaceMacroDefinition(NamespaceNameInComment)) {
+ return;
----------------
Can elide braces.
================
Comment at:
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:287
+
+ if (IsNamespaceMacroExpansion) {
+ NestedNamespaceName = MacroDefinition;
----------------
Elide braces
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
