Closed Bug 450196 Opened 12 years ago Closed 12 years ago
Don't use -Wconversion with GCC
Depending on which version you have, GCC's -Wconversion option is either completely useless for C++ (4.2 and earlier, where it told you about bugs that might have been introduced while converting a code-base from *pre-standard C* to something more modern), or enables a set of warnings that have tons of false positives on the Mozilla code base (4.3 and later, where it complains about things like assigning a PRInt32 into a PRInt8 variable, whether or not the actual range of values that the wider variable takes is at risk of truncation). Thus I recommend just dropping it altogether.
This was discussed and originally rejected in bug 409384. I am still in favor and dbaron is opposed.
It doesn't look to me like the -Wconversion issue, specifically, was addressed in all that much detail in that bug. I'm going to paste in the GCC manual's description of that option. In versions up through 4.2, it read: Warn if a prototype causes a type conversion that is different from what would happen to the same argument in the absence of a prototype. This includes conversions of fixed point to floating and vice versa, and conversions changing the width or signedness of a fixed point argument except when the same as the default promotion. Also, warn if a negative integer constant expression is implicitly converted to an unsigned type. For example, warn about the assignment x = -1 if x is unsigned. But do not warn about explicit casts like (unsigned) -1. The first paragraph describes a warning that is *completely useless* for C++ or indeed for modern C, because if everything is prototyped, nobody expects anything else. It'll complain for things like passing a value of type |float| to a function that's prototyped to take |float| arguments! The second paragraph describes a warning that might conceivably catch real bugs, but in practice will be so swamped by the junk warnings from the first paragraph that it, too, is useless. Recognizing this uselessness, the GCC team changed the effect of the option substantially in version 4.3. The new documentation reads Warn for implicit conversions that may alter a value. This includes conversions between real and integer, like abs (x) when x is double; conversions between signed and unsigned, like unsigned ui = -1; and conversions to smaller types, like sqrtf (M_PI). Do not warn for explicit casts like abs ((int) x) and ui = (unsigned) -1, or if the value is not changed by the conversion like in abs (2.0). Warnings about conversions between signed and unsigned integers can be disabled by using -Wno-sign-conversion. For C++, also warn for conversions between NULL and non-pointer types; confusing overload resolution for user-defined conversions; and conversions that will never use a type conversion operator: conversions to void, the same type, a base class or a reference to them. Warnings about conversions between signed and unsigned integers are disabled by default in C++ unless -Wsign-conversion is explicitly enabled. This sure sounds like it would be a useful warning - I can see where dbaron is coming from - but in practice it generates so much noise that it's no help at all. I'm in the middle of a test run right now but later I'll provide some examples.
I guess I'm ok with turning it off.
I should say that I'd be happy to help with an effort to go through the code and make it clean for gcc4.3's -Wconversion so we can turn it back on -- but right now there's so much noise in some files it's hard to find the actual error messages coming from changes I made. :-/
Pushed a fix to mozilla-central, revision 81039c7842be
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.