[PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

Kirill Bobyrev via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 8 14:23:47 PDT 2016


omtcyfz added a comment.

In https://reviews.llvm.org/D24349#537624, @aaron.ballman wrote:

> I think that's reasonable, depending on whether we find false positives with the warning as well (I have a slight concern about `size()` and `empty()` being unrelated operations on a non-container class that someone writes).


Agreed. I'd like to try LLVM as an example of a huge codebase tomorrow to see how many false-positives there actually are.

Generally, I can see the point of requiring `const` and providing options to reduce the set of allowed classes to `std::container`'s, but at the same time my thoughts are as follows:

1. Chances of people misusing C++ so much are very low. That's something I can check tomorrow just by running the check on few large open source codebases to see whether it's true.
2. If people are misusing C++ it's their fault anyway. One can do whatever he likes to do. One can rewrite all interfaces to STL containers and make `size()` and `empty()` most sophisticated functions the world has ever seen while performing whatever strange operations inside of them. Thus, I am not very supportive of the idea "let's try to think about all weird cornercases".

I'm very curious to see how many false-positives there would be in LLVM codebase, for example. I'll try it tomorrow.


Repository:
  rL LLVM

https://reviews.llvm.org/D24349





More information about the cfe-commits mailing list