Warnings at compile-time (GCC)

Bug #825957 reported by Hans Joachim Desserud
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
widelands
Fix Released
Low
Mark Scott

Bug Description

Noticed some warnings are printed when compiling Widelands. I know there have been some for a while, but I recently compiled it from scratch on Arch and it printed a lot more warnings than what I could remember (maybe due to newer version of gcc checking more thoroughly?)

Most of the warnings are due to unused variables or casting of variables, so nothing critical. Probably a lot of low-hanging fruit here for people who want to get involved in the development.

PS. See also bug 913369 for warnings reported by Clang, and bug 986611 for issues reported by cppcheck.

Related branches

Revision history for this message
Hans Joachim Desserud (hjd) wrote :
Revision history for this message
Hans Joachim Desserud (hjd) wrote :

A few things have changed since this bug was filed, so I'm attaching an updated list of warnings. Note that I enabled -Wextra which explains why it prints a lot of extra warnings compared to last time.

Note that I have also filed a bug report on the warnings given by clang (see bug 913369). It would also be nice if someone could file a similar report based on the output from Visual Studio (which I assume is what we use for compilation on Windows).

As I mentioned on the clang bug report, the current warning reports are a bit noisy and quite large. However fixing the issues reported would make it easier to get an overview of which issues remains and also make it easier to notice when new issues are introduced.

(I will likely post newer warning reports from time to time when larger parts of the code have changed in order to keep them up to date.)

Technical details:
Compiled widelands bzr r6183 with gcc 4.6.2 using
 cmake ../widelands -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX=/opt/widelands-clang/build/ -DWL_PORTABLE=true -DWL_EXTRAWARNINGS=true

Note that DWL_EXTRAWARNINGS adds -Wextra warnings in addition to -Wall which is standard.

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

I'm marking this as Triaged as it should contain enough information for someone to look into the issues and start fixing them. I realize no single person will probably fix all of these and close the bug report, but little by little I think it should be feasible to get rid of the warnings. :)

Changed in widelands:
status: New → Triaged
summary: - Warnings at compile-time
+ Warnings at compile-time (GCC)
Revision history for this message
SirVer (sirver) wrote : Re: [Bug 825957] Re: Warnings at compile-time

Just wanna add that llvm is maybe not such a low priority because AFAIK
new mac os X versions contain llvm as default compiler. Not sure though.

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

There's been a lot of changes over the last month, so here's an updated list of warnings.

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

We are now down to less than 100 warnings when building with GCC. :)

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

Up to 102 lines again, a large part of it seem to be caused by data type narrowing or differences between signed/unsigned. Shouldn't be too hard, assuming someone tracks down what these variables are set to in the first place and make their use more consistent.

Happy new year everyone! :)

description: updated
Mark Scott (mxsscott)
Changed in widelands:
assignee: nobody → Mark Scott (hono)
Revision history for this message
SirVer (sirver) wrote :

Thanks for taking this on, mark. It is not the most fun task wl has to offer, but it is much appreciated.

Revision history for this message
Mark Scott (mxsscott) wrote :

https://code.launchpad.net/~hono/widelands/warnings/+merge/142035 submitted for review, which, for me, compiling with GCC 4.7.2 gives a warning free build.

Mark Scott (mxsscott)
Changed in widelands:
status: Triaged → In Progress
Revision history for this message
SirVer (sirver) wrote :

Awesome! I especially love the GCC macro which reads beautiful in the code. I merged this as is, only adding a TODO for me to check if Rect::contains can be used in rt_render - very likely I only missed this when I wrote the code.

Compile time warnings are very tricky of course - other systems might still give some for the very same compiler, so I am not sure what to do with this bug now. I feel your work warrants a closed bug though, so I just go ahead and close it :P.

Changed in widelands:
status: In Progress → Fix Committed
Revision history for this message
Hans Joachim Desserud (hjd) wrote :

Great to see Widelands compiling warning free. :)

With a clean slate, it should make it easier to spot when new warnings are introduced or added by newer compiler versions/configuration. Any new warnings which pop up should probably be reported as separate issues.

I mark this for build18 since it was fixed in this release cycle.

Changed in widelands:
milestone: none → build18-rc1
Revision history for this message
SirVer (sirver) wrote :

Released in build-18 rc1.

Changed in widelands:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.