[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 23 06:20:47 PDT 2023


njames93 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.cpp:359
+void UseEarlyExitsCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(translationUnitDecl(), this);
+}
----------------
PiotrZSL wrote:
> This will trigger on all system code, and then users will complain again that they see poor clang-tidy performance...
> 
> when it could be just something like
> 
> ```
> ifStmt(unless(isExpansionInSystemHeader()),
>        unless(isConsteval()),
>        unless(isConstexpr()),
>        unless(hasElse(stmt())),
>        unless(hasInitStatement(stmt()),
>        hasThen(compoundStmt(hasSizeAboeLineTreshold())),
>        ...
> ```
> 
> Simply everything that could be put into matcher should be put into matcher (easier to maintain), also what's a point of checking functions that doesn't have if's. On that point also some implicit functions or if statement should be ignored, to avoid checking templates twice.
> 
Not triggering on system headers is a good point, but for raw performance, that code is better residing in the visitor,  which can massively outperform matchers as we can straight up ignore huge blocks of code that aren't of interest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130181



More information about the cfe-commits mailing list