[PATCH] D144217: [clang-tidy] Fix false-positive in readability-container-size-empty

Carlos Galvez via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 21 23:07:27 PST 2023


carlosgalvezp added a comment.

Looks good in general, thanks for the fix! I have minor comments.

Also, I found the comment on the Github issue interesting:

> Perhaps, this can even be generalized to all types whose size() and empty() are constexpr.

Would that be a more scalable approach than maintaining a list of classes to ignore?



================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst:30
+
+    Excludes certain classes from the check using a regular expression of ERE
+    syntax. If excluded, the check won't suggest replacing the comparison of
----------------
For better readability, please spell out this acronym.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp:721
 }
 
 struct TypedefSize {
----------------
Missing a test case to test that the option works as expected, e.g. set it to a non-default value.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp:758
+
+bool testArrayCompareToEmptye(const Array& value) {
+  return value == std::array<int, 10U>();
----------------
Empty


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144217



More information about the cfe-commits mailing list