[PATCH] D144135: [clang-tidy] Add performance-enum-size check
Piotr Zegar via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 15 23:22:13 PST 2023
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>
More information about the cfe-commits
mailing list