Warnings - should I fix them?
Author
Quote:Original post by Promit
Fix all of them, and then turn on warnings as errors. That's the only way you'll ever learn.
I plan to, but how would I sort out warnings that aren't in my code? That ffmpeg header chucks out a fair few.
Author
Good.
And what would be the difference in doing the following:
or
Both seem to work. Does one have benefits over the other?
And what would be the difference in doing the following:
Something = static_cast<int>lua_tonumber(LuaState, 3);or
Something = (int)lua_tonumber(LuaState, 3);Both seem to work. Does one have benefits over the other?
The first is the C++ version, while the second is the old C-style version. You should prefer the first, since it is more explicit in what it is doing. With the C version, you could end up doing reinterpret casts, const casts, and static casts, whereas with the C++ versions you are explicit about what you want it to do, which means it's easier for the compiler to help catch errors for you.
Quote:Original post by swiftcoder
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.
On a project of any reasonable size, there should never be a situation where time out needs to be taken to remove all the warnings. As soon as a warning appears, it needs to be removed.
With warnings as errors, and a continuous build process, there should never be a situation where a platform is ignored and then needed to be tackled in bulk.
When a new or updated compiler is released, there will generally be one or two people tasked with testing and porting the code, removing the warnings/errors again as they appear.
A "warning hunt" is a sure sign that the development of the process is not being managed correctly.
For times when you are using a library that causes warnings internally, you either need to selectivly remove them or, if possible, contact the author and flag a bug with their project.
Spree
Author
Ok I've started the task of warning-killing. A few problems are giving me a headache, mostly my Vector3 and Vertex classes.
It seems a little daft to have a copy constructor for the class to accept integers, so how would I get around something like this:
The MapData[] is an array of unsigned ints. The x and z variables in the 'for' loop are ints (use floats for a 'for' loop?) and the Vertex class accepts floats.
That one line alone generates three warnings, complaining about the conversion from int to float.
I could use static_cast<>, but that would mean expanding that single line onto three lines, and the same for approx 30 similar cases. That would make a mess of an entire page of source.
Is this necessary? As far as I can see, the data types chosen are appropriate. The Vector3 and Vertex classes are used throughout the entire project, and 99% of uses involve floats. Changing the MapData[] to a float array would be unnecessary since only unsigned integers will be stored in it.
How do people get around issues like this? Use static_cast<> all over the shop?
It seems a little daft to have a copy constructor for the class to accept integers, so how would I get around something like this:
for (int x = 0; x < Width - 3; x++) { for (int z = 0; z < Height - 3; z++) { *TempVertex = SYM_VERTEX(x, MapData[x * Width + z], z);The MapData[] is an array of unsigned ints. The x and z variables in the 'for' loop are ints (use floats for a 'for' loop?) and the Vertex class accepts floats.
That one line alone generates three warnings, complaining about the conversion from int to float.
I could use static_cast<>, but that would mean expanding that single line onto three lines, and the same for approx 30 similar cases. That would make a mess of an entire page of source.
Is this necessary? As far as I can see, the data types chosen are appropriate. The Vector3 and Vertex classes are used throughout the entire project, and 99% of uses involve floats. Changing the MapData[] to a float array would be unnecessary since only unsigned integers will be stored in it.
How do people get around issues like this? Use static_cast<> all over the shop?
for (float x = 0.0f; x < Width - 3.0f; x+= 1.0f)
{
for (float z = 0.0f; z < Height - 3.0f; z += 1.0f)
There is no reason you can not use floats for this.
theTroll
{
for (float z = 0.0f; z < Height - 3.0f; z += 1.0f)
There is no reason you can not use floats for this.
theTroll
Quote:Original post by TheTroll
for (float x = 0.0f; x < Width - 3.0f; x+= 1.0f)
{
for (float z = 0.0f; z < Height - 3.0f; z += 1.0f)
There is no reason you can not use floats for this.
Aside from killing the performance of a trivial for loop, no.
Quote:That one line alone generates three warnings, complaining about the conversion from int to float
Yes, and it's appropriate. x coordinates are uniformly spaced, your float coordinates won't be. The deviation will be minimal, but it'll be there. So compiler is right - you are losing accuracy, although it might not matter.
Without knowing the entire context, using static_cast<>, or even (float) is appropriate here.
There's also alternatives (another constructor for creating vertices from a map):
Vertex::Vertex(unsigned int mapData[], int x_, int y_, int z_) : x(static_cast<float>(x_) , y(static_cast<float>(mapData[x_*Width+z_]) , z(static_cast<float>(z_){}If you change Map into a class (encapsulate construction):
struct Map { ... const Vertex & makeVertex(unsigned int x, unsigned int y, unsigned z) { float xv(x); float yv(mapData[x*Width+z]); float zv(z); return Vertex(xv, yv, zv); }private: unsigned int width; unsigned int height; std::vector<unsigned int> mapData;};And there's probably others.
I don't know how it is other places, but where I work, we always compile with the "treat warnings as errors" compile option. It can be a bit of a hassle to begin with, but once you get into the habit, it's not so bad.
Judging by some of the comments that other posters have made, it seems that this s pretty common, so it's a good habit to get into.
Judging by some of the comments that other posters have made, it seems that this s pretty common, so it's a good habit to get into.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
