llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Piotr Zegar (PiotrZSL) <details> <summary>Changes</summary> Extend readability-function-size check by adding IgnoreMacros option. Fixes #<!-- -->112835 --- Full diff: https://github.com/llvm/llvm-project/pull/131406.diff 5 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp (+13-2) - (modified) clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h (+2) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) - (modified) clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst (+7) - (modified) clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp (+14-1) ``````````diff diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp index 3313bcb39b7f3..9535211413be6 100644 --- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp @@ -20,6 +20,8 @@ class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> { using Base = RecursiveASTVisitor<FunctionASTVisitor>; public: + FunctionASTVisitor(bool IgnoreMacros) : IgnoreMacros(IgnoreMacros) {} + bool VisitVarDecl(VarDecl *VD) { // Do not count function params. // Do not count decomposition declarations (C++17's structured bindings). @@ -39,6 +41,9 @@ class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> { if (!Node) return Base::TraverseStmt(Node); + if (IgnoreMacros && Node->getBeginLoc().isMacroID()) + return true; + if (TrackedParent.back() && !isa<CompoundStmt>(Node)) ++Info.Statements; @@ -120,6 +125,9 @@ class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> { llvm::BitVector TrackedParent; unsigned StructNesting = 0; unsigned CurrentNestingLevel = 0; + +private: + const bool IgnoreMacros; }; } // namespace @@ -135,7 +143,9 @@ FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context) NestingThreshold( Options.get("NestingThreshold", DefaultNestingThreshold)), VariableThreshold( - Options.get("VariableThreshold", DefaultVariableThreshold)) {} + Options.get("VariableThreshold", DefaultVariableThreshold)), + IgnoreMacros( + Options.getLocalOrGlobal("IgnoreMacros", DefaultIgnoreMacros)) {} void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "LineThreshold", LineThreshold); @@ -144,6 +154,7 @@ void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "ParameterThreshold", ParameterThreshold); Options.store(Opts, "NestingThreshold", NestingThreshold); Options.store(Opts, "VariableThreshold", VariableThreshold); + Options.store(Opts, "IgnoreMacros", IgnoreMacros); } void FunctionSizeCheck::registerMatchers(MatchFinder *Finder) { @@ -158,7 +169,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.TraverseDecl(const_cast<FunctionDecl *>(Func)); auto &FI = Visitor.Info; diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h index 106c69ff07393..b36de68f9583c 100644 --- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h +++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h @@ -47,6 +47,7 @@ class FunctionSizeCheck : public ClangTidyCheck { const std::optional<unsigned> ParameterThreshold; const std::optional<unsigned> NestingThreshold; const std::optional<unsigned> VariableThreshold; + const bool IgnoreMacros; static constexpr std::optional<unsigned> DefaultLineThreshold = std::nullopt; static constexpr std::optional<unsigned> DefaultStatementThreshold = 800U; @@ -58,6 +59,7 @@ class FunctionSizeCheck : public ClangTidyCheck { std::nullopt; static constexpr std::optional<unsigned> DefaultVariableThreshold = std::nullopt; + 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 0ad52f83fad85..6ae117ca57ed4 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -178,6 +178,10 @@ Changes in existing checks tolerating fix-it breaking compilation when functions is used as pointers to avoid matching usage of functions within the current compilation unit. +- Improved :doc:`readability-function-size + <clang-tidy/checks/readability/function-size>` check by adding the option + `IgnoreMacros` to allow ignoring code inside macros. + Removed checks ^^^^^^^^^^^^^^ 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 133bd3e9c8cbe..4d7c8e9ed47f2 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 @@ -43,3 +43,10 @@ Options The default is `none` (ignore the number of variables). Please note that function parameters and variables declared in lambdas, GNU Statement Expressions, and nested class inline functions are not counted. + +.. option:: IgnoreMacros + + If set to `true`, the check will ignore code inside macros. Note, that also + any macro arguments are ignored, even if they should count to the complexity. + As this might change in the future, this option isn't guaranteed to be + forward-compatible. Default is `false`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp index 45b2604b43d03..f7fe5509fed99 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp @@ -15,7 +15,8 @@ // RUN: readability-function-size.BranchThreshold: "5", \ // RUN: readability-function-size.ParameterThreshold: "none", \ // RUN: readability-function-size.NestingThreshold: "", \ -// RUN: readability-function-size.VariableThreshold: "" \ +// RUN: readability-function-size.VariableThreshold: "", \ +// RUN: readability-function-size.IgnoreMacros: "true" \ // RUN: }}' // Bad formatting is intentional, don't run clang-format over the whole file! @@ -319,3 +320,15 @@ void variables_16() { // CHECK-MESSAGES: :[[@LINE-5]]:6: note: 3 lines including whitespace and comments (threshold 0) // CHECK-MESSAGES: :[[@LINE-6]]:6: note: 4 statements (threshold 0) // CHECK-MESSAGES: :[[@LINE-7]]:6: note: 2 variables (threshold 1) + +#define CONDITION(value) if (value) { return value; } + +int macro_test(int v) { +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'macro_test' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-2]]:5: note: 8 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-3]]:5: note: 31 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-4]]:5: note: 8 branches (threshold 0) + + CONDITION(v);CONDITION(v);CONDITION(v);CONDITION(v);CONDITION(v);CONDITION(v);CONDITION(v); + if (v) { return v; } +} `````````` </details> https://github.com/llvm/llvm-project/pull/131406 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
