4 ms·

The one time I still use C casts in C++ is when comparing an int against the size of a container: if (i < (int)container.size()) This is because container.s

by cannam 12y ago

The one time I still use C casts in C++ is when comparing an int against the size of a container: if (i < (int)container.size()) This is because container.size() returns an unsigned type, and I try to avoid unsigneds in everyday code, and I like to use -Wall -Werror which, amongst other things, produces errors for signed/unsigned comparison warnings. I don't want to have to make the variable "i" unsigned, because using unsigneds for anything other than bit arithmetic is quite hazardous in practice (http://soundsoftware.ac.uk/c-pitfall-unsigned http://soundsoftware.ac.uk/c-pitfall-unsigned). How do others resolve this problem?

mappu 12y ago
You've traded one hazard for another there, now you have a bug at runtime if the container size is between the range of signed int and unsigned int. Hope that container size isn't user controlled. I'm guilty of a few C-style casts, but certainly prefer catching things at compile time than runtime. So +1 unsigned types for me. At least the compiler helps with catching signedness issues.
- cannam 12y ago
  Does the compiler help? $ cat unsigned.cpp #include <iostream> void f(unsigned int x) { std::cout << "x = " << x << std::endl; } int main() { f(-1); } $ g++ -Wall -Wextra -Wconversion -Werror unsigned.cpp $ ./a.out x = 4294967295 No error, no warning. The two are not really distinct types. See the link I posted above for more argumentation along these lines.
shin_lao 12y ago
You should use static_cast<>() and not C style cast. You should not care about signed/unsigned, use auto.
- cannam 12y ago
  I would prefer to use auto when iterating through the container, certainly. But much of the code I write is numerical stuff and "i" really is logically an integer rather than an iterator, arising as the result of some actual arithmetic. I find I have to do this quite often, and I'm not going to write static_cast<int>() every time. Yes, I can see why this isn't a good idea -- I'm pained by it myself. I'm just wondering what I might have overlooked among the possibilities available for this kind of code.
  - shin_lao 12y ago
    If you are writing numerical stuff, you should be using more explicit types such as std::uint32_t.
to3m 12y ago
I stopped doing this once 64 bits became a thing. There's nothing wrong per se with having your indexes signed, if you don't mind potentially not being able to access the entire array - though I'm not convinced using int is very forward-thinking, as they're fairly reliably still 32 bits - but there's everything wrong with casting size_t to int. For example, assuming 64 bit, suppose i is 0x1000 and container.size() is 0x100000fff. Now (int)container.size() is very likely to be 0xfff, meaning i is greater, and something goes wrong. Or - and this would be very likely true for 32 bit systems as well - suppose container.size() is 0x80000000. Now (int)container.size() is -2147483648, meaning i is greater, and something goes wrong again. A better option would be (assuming size_t is at least as wide as int, which is vanishingly unlikely not to be true on modern systems, and can anyway be checked for using a static assert) i>=0&&(size_t)i<container.size(). Then you know the cast won't produce nonsense.
- cannam 12y ago
  That is a good point. The motivation for casting the size to int, rather than the other way around, is to get it out of "unsigned world" (where people often reason badly about even simple arithmetic) and into "signed world" as simply as possible. Often where this is going on, the container is expected to be small, but of course that's a hazardous intuition as well. Maybe I need a little helper: template<typename T, typename C> bool is_in_range(T i, const C &container) { if (i < 0) return false; if (sizeof(T) > sizeof(typename C::size_type)) { return i < static_cast<T>(container.size()); } else { return static_cast<typename C::size_type>(i) < container.size(); } } then if (is_in_range(i, container)) ...

by cannam 12y ago

mappu 12y ago

You've traded one hazard for another there, now you have a bug at runtime if the container size is between the range of signed int and unsigned int. Hope that container size isn't user controlled. I'm guilty of a few C-style casts, but certainly prefer catching things at compile time than runtime. So +1 unsigned types for me. At least the compiler helps with catching signedness issues.

cannam 12y ago
Does the compiler help? $ cat unsigned.cpp #include <iostream> void f(unsigned int x) { std::cout << "x = " << x << std::endl; } int main() { f(-1); } $ g++ -Wall -Wextra -Wconversion -Werror unsigned.cpp $ ./a.out x = 4294967295 No error, no warning. The two are not really distinct types. See the link I posted above for more argumentation along these lines.

shin_lao 12y ago

You should use static_cast<>() and not C style cast. You should not care about signed/unsigned, use auto.

cannam 12y ago
I would prefer to use auto when iterating through the container, certainly. But much of the code I write is numerical stuff and "i" really is logically an integer rather than an iterator, arising as the result of some actual arithmetic. I find I have to do this quite often, and I'm not going to write static_cast<int>() every time. Yes, I can see why this isn't a good idea -- I'm pained by it myself. I'm just wondering what I might have overlooked among the possibilities available for this kind of code.
- shin_lao 12y ago
  If you are writing numerical stuff, you should be using more explicit types such as std::uint32_t.

to3m 12y ago

I stopped doing this once 64 bits became a thing. There's nothing wrong per se with having your indexes signed, if you don't mind potentially not being able to access the entire array - though I'm not convinced using int is very forward-thinking, as they're fairly reliably still 32 bits - but there's everything wrong with casting size_t to int. For example, assuming 64 bit, suppose i is 0x1000 and container.size() is 0x100000fff. Now (int)container.size() is very likely to be 0xfff, meaning i is greater, and something goes wrong. Or - and this would be very likely true for 32 bit systems as well - suppose container.size() is 0x80000000. Now (int)container.size() is -2147483648, meaning i is greater, and something goes wrong again. A better option would be (assuming size_t is at least as wide as int, which is vanishingly unlikely not to be true on modern systems, and can anyway be checked for using a static assert) i>=0&&(size_t)i<container.size(). Then you know the cast won't produce nonsense.

cannam 12y ago
That is a good point. The motivation for casting the size to int, rather than the other way around, is to get it out of "unsigned world" (where people often reason badly about even simple arithmetic) and into "signed world" as simply as possible. Often where this is going on, the container is expected to be small, but of course that's a hazardous intuition as well. Maybe I need a little helper: template<typename T, typename C> bool is_in_range(T i, const C &container) { if (i < 0) return false; if (sizeof(T) > sizeof(typename C::size_type)) { return i < static_cast<T>(container.size()); } else { return static_cast<typename C::size_type>(i) < container.size(); } } then if (is_in_range(i, container)) ...