[PATCH] D29692: [clang-tidy] add check modernize-use-const-instead-of-define

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D29692#676559, @AlexanderLanin wrote:

> Thanks for the feedback. As I'm new to this I cannot say whether checking only the file in question fits better with clang-tidy’s policy or not.
>  Also, I’m not sure about ODR. Of course, it’s a possibility, but isn’t the programmer responsible for that? This will be more of an issue as soon as this check provides a Fix-It solution.


It's still an issue without the FixIt because the diagnostic guides the user to do something that may make their well-formed code into ill-formed (no diagnostic required!) code. I think the correct way to handle this is to ensure we don't diagnose macro definitions that exist in a header file, at least for C++ code.

> As for the other part, I've checked some guidelines (without any order or selection process)
>  MISRA C++: Don’t use `#define`, use `const` variables; Also don’t do math on enums
>  CppCoreGuidelines: Don’t use `#define`, use `constexpr` variables 
>  SEI CERT C++: No mention of `#define` as far as I can tell
>  JSF AV C++: Don’t use `#define`, use `const` variables
>  HIC++: Don’t use `#define`, use `const` objects (reference to JSF AV C++ and MISRA C++)
> 
> So basically they're all the same. The only question is `const` vs `constexpr`

I don't think the guidance in this check is incorrect, I just think the check is implemented too broadly. As for 'const' vs 'constexpr', such a decision may differ when compiling in C vs C++ mode.


https://reviews.llvm.org/D29692