search

tarantool / checkpatch

Checkpatch for Tarantool
GNU General Public License v2.0
2 stars 2 forks source link

False positive MACRO_ARG_PRECEDENCE #46

Closed dmitrylala closed 1 year ago

dmitrylala commented 1 year ago

https://github.com/tarantool/tarantool/actions/runs/3210914841/jobs/5249710180

checkpatch finds an error in macro: Macro argument 'PROTO_TYPE' may be better as '(PROTO_TYPE)' to avoid precedence issues

#define PROTO_TOSTRING(PROTO_TYPE, VAR_NAME) \
    std::string PROTO_TYPE##ToString(const PROTO_TYPE & (VAR_NAME))

However, if i place PROTO_TYPE in parentheses, compilation error occurred, so I think it is a false positive.

rtolkacheva commented 1 year ago

I had a similar problem, but only with VAR_NAME. PROTO_TYPE in this example is a class name, VAR_NAME is a variable name, so in both these cases macros arguments must be valid string literals, or the program won't be compiled.

locker commented 1 year ago 
#define PROTO_TOSTRING(PROTO_TYPE, VAR_NAME) \
  std::string PROTO_TYPE##ToString(const PROTO_TYPE & (VAR_NAME))

Macros like this obfuscate the code and make it difficult for understanding. Generally, we don't use macros that expand in unbalanced C/C++ constructs in our code. The Google C++ Style Guide gives an explanation why such macros should be avoided. I think you should drop the macros and define these functions properly.