Jump to content
  • Advertisement
Sign in to follow this  
deadstar

Warnings - should I fix them?

This topic is 3744 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

My engine is now (unsurprisingly) riddled with warnings, since I'm not a brilliant programmer to say the least. I now have a collection of about 50 - 80 things like this:
warning C4244: 'return' : conversion from 'const int64_t' to 'int', possible loss of data
warning C4244: 'argument' : conversion from 'float' to 'int', possible loss of data
warning C4800: 'int' : forcing value to bool 'true' or 'false' (performance warning)
warning C4244: '=' : conversion from 'float' to 'int', possible loss of data
warning C4018: '<' : signed/unsigned mismatch


As far as I'm aware, it works 100% on both Windows and Linux. It's been tested rigorously. What kind of problems will these warnings give me, if any. Can I get away with disabling the warning output, or is this something serious I should be addressing as soon as possible? Most likely I'll go through all of them and fix them since I'd like to tidy up my code, but I was just wondering.

Share this post


Link to post
Share on other sites
Advertisement
Those floats to ints just mean that your number now has no decimals. So float x=1 is the same as int x=1. If it were 1.1, then it would stay float. I would just suggest initializing things as ints if they are not going to involve decimals.

Share this post


Link to post
Share on other sites
I'm not a brilliant programmer either, but the compiler wouldn't really generate warnings if there was nothing to warn you about. Warnings are often caused when you have a syntactically correct statement that might not do what you expect. For example, you have a couple conversions from float to int in which your numbers lose precision due to forcing a decimal to become an integer. The compiler wants to make sure you know what you are doing because it assumes that if you created a float it was because you needed more precision than an int, but here you are losing that precision. Some warnings, though most likely signs of poor programming, are harmless, but some may really screw up your application.

The best solution would be to re-think your code. Look, for example, at the use of your float. Why are you converting a float to an int? Do you really just need an int? Maybe you should redesign your function to accept floats instead? These are the sorts of things you can think about and very easily use to determine whether you can do away with your warnings. Though not necessary, fixing the warnings can only improve the maintainability and stability of your code.

Share this post


Link to post
Share on other sites
Quote:
Original post by deadstar
I am going to fix them. Just wondering what people's take on them was.
A lot of larger projects have a "warning hunt" every now and again, much like a "bug hunt" where a bunch of programmers very familiar with code-base just go through and eliminate every warning that can be removed without major code restructuring.

The reason for this is typically portability - even though you currently have Windows/Linux support, you will find that those warnings become errors in newer compiler versions, compilers for embedded (consoles), etc. For instance, many programmers had a nasty shock when GCC started enforcing const-correctness.

Share this post


Link to post
Share on other sites
Always set warnings to maximum possible (/W4 or -pedantic or whichever you prefer). After short while, you'll realize that majority of them are trivial.

But warnings are useful since they often expose underlying problems with design.

When you get type conversion warning, check if the conversion is really needed, or why it occurs. If you find it's necessary, use static_cast<>. When comparing signed/unsigned, it's similar. Why are you mixing those two types? It may expose some other problems which would occur under unusual circumstances.

Quote:
Can I get away with disabling the warning output


I'll close my eyes and play in traffic. If I can't see cars, they can't hit me.

Look at it this way - compiler is trying to tell you about possible problems. Why ignore it. You'll be fixing enough bugs already, at least here someone's point out where to prevent them.


Personally, for libraries and unit tests, I run dual compilation. One under MVC with maximum warnings (including those disabled by default) and disabled language extensions, the other under GCC with maximum warnings. On both, I set warnings as errors. This approach obviously doesn't work with code that requires platform specific includes (such as windows.h)

After this passes, I sometimes run static analysis on code. And it's not uncommon to get dozens to hundreds of warnings per compilation unit.

C++ is hell. Use any tool you have to point out possible problems. If anything, compiler's errors are too mild.

Share this post


Link to post
Share on other sites
Quote:
warning C4244: 'return' : conversion from 'const int64_t' to 'int', possible loss of data


This one's could be dangerous, depending on the meaning of that integer. Where do you get a int64_t from, and why do you need to cast it down to an int?

Quote:
warning C4244: 'argument' : conversion from 'float' to 'int', possible loss of data


If your intentions are to cast a float to an int (you don't need the decimal part), then just add a static_cast<int>() and the warning will diseappear.

Quote:
warning C4800: 'int' : forcing value to bool 'true' or 'false' (performance warning)


That's an easy one, I beleive you are doing something like "bool ok = x;", where "x" is an integer expression. Be explicit and change your code to "bool ok = x != 0;", if that's what you want.

Quote:
warning C4018: '<' : signed/unsigned mismatch


I've seen this often when comparing size_t to int. Make sure you're using the appropriate data types. If you don't want to change your data types, use a static_cast<>.

Share this post


Link to post
Share on other sites
Quote:
Original post by Trillian

Quote:
warning C4800: 'int' : forcing value to bool 'true' or 'false' (performance warning)


That's an easy one, I beleive you are doing something like "bool ok = x;", where "x" is an integer expression. Be explicit and change your code to "bool ok = x != 0;", if that's what you want.


Missed that one. That's an important one, since it can reveal a typo.

bool foo(int x)
{
return x=17;
}

int main()
{
foo(5);
}


foo should say:
return x==17;

Share this post


Link to post
Share on other sites
Quote:
Original post by Trillian
Quote:
warning C4018: '<' : signed/unsigned mismatch

I've seen this often when comparing size_t to int. Make sure you're using the appropriate data types. If you don't want to change your data types, use a static_cast<>.

That is more dangerous than it looks at first blush - size_t is not in any way guaranteed to be the same size as int, especially on 64-bit platforms. Prefer size_t whenever dealing with std::containers.

Share this post


Link to post
Share on other sites
Quote:
warning C4244: 'return' : conversion from 'const int64_t' to 'int', possible loss of data


This one isn't mine, it's from the ffmpeg includes.

Quote:
warning C4800: 'int' : forcing value to bool 'true' or 'false' (performance warning)


And this one is when I register a Lua function to a C++ function that takes a bool. I might alter the function to take an int instead.


Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!