On Thu, Nov 5, 2009 at 1:45 PM, Sid Sacek <ssacek@securewatch24.com> wrote:
-----Original Message-----
[ Steven Watanabe wrote ] I am not convinced that tracing through all the noise from a dozen different compilers is worth the effort from that standpoint. As far as I am concerned, the primary reason for eliminating warnings is that they are annoying to users.
Don't you believe that with the right syntax a coder can get every compiler to become happy?
No. I'm sorry I don't remember the details, but I've had code where fixing a warning in MSVC caused a warning in gcc, and then vice versa. There really wasn't a reasonable way to avoid it via code changes. We had to change the code to make gcc happy, then use pragmas to suppress MSVC. But I will say we still did it - ie we still found it valuable to get to zero warnings. And while I'm at it, since I've gone through this process with sizeable projects (ie Adobe applications): 1. Being at 0 warnings is worth it. Important to remember. 2. Once you get there, stay there. Much easier than getting there. 3. Fixing warnings can fix bugs. 4. Fixing warnings can *cause* bugs. For large 'sprints' of warning fixes, I'd say it causes more bugs than it finds. Way more. Once you get to 0, warnings will find more bugs than they cause, but not during the drive to 0. The key here is to make sure the person fixing the warning knows the code well. Just because you fixed the warning the same way the last 10 times, it doesn't mean the next occurrence will work the same way. And again, once you get to 0, new warnings will be fixed by their instigator, and fixed when the code is fresh in their minds - much more likely to be correct. Some specific warnings, and our conclusions: - missing virtual dtor - didn't find any bugs in our code, but it is not a bad warning in general, when you think about who might be using/maintaining your code in the future. - solution: add virtual dtor, make it protected, leave a comment about the warning - missing enum values in switch case - didn't find any bugs - seemed like a nice idea at first - lets you know if you missed a case. We tried various ways to restructure our code to take advantage of this warning (ie write the code so that if a new enum value was added you would get a warning about the switch). However, more often than not, the restructuring introduced bugs, or made the code more convoluted or ... etc (typically with how to deal with "default:" case). Maybe we were being "too smart by half" trying to make use of the warning. - I'd disable this warning if I could, otherwise I'd fix the code - order of member initialization list does not match order actual initialization (ie members are initialized in the order listed inside the class, not the order in the constructor mem-init list) - didn't find any bugs - you shouldn't rely on member init order, and if you ever need to, you should really comment it well - reordering your mem-int lists is an annoying waste of time - the warning basically says "hey, know your language". Should we warn whenever you use the comma operator or ?: because they esoteric parts of the language? (I once worked somewhere where these were banned for that reason...) - I'd disable this warning if I could, otherwise I'd fix the code etc. Can't remember the rest, but these 3 took up a lot of time in our code. The other was MS warnings for 64bit compatibility. And changing int to size_t without looking leads to bugs about how negative numbers are (not) handled... Note that as much as I hate some of the warnings, and for us overall the fixes caused more problems than they solved, I still think it is worth doing. CAREFULLY. Tony