aaron.ballman added a comment.
In https://reviews.llvm.org/D24349#537350, @Eugene.Zelenko wrote:
> Probably check should have options to extend list of containers and also to
> assume all classes with integer type size() const and bool empty() const as
> containers. It may be not trivial to find out all custom containers and last
> option will be helpful to assemble such list.
I think this should actually have two options: one is a list of default-checked
containers (default is the list of STL containers we previously had), and the
other is a Boolean option for guessing at what might be a container based on
function signature (default is true). This gives people a way to silence the
diagnostic while still being useful. I would document what function signatures
we use as our heuristic, but note that the function signatures we care about
may change (for instance, we may decide that const-qualification is
unimportant, or that we want to require begin()/end() functions, etc). I think
`<standard integer type> size() const` and `bool empty() const` are a
reasonable heuristic for a first pass.
================
Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:33
@@ +32,3 @@
+ const auto validContainer = namedDecl(
+ has(functionDecl(
+ isPublic(), hasName("size"), returns(isInteger()),
----------------
omtcyfz wrote:
> Thank you!
>
> Blacklisted these types.
>
> Actually, I believe if someone has `bool size()` in their codebase there's
> still a problem...
> Blacklisted these types.
I'm still not certain this is correct. For instance, if someone wants to do
`uint8_t size() const;`, we shouldn't punish them because `uint8_t` happens to
be `unsigned char` for their platform. But you are correct that we don't want
this to work with char32_t, for instance.
I think the best approach is to make a local matcher for the type predicate. We
want isInteger() unless it's char (that's not explicitly qualified with signed
or unsigned), bool, wchar_t, char16_t, char32_t, or an enumeration type.
> Actually, I believe if someone has bool size() in their codebase there's
> still a problem...
Agreed, though not as part of this check. ;-)
https://reviews.llvm.org/D24349
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
