gribozavr2 added a comment.
I find the code more readable and robust when the `check*` function checks its
applicability itself. After this refactoring, it is not so clear when each
check functions applies, what are the correct conditions to call them. To
ensure correct usage, probably we should be adding equivalent asserts to the
beginning of each function; at which point I'm not sure if the new code is
better.
For example, consider `checkContainerDecl`. It used to start with:
const CommandInfo *Info = Traits.getCommandInfo(Comment->getCommandID());
if (!Info->IsRecordLikeDetailCommand || isRecordLikeDecl())
return;
This code clearly shows the condition for the warning: the command is
"RecordDetailLike" but the decl is not "RecordLike". In the new code this logic
is split across two functions.
Is repeated `CommandInfo` fetching a performance issue? It shouldn't be because
it is basically address arithmetic on the static array of commands using the
command ID as an index.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113793/new/
https://reviews.llvm.org/D113793
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
