size_t and vtkIdType (aka int, long or long long)

Development
1

What’s your opinion about the use of size_t for arguments in VTK that are defined as vtkIdType? Look at this:

// p is a vector
std::size_t i, num = p.size();

vtkIdList *cell = vtkIdList::New();
cell->SetNumberOfIds(num); // what happens if num is bigger than int?

for (i = 0; i < num; i++) {
    cell->SetId(i, p[i]); // also here
}
2

size_t isn’t viable because the fact that vtkIdType is signed is important. The other is that 64-bit builds with 32-bit ID types are still useful (decent memory savings).

Usually we just static_cast the warnings away though proper checks should actually be done.

3

Python and Java do not have native unsigned integer types, so it’s good to limit the use of unsigned types in the VTK API. But I agree that vtkIDType is inconvenient when working with STL.

4

What’s a good practice? The compiler does not print any warning.

5

It may also come to pass that C++ STL changes from using unsigned to signed size types.

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1227r2.html

Ronald, -Wconversion should be able to warn.

6

This is a very bad idea :rofl: :joy: :crazy_face:

7

MSVC does it by default, so usually we fix the warnings from that compiler. Doesn’t make them not bugs in the other compilers.

1 Like
8

I think the most we can expect is an ssize() method (though not even that; seems we get std::ssize(container) instead). Hopefully these have assertions or something internally if the size does grow beyond std::ssize_t.

9

But the signed/unsigned is not the problem. It’s the range of both types.

10

By default, vtkIdType is configured to have the same width as size_t. The only time you will see a 32-bit vtkIdType with a 64-bit size_t is if you build VTK with VTK_USE_64BIT_IDS=OFF.

11

Signed vs. unsigned is a problem. This code can be optimized much more easily in C or C++:

for (int i = 0; i <= N; ++i)
    do_something(i);

because signed overflow is UB. Because it is UB, the compiler can assume it doesn’t happen and can unroll your loop. Change int to unsigned, and if the compiler can’t prove N isn’t UINT_MAX, it has to pessimize.

In any case, .size() on containers will be unsigned forever and it looks like std::ssize(container) will be the C++20 solution for these things.

But vtkIdType can be 32-bit even on 64-bit platforms, so range checking is actually always needed.

1 Like