[PATCH] D113793: Comment Sema: Run checks only when appropriate (NFC)

Dmitri Gribenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 17 04:03:05 PST 2021


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



More information about the cfe-commits mailing list