It is currently Sat Aug 22, 2020 4:39 am


All times are UTC




Post new topic Reply to topic  [ 3 posts ] 
Author Message
 Post subject: [Resolved]: More compiler warnings ...
PostPosted: Mon Jul 04, 2011 9:52 am 

Joined: Thu Apr 28, 2011 9:07 pm
Posts: 7
Location: near Frankfurt/Main, Germany
My last patches made the compilation in the library/ subdirectory warning free. With this bunch of patches, all get's warning free, .e.g. even the examples.

glew.cpp contains a check "glXGetClientString == NULL". However, this can never be the cast, as this is defined in glew.h as "extern const char* glXGetClientString (Display *dpy, int name);". g++ 4.6 warns "can never be NULL".

I also noticed that there are 4 identical glew/ subdirectories now in the code base.

examples/Paging/main.cpp: the callback function unload() doesn't use the parameter "vol". "Q_UNUSED(vol)" or "(void)vol" will make the compiler not spit out an unused parameter warning.

library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractorWithNormals.inl in the CubicSurfaceExtractorWithNormals constructor: initialization order, m_meshCurrent must be initialized before m_regSizeInVoxels.

library/PolyVoxCore/include/PolyVoxCore/LargeVolume.inl: directly at the end of a for loop there's the code "Block<VoxelType>* block = getUncompressedBlock(x,y,z);". This produces an unused variable warning. I haven't looked if getUncompressedBlock() is called because of a side effect or not. If not, then this line can vanish. In the other case, the "Block<VoxelType>* block =" can vanish. If this produces a warning on other compiler, then "(void) getUncompressedBlock(x,y,z)" can be used, probably with a comment to hint to the side-effect.

library/PolyVoxCore/include/PolyVoxCore/SimpleVolumeBlock.inl: in the SimpleVolume constructor, the initizalization order for m_uSideLength and m_tUncompressedData doesn't match the header file.

examples/Paging/Perlin.cpp: there are two lines "float freq = mFrequency;" in this file where the variable isn't used.

library/PolyVoxCore/include/PolyVoxCore/Vector.inl: in the for loop "for(int ct = 0; ct < Size; ++ct)" the variable Size is unsigned, while "int ct" makes it signed. Chaning "int" to "uint32_t" (the type of Size) get's rid of the sign-comparison mismatch warning.

library/PolyVoxCore/include/PolyVoxCore/AStarPathfinder.h: in the constructor of AStarPathfinderParamsM the initialization order of hBias and isVoxelValidForPath doesn't match the variable ordering.

library/PolyVoxCore/include/PolyVoxCore/MeshDecimator.inl: the method "performDecimationPass" get's called with a parameter m_fMinDotProductForCollapse, which is never used.

In the same file, the method "canCollapseMaterialEdge" get's called with two parameters, which are both unused. I actually wonder if this method should vanish completely, it doesn't seem used.

library/PolyVoxCore/include/PolyVoxCore/Raycast.inl: in the Raycast constructor, the initialization order of m_result doesn't match the order of the header file.

library/PolyVoxCore/include/PolyVoxCore/SurfaceExtractor.inl: the method generateIndicesForSlice() doesn't use it's last parameter "m_pCurrentVertexIndicesZ".

examples/OpenGL/OpenGLWidget.cpp: the "wheelEvent" doesn't use it's parameter "wheel", I suggest a Q_UNUSED(wheel) here. I haven't looked in detail, maybe the method can go away completely.

library/PolyVoxCore/include/PolyVoxCore/AmbientOcclusionCalculator.inl: the three local variables "iRatioMax", "fRatioMax" and "fHalfRatioMax" are unused.


I again attached a patch.


Attachments:
fix-warnings.patch [9.03 KiB]
Downloaded 222 times
Top
Offline Profile  
Reply with quote  
 Post subject: Re: More compiler warnings ...
PostPosted: Mon Jul 04, 2011 9:25 pm 
Developer
User avatar

Joined: Sun May 04, 2008 6:35 pm
Posts: 1827
Thanks for the fixes. I've applied them all, though I changed some of them to do things differently. See the notes below.

@Matt - If you read this, is is possible to get the CDash nightly builds to use the '-Wall -Wextra' parameters? That way we can stop these warnings coming back in the future.

HolgerSchurig wrote:
glew.cpp contains a check "glXGetClientString == NULL". However, this can never be the cast, as this is defined in glew.h as "extern const char* glXGetClientString (Display *dpy, int name);". g++ 4.6 warns "can never be NULL".


I've changed these, though glew.cpp isn't actually our code. It's from http://glew.sourceforge.net/. You could let them know, and we'll also try to remember this change when we update in the future.

HolgerSchurig wrote:
I also noticed that there are 4 identical glew/ subdirectories now in the code base.


Yep, this is getting a bit silly now... I need to factor out some common code into an example framework.

HolgerSchurig wrote:
examples/Paging/main.cpp: the callback function unload() doesn't use the parameter "vol". "Q_UNUSED(vol)" or "(void)vol" will make the compiler not spit out an unused parameter warning.


Rather than using your '(void)variable_name' trick I've changed the function declaration by commenting out the variable name:

Code:
void unload(const ConstVolumeProxy<MaterialDensityPair44>& /*vol*/, const PolyVox::Region& reg)


This is a user-supplied function though, and in the general case the first parameter might be required. But let me know if this doesn't actually fix the warning.

I've done the same for some other unused variables you identified, and some I've removed completely. In the case of the MeshDecimator it's just a mess anyway - large parts of that need deleting as it is now handled inside the CubicSurfaceExtractor.

HolgerSchurig wrote:
library/PolyVoxCore/include/PolyVoxCore/LargeVolume.inl: directly at the end of a for loop there's the code "Block<VoxelType>* block = getUncompressedBlock(x,y,z);". This produces an unused variable warning. I haven't looked if getUncompressedBlock() is called because of a side effect or not. If not, then this line can vanish. In the other case, the "Block<VoxelType>* block =" can vanish. If this produces a warning on other compiler, then "(void) getUncompressedBlock(x,y,z)" can be used, probably with a comment to hint to the side-effect.


Yep, getUncompressedBlock() does have important side effects, but I've removed the assignment.

I'll make this thread as fixed, but it's probably work having a sticky thread for people to post things like this. I'll set one up. Also, I applied these changes by hand so let me know if I missed anything.

[Edit:] I created such a thread here


Top
Offline Profile  
Reply with quote  
 Post subject: Re: More compiler warnings ...
PostPosted: Mon Jul 04, 2011 9:52 pm 
Developer
User avatar

Joined: Sun May 11, 2008 4:29 pm
Posts: 198
Location: UK
David Williams wrote:
@Matt - If you read this, is is possible to get the CDash nightly builds to use the '-Wall -Wextra' parameters? That way we can stop these warnings coming back in the future.
Yes it is. It used to be built with all the warnings enabled but I lost that setting when I changed the build box. I should be able to do it in the next few days.

_________________
Matt Williams
Linux/CMake guy


Top
Offline Profile  
Reply with quote  
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 3 posts ] 

All times are UTC


Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
cron
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group
Theme created StylerBB.net