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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL updated this revision to Diff 497787.
PiotrZSL added a comment.
PiotrZSL updated this revision to Diff 497804.
PiotrZSL updated this revision to Diff 497826.
Eugene.Zelenko added reviewers: aaron.ballman, carlosgalvezp.
PiotrZSL marked 4 inline comments as done.
PiotrZSL updated this revision to Diff 497887.
PiotrZSL published this revision for review.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Removed change in list.rst that is related to cppcoreguidelines-avoid-capture-default-when-capturing-this.
Looks like that check have fixes but didnt mark fixes in that file.


PiotrZSL added a comment.

Fix typo in documentation.


PiotrZSL added a comment.

Fix tests on windows (they use int as base for enums, when on linux unsigned int is used)


PiotrZSL added a comment.

Fix review comments


PiotrZSL added a comment.

Ready for review



================
Comment at: clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp:99
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<EnumDecl>("e");
+  auto BaseType = MatchedDecl->getIntegerType().getCanonicalType();
+  if (not BaseType->isIntegerType())
----------------
Please do not use `auto` unless type is explicitly spelled in same function or iterator.


================
Comment at: clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp:100
+  auto BaseType = MatchedDecl->getIntegerType().getCanonicalType();
+  if (not BaseType->isIntegerType())
+    return;
----------------
I don't think that literal boolean operators are part of LLVM coding guidelines.


================
Comment at: clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp:103
+
+  auto Size = Result.Context->getTypeSize(BaseType) / 8U;
+  if (1U == Size)
----------------
Ditto.


================
Comment at: clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp:120
+  auto NewType = getNewType(Size, MinV, MaxV);
+  if (not NewType.first or Size <= NewType.second)
+    return;
----------------
Ditto.


Finds enum type definitions that could use smaller integral type as a base.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144135

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp
  clang-tools-extra/clang-tidy/performance/EnumSizeCheck.h
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst
  clang-tools-extra/test/clang-tidy/checkers/performance/enum-size.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144135.497887.patch
Type: text/x-patch
Size: 13212 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230216/e51b916a/attachment-0001.bin>