https://github.com/savchart created https://github.com/llvm/llvm-project/pull/199549
This patch adds an IgnoreMacros option to the readability-function-size check. Fixes https://github.com/llvm/llvm-project/issues/112835 >From 09a264762ec7dcb5da74d445d6c5a956f6d8d5b0 Mon Sep 17 00:00:00 2001 From: savchart <[email protected]> Date: Mon, 25 May 2026 17:16:50 +0200 Subject: [PATCH] [clang-tidy][readability] Ignore macros in function-size check --- .../readability/FunctionSizeCheck.cpp | 37 ++++++++--- .../readability/FunctionSizeCheck.h | 5 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 ++ .../checks/readability/function-size.rst | 5 ++ .../function-size-ignore-macros.cpp | 63 +++++++++++++++++++ 5 files changed, 105 insertions(+), 10 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/function-size-ignore-macros.cpp diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp index c331a74b9b794..7d7aa7ffcc11a 100644 --- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp @@ -20,17 +20,20 @@ class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> { using Base = RecursiveASTVisitor<FunctionASTVisitor>; public: + explicit FunctionASTVisitor(bool IgnoreMacros) : IgnoreMacros(IgnoreMacros) {} + bool VisitVarDecl(VarDecl *VD) { // Do not count function params. // Do not count decomposition declarations (C++17's structured bindings). if (StructNesting == 0 && - !(isa<ParmVarDecl>(VD) || isa<DecompositionDecl>(VD))) + !(isa<ParmVarDecl>(VD) || isa<DecompositionDecl>(VD)) && + shouldCountLocation(VD->getBeginLoc())) ++Info.Variables; return true; } bool VisitBindingDecl(BindingDecl *BD) { // Do count each of the bindings (in the decomposition declaration). - if (StructNesting == 0) + if (StructNesting == 0 && shouldCountLocation(BD->getBeginLoc())) ++Info.Variables; return true; } @@ -39,7 +42,8 @@ class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> { if (!Node) return Base::TraverseStmt(Node); - if (TrackedParent.back() && !isa<CompoundStmt>(Node)) + if (TrackedParent.back() && !isa<CompoundStmt>(Node) && + shouldCountLocation(Node->getBeginLoc())) ++Info.Statements; switch (Node->getStmtClass()) { @@ -49,7 +53,8 @@ class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> { case Stmt::CXXForRangeStmtClass: case Stmt::ForStmtClass: case Stmt::SwitchStmtClass: - ++Info.Branches; + if (shouldCountLocation(Node->getBeginLoc())) + ++Info.Branches; [[fallthrough]]; case Stmt::CompoundStmtClass: TrackedParent.push_back(true); @@ -67,15 +72,19 @@ class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> { } bool TraverseCompoundStmt(CompoundStmt *Node) { + const bool CountThisCompound = shouldCountLocation(Node->getBeginLoc()); + // If this new compound statement is located in a compound statement, which // is already nested NestingThreshold levels deep, record the start location // of this new compound statement. - if (CurrentNestingLevel == Info.NestingThreshold) + if (CountThisCompound && (CurrentNestingLevel == Info.NestingThreshold)) Info.NestingThresholders.push_back(Node->getBeginLoc()); - ++CurrentNestingLevel; + if (CountThisCompound) + ++CurrentNestingLevel; Base::TraverseCompoundStmt(Node); - --CurrentNestingLevel; + if (CountThisCompound) + --CurrentNestingLevel; return true; } @@ -109,7 +118,7 @@ class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> { } bool TraverseConstructorInitializer(CXXCtorInitializer *Init) { - if (CountMemberInitAsStmt) + if (CountMemberInitAsStmt && shouldCountLocation(Init->getSourceLocation())) ++Info.Statements; Base::TraverseConstructorInitializer(Init); @@ -129,6 +138,12 @@ class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> { unsigned StructNesting = 0; unsigned CurrentNestingLevel = 0; bool CountMemberInitAsStmt; + const bool IgnoreMacros; + +private: + bool shouldCountLocation(SourceLocation Loc) const { + return !IgnoreMacros || !Loc.isMacroID(); + } }; } // namespace @@ -146,7 +161,8 @@ FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context) VariableThreshold( Options.get("VariableThreshold", DefaultVariableThreshold)), CountMemberInitAsStmt( - Options.get("CountMemberInitAsStmt", DefaultCountMemberInitAsStmt)) {} + Options.get("CountMemberInitAsStmt", DefaultCountMemberInitAsStmt)), + IgnoreMacros(Options.get("IgnoreMacros", DefaultIgnoreMacros)) {} void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "LineThreshold", LineThreshold); @@ -156,6 +172,7 @@ void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "NestingThreshold", NestingThreshold); Options.store(Opts, "VariableThreshold", VariableThreshold); Options.store(Opts, "CountMemberInitAsStmt", CountMemberInitAsStmt); + Options.store(Opts, "IgnoreMacros", IgnoreMacros); } void FunctionSizeCheck::registerMatchers(MatchFinder *Finder) { @@ -170,7 +187,7 @@ void FunctionSizeCheck::registerMatchers(MatchFinder *Finder) { void FunctionSizeCheck::check(const MatchFinder::MatchResult &Result) { const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func"); - FunctionASTVisitor Visitor; + FunctionASTVisitor Visitor(IgnoreMacros); Visitor.Info.NestingThreshold = NestingThreshold.value_or(-1); Visitor.CountMemberInitAsStmt = CountMemberInitAsStmt; Visitor.TraverseDecl(const_cast<FunctionDecl *>(Func)); diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h index 0459db6abfe31..39704135550cf 100644 --- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h +++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h @@ -32,6 +32,9 @@ namespace clang::tidy::readability { /// nesting level). /// * `VariableThreshold` - flag functions having a high number of variable /// declarations. The default is `-1` (ignore the number of variables). +/// * `IgnoreMacros` - if set to `true`, the check will not count statements, +/// branches, nesting levels, or variable declarations inside macros. The +/// default is `false`. class FunctionSizeCheck : public ClangTidyCheck { public: FunctionSizeCheck(StringRef Name, ClangTidyContext *Context); @@ -48,6 +51,7 @@ class FunctionSizeCheck : public ClangTidyCheck { const std::optional<unsigned> NestingThreshold; const std::optional<unsigned> VariableThreshold; const bool CountMemberInitAsStmt; + const bool IgnoreMacros; static constexpr std::optional<unsigned> DefaultLineThreshold = std::nullopt; static constexpr std::optional<unsigned> DefaultStatementThreshold = 800U; @@ -60,6 +64,7 @@ class FunctionSizeCheck : public ClangTidyCheck { static constexpr std::optional<unsigned> DefaultVariableThreshold = std::nullopt; static constexpr bool DefaultCountMemberInitAsStmt = true; + static constexpr bool DefaultIgnoreMacros = false; }; } // namespace clang::tidy::readability diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index f1b49e2cb6056..1c89c55015079 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -668,6 +668,11 @@ Changes in existing checks now uses separate note diagnostics for each uninitialized enumerator, making it easier to see which specific enumerators need explicit initialization. +- Improved :doc:`readability-function-size + <clang-tidy/checks/readability/function-size>` check by adding an + `IgnoreMacros` option to exclude statements, branches, nesting levels, and + variable declarations inside macros from the reported metrics. + - Improved :doc:`readability-identifier-length <clang-tidy/checks/readability/identifier-length>` check: diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst index 253e7c483cb85..a87862f77c366 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst @@ -48,3 +48,8 @@ Options When `true`, count class member initializers in constructors as statements. Default is `true`. + +.. option:: IgnoreMacros + + If set to `true`, the check will not count statements, branches, nesting + levels, or variable declarations inside macros. Default is `false`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/function-size-ignore-macros.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/function-size-ignore-macros.cpp new file mode 100644 index 0000000000000..1db34f68e6371 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/function-size-ignore-macros.cpp @@ -0,0 +1,63 @@ +// RUN: %check_clang_tidy -check-suffix=IGNORE-MACROS %s readability-function-size %t -- \ +// RUN: -config='{CheckOptions: { \ +// RUN: readability-function-size.StatementThreshold: 1, \ +// RUN: readability-function-size.BranchThreshold: 0, \ +// RUN: readability-function-size.NestingThreshold: 1, \ +// RUN: readability-function-size.VariableThreshold: 0, \ +// RUN: readability-function-size.IgnoreMacros: true \ +// RUN: }}' + +#define STATEMENT ; +#define BRANCH if (true) {} +#define VARIABLE int X; +#define INIT_X X(0) +#define INIT_Y Y(0) +#define WRAP(x) x + +void user_statements() { + ; +} + +void user_branch() { + // CHECK-NOTES-IGNORE-MACROS: :[[@LINE-1]]:6: warning: function 'user_branch' exceeds recommended size/complexity thresholds [readability-function-size] + // CHECK-NOTES-IGNORE-MACROS: :[[@LINE-2]]:6: note: 2 statements (threshold 1) + // CHECK-NOTES-IGNORE-MACROS: :[[@LINE-3]]:6: note: 1 branches (threshold 0) + if (true) { + // CHECK-NOTES-IGNORE-MACROS: :[[@LINE-1]]:13: note: nesting level 2 starts here (threshold 1) + } +} + +void user_variable() { + // CHECK-NOTES-IGNORE-MACROS: :[[@LINE-1]]:6: warning: function 'user_variable' exceeds recommended size/complexity thresholds [readability-function-size] + // CHECK-NOTES-IGNORE-MACROS: :[[@LINE-2]]:6: note: 1 variables (threshold 0) + int X; +} + +void macro_statement() { + ; + STATEMENT +} + +void macro_branch() { + ; + BRANCH +} + +void macro_variable() { + ; + VARIABLE +} + +struct MacroCtorInit { + int X, Y; + MacroCtorInit() : INIT_X, INIT_Y { + ; + } +}; + +void macro_wrap() { + // The macro argument written by the user is currently ignored. + ; + WRAP(if (true){}) +} + _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
