Makis
Makis

Reputation: 12988

Dangerous ways of removing compiler warnings?

I like to force a policy of no warnings when I check someone's code. Any warnings that appear have to be explicitly documented as sometimes it's not easy to remove some warnings or might require too many cycles or memory etc.

But there is a down-side to this policy and that is removing warnings in ways that are potentially dangerous, i.e. the method used actually hides the problem rather than fixes it.

The one I'm most acutely aware of is explicitly casts which might hide a bug.

What other potentially dangerous ways of removing compiler warnings in C(++) are there that I should look out for?

Upvotes: 2

Views: 912

Answers (6)

Tal Pressman
Tal Pressman

Reputation: 7317

Well, there's the obvious way - disabling a specific warning for parts of the code:

#pragma warning( disable : 4507 34 )

EDIT: As has been pointed out in the comments, it is sometimes necessary to use in cases where you know that the warnings are OK (if it wasn't a useful feature, there would have been no reason to put it in in the first place). However, it is also a very easy way to "ignore" warnings in your code and still get it to compile silently, which is what the original question was about.

Upvotes: 5

Chris Arguin
Chris Arguin

Reputation: 12008

I also enforce a no-warnings rule, but you are right that you can't just remove the warning without careful consideration. And to be honest, at times I've left warnings in for a while because the code was right. Eventually I clean it up somehow, because once you have more than a dozen warnings in the build, people stop paying attention to them.

What you described is not a problem unique to warnings. I can't tell you how many times I see someones bug-fix for crash to be "I added a couple of NULL checks". You have to go to the root cause: Should that variable be NULL? If not, why was it?

This is why we have code reviews.

Upvotes: 1

piotr
piotr

Reputation: 5745

I think it's a delicate issue. My view is that warnings should be checked thoroughly to see if the code is correct / does what is intended. But often there is correct code that will produce warnings, and trying to eliminate them just convolutes the code or forces a rewrite in a less natural way.

I recall in a previous release I had correct and solid code that produced a couple of warnings, and coworkers started complaining about this. The code was much cleaner and did what it was intented. In the end the code went on production with the warnings.

Also, different compiler versions will produce different warnings, so it becomes more pointless to enforce a "no warnings" policy when the result depends on the mood of the compiler developers.

I want to emphasize how important is to check all warnings at least once.

Btw I develop in C and C++ for embedded systems.

Upvotes: 2

Wim ten Brink
Wim ten Brink

Reputation: 26682

The biggest risk would be that someone would spend hours of development time to solve a minor warning that has no effect on the code. That would be a waste of time. Sometimes it's just easier to keep a warning and add a line of comment explaining why the warning occurs. (Until someone has time to resolve these trivial warnings.)

In my experience, resolving trivial warnings often adds two more days of work for developers. Those could make the difference between finishing before and after the deadline.

Upvotes: 0

Vatine
Vatine

Reputation: 21288

Commenting out (or worse, deleting) the code that generates the warning. Sure, the warning goes away, but you are more than just a little likely ending up with code that doesn't do what you intend.

Upvotes: 1

anon
anon

Reputation:

const correctness can cause a few problems for beginners:

// following should have been declared as f(const int & x)
void f( int & x ) {
  ...
}

later:

// n is only used to pass the parameter "4"
int n = 4;
// really wanted to say f(4)
f( n );

Edit1: In a somewhat similar vein, marking all member variables as mutable, because your code often changes them when const correctness says it really shouldn't.

Edit2: Another one I've come across (possibly from Java programmers ) is to tack throw() specifications onto functions, whether they could actually throw or not.

Upvotes: 6

Related Questions