6

C/C++ implicit conversions can cause bugs. Here's one example:

int foo, bar;
scanf("%d", &foo);
scanf("%d", &bar);
float foobar = foo / bar;

If I input 7 and 2, it's not 3.5 as expected - it's 3 -> bug (let's ignore the buffer overflow).

gcc's -Wconversion warns about this kind of stuff, so I turned it on. But it just seems to me like it's going over the top with the warnings. For instance, this:

int foo;
float bar;
scanf("%d", &foo);
scanf("%f", &bar);
float foobar = foo / bar;

Causes a warning:

warning: conversion to 'float' from 'int' may alter its value [-Wconversion]

Even though it works as intended, returning 3.5 when I input 7 and 2. I know that float cannot represent all possible int values precisely, but I'm questioning if adding a cast here does anything to help - other than adding more code. In the example above, casting one of the int values in the expression to float actually changed the result, replacing an implicit cast by an explicit one in this example doesn't.

So much for float to int conversion warnings, but that's not all. There's also sign conversion warnings. Traditional example:

std::vector<char> chars = get_chars();
for (int i = 0; i < chars.size(); i++)
    std::cout << chars[i] << std::endl;

Causes this warning:

warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

I could use the more cumbersome unsigned int for the index variable instead - which will require further casting when I do calculations with it. All for the unusual situation that I have between 2^31 + 1 and 2^32 elements in my vector and don't do any calculations with the index variable that cast it to int.

So, my question is: Which conversion warnings make sense in practice? Does adding explicit casts to fix them really improve anything?

  • In your above vector iteration example you should use std::size_t, or better yet... iterators – Charles Salvia Aug 10 '14 at 0:37
  • 5
    It's impossible to reduce this to a rule, it depends on the circumstances. For example, I work in bioinformatics and warnings about signed vs. unsigned comparisons have to be taken very seriously, because strings larger than 2^32 are common, and your loop would cause a runtime error when i overflowed. time, Casts are useful as a form of documentation to let future readers know that you are aware of the conversion, and are doing it deliberately, and not just being feckless. – Charles E. Grant Aug 10 '14 at 0:39
  • 4
    In C++, if you divide the int 7 by the int 2, getting 3 is neither a bug nor a buffer overflow. If you don't want integer math, you should convert everything to float. – Steven Burnap Aug 10 '14 at 3:23
10

It goes kind of like this.

  1. The ability to use unsigned values and variables and real numbers of different precisions are not particularly common features in programming languages, but are critical in order for C/C++ to do its job.
  2. Mixing arithmetic types like these is a major cause of bugs in real, commercial C/C++ software. Trust me, I've seen plenty.
  3. So the C/C++ standards nail down conversion behaviour with great care. There are pages and pages on this stuff.
  4. Therefore we ask the compiler to do as much as it possibly can to help us find and avoid bugs in the conversion world. Some things are errors (contravene the standard); others are warnings (default is associated with possible or likely bugs).
  5. Now it is up to us to identify programming strategies that minimise the risk of bugs, and in particular that do not trigger warnings. That's our job.

Casts are bad. Every cast potentially papers over a bug or a warning or a design issue. We can't live without them, but the fewer the better. Most are avoidable, with proper design.

Your two examples all look like wrong code to me. In the first two examples you need to tell the compiler whether to use int, float or double arithmetic, as the defaults could easily be incorrect. The third example must use an unsigned loop variable. C/C++ is a very sharp knife and you need more care to avoid cutting yourself.

  • 3
    +1: Although I think of C++ as a chainsaw rather than a sharp knife - Its far from a surgeons scalpel, its crude, useful and powerful, can cut down a forest in a day or create great art in the hands of master - but will quickly take off an arm and a leg in the hands of the untrained or careless. – mattnz Aug 10 '14 at 7:28
  • 5
    @mattnz: Actually, it's both. It's like a machine shop full of tools with no safety guards. You will get hurt. – david.pfx Aug 10 '14 at 8:13
1

With GCC you'll find alot of the important warnings are already grouped together under -Wall and -Wextra, its also useful to use -Werror to turn warnings into errors.

0

The -Wconversion flag frequently gives false positives, so it may be a good idea to turn these warnings off unless you really need the rigor.

In most cases, I find -Wsign-conversion to be more useful as unintentional signed-to-unsigned conversions can often cause serious bugs.

It would be nice if GCC provided a separate flag that warns only about non-subtype conversions though (e.g. int to char).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.