[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas added a comment.

I also tested on LLVm code.
The output is here:
https://gist.github.com/tzolnai/fe4f23031d3f9fdbdbf1ee38abda00a4

I found 362 warnings.

Around 95% of these warnings are similar to the next example:

  /home/zolnai/lohome/llvm/lib/Support/Chrono.cpp:64:24: warning: loop variable has narrower type 'unsigned int' than iteration's upper bound 'size_t' (aka 'unsigned long') [bugprone-too-small-loop-variable]
    for (unsigned I = 0; I < Style.size(); ++I) {

Where the loop variable has an `unsigned int` type while in the loop condition it is compared with a container size which has `size_t` type. The actualy size method can be std::string::length() or array_lengthof() too.

An interesting catch related to a template function:

  /home/zolnai/lohome/llvm/tools/clang/lib/CodeGen/CGNonTrivialStruct.cpp:310:24: warning: loop variable has narrower type 'unsigned int' than iteration's upper bound 'size_t' (aka 'unsigned long') [bugprone-too-small-loop-variable]
    for (unsigned I = 0; I < N; ++I)

Where N is a template value with `size_t` type. If the container function is instantiated with a "large" value I expect the function won't work. I guess it never actually happens.

An other catch inside a macro:

  /home/zolnai/lohome/llvm/lib/ExecutionEngine/Interpreter/Execution.cpp:157:5: warning: loop variable has narrower type 'uint32_t' (aka 'unsigned int') than iteration's upper bound 'std::vector::size_type' (aka 'unsigned long') [bugprone-too-small-loop-variable]
      IMPLEMENT_VECTOR_INTEGER_ICMP(ne,Ty);
      ^
  /home/zolnai/lohome/llvm/lib/ExecutionEngine/Interpreter/Execution.cpp:123:24: note: expanded from macro 'IMPLEMENT_VECTOR_INTEGER_ICMP'
      for( uint32_t _i=0;_i<Src1.AggregateVal.size();_i++)

I can't see similar false positives what LibreOffice code produces. In the case of LibreOffice `short` type loop variables used to lead to false positives. I expect that in LLVM code `short` type is not used frequently in a loop. The closest case which can be interpreted as a false positive is the next one:

  /home/zolnai/lohome/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2432:21: warning: loop variable has narrower type 'unsigned int' than iteration's upper bound 'unsigned long' [bugprone-too-small-loop-variable]
      for (Chunk = 0; Chunk < NumBytes / sizeof(uint64_t); ++Chunk)

Where `Chunk` and `NumBytes` both have `unsigned` type. `sizeof(uint64_t)` leads to the warning, which won't make the whole expression actually bigger. It's a similar constant like value as enums, literals, etc, which are ignored by the check. I count around 10 similar use cases where I saw that the sizeof() is used on a type, so must be constant in runtime.

I did not see any other kind of false positives.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974