[PATCH] D85697: [clang-tidy] Add cppcoreguidelines-prefer-scoped-enums-over-unscoped

Mon Oct 19 03:47:16 PDT 2020

njames93 added a comment.

In D85697#2234468 <https://reviews.llvm.org/D85697#2234468>, @janosbenjaminantal wrote:

> The known "limitations" are mostly similar to other checks:
>
> - It cannot detect unfixable cases from other translation units. Practically that means if an `enum` is used in multiple source files, one of them might end up not fixed. I tried to work around this, but I haven't found any solution for this, moreover this cause problems for other checks also. Therefore I think it shouldn't be a blocking issue.

That's what the run_clang_tidy.py script is meant to handle.

> - It doesn't take into account if an `enum` is defined in a header that is filtered out. Similarly to the previous one, I tried to find a solution for this, but I was unable (the `ClangTidyCheck` class can access the header filter information, but it doesn't expose it for the descendant classes). I also checked other checks, and they behave in the same way. Therefore I also think it is shouldn't be a blocking issue.

I recently pushed an upgrade to readability-identifier-naming where it would check the naming style for identifiers declared in header files, maybe thats something this could also use, this is the commit 4888c9ce97d8 <https://reviews.llvm.org/rG4888c9ce97d8c20d988212b10f1045e3c4022b8e>

> It is not strongly connected to this review, but in the future I am planning to extend the check with:
>
> - options to exclude enums, because changing them to scoped enumerations might not be suitable for every cases

Not strictly necessary, if people don't want the fix they could annotate the code with a `// NOLINT(*prefer-unscoped-enums)` comment.

> - options to force the doable fixes: based on my little experience it might be easier to force the doable fixes and manually fix the remaining ones

Forcing the fix is usually just a case of converting implicit cast usages of the constants into static casts.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85697