[PATCH] D96281: [clang-tidy] Add options to flag individual core increments and to ignore macros to readability-function-cognitive-complexity check.

Jens Massberg via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 11 09:47:57 PST 2021


massberg added a comment.



In D96281#2556586 <https://reviews.llvm.org/D96281#2556586>, @njames93 wrote:

> How does this handle a macro where the argument has complex code.
>
>   MACRO(if (...) {});

I agree that macro arguments should account to the complexity.

However, with this change macro arguments are ignored.
Unfortunately, I do not see an easy way how to ignore the macro code, but consider the arguments during analysis.

Optimally, macro calls should be considered like function calls in code. Consider the following example:

  #define noop
  
  #define SomeMacro(x)  \
    if (1) {            \
      x                 \
    }
  
  void f() {
    SomeMacro(if (1) { noop; })
  }

With IgnoreMacros='false' the check gives the function `f()` a complexity of 3, while with IgnoreMacro='true' the check gives it at complexity of 0.
IMO the complexity should be 1 (and maybe the macro definition itself should be flagged to have a complexity of 1).

So there is still room for improvements. However, from the code that I have seen it is more likely that there is complexity in the macros itself and not in their arguments when being used.
So I would tend to use the option IgnoreMacros='true'. However, as this isn't perfect and others might not agree with it I decided to add the option and also default it to
the old implementation.

I have added test cases that show this problem of ignoring macro arguments and added a comment to the documentation of the option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96281



More information about the cfe-commits mailing list