ztamas marked an inline comment as done. ztamas added a comment. In D59870#1444645 <https://reviews.llvm.org/D59870#1444645>, @JonasToth wrote:
> I think in general this is a good direction. What do you think about other > heuristics? > E.g. one could say that a vector will not contain more then X GB or similar. > That is probably more complicated to figure out though. Yes, it would be very complicated to find out the memory size of the container used in the code, I think. The used size method is not always the size() function. In llvm code I see also std::string::length() and array_lengthof(). Also a code might use a non-standard container, which might have different method for getting the size of the container which makes it hard to create a matcher for this and find out which container I need to get it's size. Also the check can catch loops which are not related to containers, so a container related option would have no meening in these cases. I think it's the easiest way to specify the bits of the ineteger type to limit the catches. In real life, I met with this overflow / infinite loop problem with 16-bit short type, so I think the real use cases are 8 and 16 bit integers. It seems intuitive to me to use the size of the loop variable's type to separate those catches which can lead broken functionality in practice from those use cases which are just integer incompatibilities. ================ Comment at: docs/ReleaseNotes.rst:122 +- The :bugprone-too-small-loop-variable + <clang-tidy/checks/bugprone-too-small-loop-variable>` now supports ---------------- Eugene.Zelenko wrote: > :doc prefix and ` is missing. Please also rebase patch from trunk. I'm a bit confused now about the code repositories, because of the recent changes. Which repo should I clone and rebase my commit? Is this one the trunk?: http://llvm.org/svn/llvm-project/llvm/trunk Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59870/new/ https://reviews.llvm.org/D59870 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
