[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Since this is a `modernize` check, shouldn't it be proposing to use `enum class` instead of `enum`? This will conflict with `Enum.3` from `cppcoreguidelines`, and I don't think it's unreasonable that people enable both `modernize*` and `cppcoreguidelines*`. Not sure if such a check for `Enum.3` exists today but it could easily be added in the future.

I understand that the fix perhaps cannot be automated just as easily, but maybe it's OK? Or at least make the default use `enum class` and leave it as an option to use unscoped enums to be able to apply the fix. Still I find it a bit inconsistent with the rest of the `modernize` module (modernize as in "use post-C++11 stuff").

Another point is that macros typically have a different naming convention than enums or constants, so users will likely have to do manual work anyway, or rely on the "readability-identifier-naming" check - how does it play out then if 2 checks propose different fixes?


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

https://reviews.llvm.org/D117522