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 cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits