12

I just saw some C++ code like this. It was using a condition to decide whether to walk forward or backward through a std::vector. The compiler doesn't complain, but I thought size_t was unsigned. Is this dangerous?

vector<int> v { 1,2,3,4,5 };    
bool rev = true;

size_t start, end, di;
if (rev) {
    start = v.size()-1;
    end = -1;
    di = -1;
}
else {
    start = 0;
    end = v.size();
    di = 1;
}

for (auto i=start; i!=end; i+=di) {
    cout << v[i] << endl;
}
Rob N
  • 15,024
  • 17
  • 92
  • 165
  • 1
    The standard defines `std::string::npos` as `static const size_type npos = -1;` which is basically a similar trick. To be pedantic you might prefer: `std::vector::size_type start, end, di;`. – Galik Jan 31 '15 at 03:17
  • @Galic: I fail to see what the pedantry (as you call it) achieves, what advantage it has.If a maintainer goes out of his or her way to break things by instantiating `std::vector` with an allocator that provides a silly size type, all that the pedantry does is increase the chance the he or she succeeds in fouling up things. As I see it. – Cheers and hth. - Alf Jan 31 '15 at 03:24
  • @Cheersandhth.-Alf Well you're probably right. As far as I can tell though the standard says `std::vector::size_type` must be unsigned, but I think it can be a different size to `std::size_t`. Although I imagine most implementations will make `size_type` the same as `size_t`. – Galik Jan 31 '15 at 04:04

4 Answers4

10

It's well defined to use unsigned integers (and size_t is unsigned) this way, with wraparound: that behavior is guaranteed by the standard, as opposed to with signed integers, where it's not guaranteed by the standard.

It is however needlessly clever.

As a general rule, to avoid problems due to implicit wrapping promotions to unsigned, use unsigned integers for bit-level stuff, use signed integers for numbers. Where you need a signed integer corresponding to size_t there's ptrdiff_t for you. Define an n_items function with signed result, e.g.

using Size = ptrdiff_t;

template< class Container >
auto n_items( Container const& c )
    -> Size
{ return end( c ) - begin( c ); }

and you're set to go, no more sillywarnings from the compiler.

Instead of the too clever given code

vector<int> v { 1,2,3,4,5 };    
bool rev = true;

size_t start, end, di;
if (rev) {
    start = v.size()-1;
    end = -1;
    di = -1;
}
else {
    start = 0;
    end = v.size();
    di = 1;
}

for (auto i=start; i!=end; i+=di) {
    cout << v[i] << endl;

do e.g.

const vector<int> v { 1,2,3,4,5 };    
const bool reverse = true;  // whatever

for( int i = 0; i < n_items( v );  ++i )
{
    const int j = (reverse? n_items( v ) - i - 1 : i);
    cout << v[j] << endl;
}
Cheers and hth. - Alf
  • 142,714
  • 15
  • 209
  • 331
  • `n_items()` should be cached in a loop variable to avoid potentially, and potentially expensively depending on the container type, calling/calculating the size for every loop iteration. – underscore_d Nov 12 '18 at 22:55
3

Whenever I need to deal with signed types, I always use:

typedef std::make_signed<std::size_t>::type ssize_t; // Since C++11

...as a signed alternative to std::size_t.

I appreciate this question is a few years old, but I'm hoping that will help others. Credit to moodycamel::ConcurrentQueue.

ZeroDefect
  • 663
  • 1
  • 8
  • 27
1

I can't speak to how safe that code is but I think it's a pretty poor style. A better way would be to use iterators which support forward or reverse iteration.

For example:

std::vector<int> v = { 1, 2, 3, 4, 5 };
bool rev = true;

if (rev)
{
    for (auto itr = v.rbegin(); itr != v.rend(); ++itr)
    {
        std::cout << *itr << "\n";
    }
}
else
{
    for (auto itr = v.begin(); itr != v.end(); ++itr)
    {
        std::cout << *itr << "\n";
    }
}
James Adkison
  • 9,412
  • 2
  • 29
  • 43
  • Somehow that code doesn't compile with g++ 4.8.2. Can you think of any reason? – Cheers and hth. - Alf Jan 31 '15 at 03:18
  • 1
    I'm not sure (it might have to do with the fact that *rend()* and *begin()* won't evaluate to the same type). It was a conceptual answer; I didn't test it. I'll update with something that actually compiles. – James Adkison Jan 31 '15 at 03:25
  • 1
    Speaking of good style, you should declare in-loop variables to cache the `end` iterators to avoid potentially calling/instantiating a new copy for each iteration. ;-) – underscore_d Nov 12 '18 at 22:53
  • @underscore_d Have you checked? Sticking with common idioms provides the best opportunity for compilers to optimize your code, they generally do a better job than humans. :) – James Adkison Nov 13 '18 at 12:50
  • I get your point in the abstract, but here IMO writing `for (auto it = c.begin(), end = c.end(); it != end; ++it)` is so little extra typing - & it's so hard to imagine the optimiser could do any better - that I prefer to spell it out. I think some little things we can do to ensure optimality don't count as premature optimisation, but this may only be my definition! (What I don't like here is that `end` should be `const`, but it's not! But I tend to avoid temptation to declare it `const` outside the loop unless there's already other stuff there.) – underscore_d Nov 14 '18 at 15:18
0

Is it safe to use negative integers with size_t?

No, it is dangerous. Overflow.

size_t a = -1;
std::cout << a << "\n";

Output:

4294967295 // depends on the system, largest value possible here
gsamaras
  • 71,951
  • 46
  • 188
  • 305
  • 5
    This is not dangerous overflow, but well-defined wrap-around. Casting negative numbers to size_t is a common way to return errors from functions that normally return a size; the C standard library itself does this for example in functions like `mbrtoc32()`. – dpi May 17 '15 at 19:22