massberg created this revision.
massberg added reviewers: lebedev.ri, alexfh.
Herald added a subscriber: nullptr.cpp.
massberg requested review of this revision.
Herald added a project: clang.
Often you are only interested in the overall cognitive complexity of a
function and not every individual increment. Thus the flag
'FlagBasicIncrements' is added. If it is set to 'true', each increment
is flagged. Otherwise, only the complexity of function with complexity
of at least the threshold are flagged.
If a macro is used within the function, the code inside the macro
doesn't make the code less readable. Instead, for a reader a macro is
more like a function that is called. Thus the code inside a macro
shouldn't increase the complexity of the function in which it is called.
Thus the flag 'IgnoreMacros' is added. If set to 'true' code inside
macros isn't considered during analysis.
This isn't perfect, as now the code of a macro isn't considered at all,
even if it has a high cognitive complexity itself. It might be better if
a macro is considered in the analysis like a function and gets its own
cognitive complexity. Implementing such an analysis seems to be very
complex (if possible at all with the given AST), so we give the user the
option to either ignore macros completely or to let the expanded code
count to the calling function's complexity.
By default 'FlagBasisIncrements' is set to 'true' and 'IgnoreMacros' is
set to 'false', which is the original behavior of the check.
Added a new test for different flag combinations.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D96281
Files:
clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
clang-tools-extra/test/clang-tidy/checkers/readability-function-cognitive-complexity-flags.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-function-cognitive-complexity-flags.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-function-cognitive-complexity-flags.cpp
@@ -0,0 +1,64 @@
+// RUN: %check_clang_tidy %s readability-function-cognitive-complexity %t -- \
+// RUN: -config='{CheckOptions: \
+// RUN: [{key: readability-function-cognitive-complexity.Threshold, \
+// RUN: value: 0}, \
+// RUN: {key: readability-function-cognitive-complexity.FlagBasicIncrements, \
+// RUN: value: "false"} ]}'
+// RUN: %check_clang_tidy -check-suffix=THRESHOLD5 %s readability-function-cognitive-complexity %t -- \
+// RUN: -config='{CheckOptions: \
+// RUN: [{key: readability-function-cognitive-complexity.Threshold, \
+// RUN: value: 5}, \
+// RUN: {key: readability-function-cognitive-complexity.FlagBasicIncrements, \
+// RUN: value: "false"} ]}'
+// RUN: %check_clang_tidy -check-suffix=IGNORE-MACROS %s readability-function-cognitive-complexity %t -- \
+// RUN: -config='{CheckOptions: \
+// RUN: [{key: readability-function-cognitive-complexity.Threshold, \
+// RUN: value: 0}, \
+// RUN: {key: readability-function-cognitive-complexity.IgnoreMacros, \
+// RUN: value: "true"}, \
+// RUN: {key: readability-function-cognitive-complexity.FlagBasicIncrements, \
+// RUN: value: "false"} ]}'
+
+void func_of_complexity_4() {
+ // CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'func_of_complexity_4' has cognitive complexity of 4 (threshold 0) [readability-function-cognitive-complexity]
+ // CHECK-NOTES-IGNORE-MACROS: :[[@LINE-2]]:6: warning: function 'func_of_complexity_4' has cognitive complexity of 4 (threshold 0) [readability-function-cognitive-complexity]
+ if (1) {
+ if (1) {
+ }
+ }
+ if (1) {
+ }
+}
+
+#define MacroOfComplexity10 \
+ if (1) { \
+ if (1) { \
+ if (1) { \
+ if (1) { \
+ } \
+ } \
+ } \
+ }
+
+void function_with_macro() {
+ // CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'function_with_macro' has cognitive complexity of 11 (threshold 0) [readability-function-cognitive-complexity]
+ // CHECK-NOTES-THRESHOLD5: :[[@LINE-2]]:6: warning: function 'function_with_macro' has cognitive complexity of 11 (threshold 5) [readability-function-cognitive-complexity]
+ // CHECK-NOTES-IGNORE-MACROS: :[[@LINE-3]]:6: warning: function 'function_with_macro' has cognitive complexity of 1 (threshold 0) [readability-function-cognitive-complexity]
+
+ MacroOfComplexity10
+
+ if (1) {
+ }
+}
+
+#define uglyfunctionmacro(name) \
+ void name() { \
+ if (true) { \
+ try { \
+ } catch (...) { \
+ } \
+ } \
+ }
+
+uglyfunctionmacro(MacroFunction)
+ // CHECK-NOTES: :[[@LINE-1]]:19: warning: function 'MacroFunction' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
Index: clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
@@ -17,6 +17,16 @@
Flag functions with Cognitive Complexity exceeding this number.
The default is `25`.
+.. option:: FlagBasicIncrements
+
+ If set to `true`, the check will flag every basic increment. Default
+ is `true`.
+
+.. option:: IgnoreMacros
+
+ If set to `true`, the check will ignore code inside macros. Default is
+ `false`.
+
Building blocks
---------------
Index: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
+++ clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
@@ -17,10 +17,14 @@
/// Checks function Cognitive Complexity metric.
///
-/// There is only one configuration option:
+/// There are the following configuration option:
///
/// * `Threshold` - flag functions with Cognitive Complexity exceeding
/// this number. The default is `25`.
+/// * `FlagBasicIncrements`- if set to `true`, the check will flag every
+/// basic increment. Default is `true`
+/// * `IgnoreMacros` - if set to `true`, the check will ignore code inside
+/// macros. Default is `false`.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/readability-function-cognitive-complexity.html
@@ -34,6 +38,8 @@
private:
const unsigned Threshold;
+ const bool FlagBasicIncrements;
+ const bool IgnoreMacros;
};
} // namespace readability
Index: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
@@ -213,6 +213,9 @@
: public RecursiveASTVisitor<FunctionASTVisitor> {
using Base = RecursiveASTVisitor<FunctionASTVisitor>;
+ // If set to true, macros are ignored during analysis.
+ const bool IgnoreMacros;
+
// The current nesting level (increased by Criteria::IncrementNesting).
unsigned short CurrentNestingLevel = 0;
@@ -223,6 +226,9 @@
std::stack<OBO, SmallVector<OBO, 4>> BinaryOperatorsStack;
public:
+ explicit FunctionASTVisitor(const bool IgnoreMacros)
+ : RecursiveASTVisitor<FunctionASTVisitor>(), IgnoreMacros(IgnoreMacros){};
+
bool traverseStmtWithIncreasedNestingLevel(Stmt *Node) {
++CurrentNestingLevel;
bool ShouldContinue = Base::TraverseStmt(Node);
@@ -364,6 +370,9 @@
if (!Node)
return Base::TraverseStmt(Node);
+ if (IgnoreMacros && Node->getBeginLoc().isMacroID())
+ return true;
+
// Three following switch()'es have huge duplication, but it is better to
// keep them separate, to simplify comparing them with the Specification.
@@ -492,11 +501,15 @@
FunctionCognitiveComplexityCheck::FunctionCognitiveComplexityCheck(
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- Threshold(Options.get("Threshold", CognitiveComplexity::DefaultLimit)) {}
+ Threshold(Options.get("Threshold", CognitiveComplexity::DefaultLimit)),
+ FlagBasicIncrements(Options.get("FlagBasicIncrements", true)),
+ IgnoreMacros(Options.get("IgnoreMacros", false)) {}
void FunctionCognitiveComplexityCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "Threshold", Threshold);
+ Options.store(Opts, "FlagBasicIncrements", FlagBasicIncrements);
+ Options.store(Opts, "IgnoreMacros", IgnoreMacros);
}
void FunctionCognitiveComplexityCheck::registerMatchers(MatchFinder *Finder) {
@@ -514,7 +527,7 @@
assert(Func->hasBody() && "The matchers should only match the functions that "
"have user-provided body.");
- FunctionASTVisitor Visitor;
+ FunctionASTVisitor Visitor(IgnoreMacros);
Visitor.TraverseDecl(const_cast<FunctionDecl *>(Func), true);
if (Visitor.CC.Total <= Threshold)
@@ -524,6 +537,10 @@
"function %0 has cognitive complexity of %1 (threshold %2)")
<< Func << Visitor.CC.Total << Threshold;
+ if (!FlagBasicIncrements) {
+ return;
+ }
+
// Output all the basic increments of complexity.
for (const auto &Detail : Visitor.CC.Details) {
unsigned MsgId; // The id of the message to output.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
