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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to