[PATCH] D144135: [clang-tidy] Add performance-enum-size check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL marked an inline comment as done.
PiotrZSL added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp:127-128
+
+  diag(MatchedDecl->getLocation(), "enum %0 derive from %1 of size %2 bytes, "
+                                   "derive from '%3' to reduce it size to %4")
+      << MatchedDecl << MatchedDecl->getIntegerType() << Size << NewType.first
----------------
unterumarmung wrote:
> I find the enum's type derivation message to be a bit unintuitive. It would slightly improve the user experience if the error message provided clearer information, like stating "enum %0 has a base type of %1..." or "the base type of enum %0 appears excessive for its value set...". However, please remember that these are merely personal thoughts, and as a non-contributor, my suggestions are not obligatory.
Hmm, ok, I think I can change it to utilize a "base type" instead of "derive" and something like "appears excessive for its value set.".


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst:62
+Requires C++11 or above.
+Does not provide auto-fixes.
+
----------------
unterumarmung wrote:
> Why not? 
Problem is mainly with forward declarations, and a fact that some of these changes may be unnecessary from a domain point of view. I would prefer users to change enum sizes on their own risk. And I didn't wanted to add this at the beginning, in future maybe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144135