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

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D144217#4143554 <https://reviews.llvm.org/D144217#4143554>, @PiotrZSL wrote:

> In D144217#4143540 <https://reviews.llvm.org/D144217#4143540>, @carlosgalvezp wrote:
>
>>> Perhaps, this can even be generalized to all types whose size() and empty() are constexpr.
>
> Problem is that you can mark function constexpr, but it doesnt need to be constexpr evaluated:
>
>   struct C
>   {
>       constexpr C(int v) : v(v) {}
>       constexpr int size() const { return v; };
>       constexpr bool empty() const { return v == 0; };
>       constexpr void setSize(int vv) { v = vv; };
>       int v;
>   };
>   
>   constexpr C value(6);
>   C non_const(6);
>
> I would need to check if it's constexpr evaluated, but here we don't evaluate it. Alternative would be to check if method use only template arguments, but then it could use other const static or something...
> This is why I decided to go with config. And still you may have other user provided classes that does not have empty/size constexpr.

Ok, good point!



================
Comment at: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp:101
+      IgnoreComparisonForTypesRegexp(
+          Options.get("IgnoreComparisonForTypesRegexp", "^::std::array")) {}
+
----------------
I also realize that in other checks, this is typically implemented as a list (comma or semicolon-separated) instead of a regex. Would it make sense to do that here as well, for consistency? As a user I would also find it easier and more intuitive to type a list than to have to google how to create a list using regex :) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144217