#1179 closed defect (fixed)
[PATCH] Enable more warning flags when using GCC
Reported by: | Owned by: | warmerdam | |
---|---|---|---|
Priority: | high | Milestone: | |
Component: | Build Problems | Version: | 4.4 |
Severity: | normal | Keywords: | |
Cc: |
Description
Several mistakes in the source might go unnoticed because the compiler fail to report them. Adding more warning flags will increase the chance of detecting such problems, and I suggest doing so. The attached patch increases the warning level and make sure the C code is compiled using CFLAGS and C++ code is compiled using CXXFLAGS.
Attachments (1)
Change History (5)
by , 19 years ago
Attachment: | warnings.diff added |
---|
comment:1 by , 19 years ago
Cc: | added |
---|---|
Owner: | changed from | to
I'll take this one unless someone else really wants it. I only plan to implement in 4.5, not 4.4.x.
comment:2 by , 19 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
OK, I have reviewed that patch, and come to realize that amoung other things it enables a big bunch of new strict warnings. The MapServer team has made no committment to addressing warnings beyond the "-Wall" warnings so I don't feel it would be appropriate to enable all these warnings by default. What I have done is add *another* configure switch to enable strict warnings: --with-warnings. By default we will configure to use -Wall. It is also possible to set specific warnings as an argument to --with-warnings, or to select --without-warnings to disable even -Wall. I also restructured the warnings logic in configure substantially, removing the two warnings related macros from aclocal.m4, and disentangling the warnings logic from the debug logic.
comment:3 by , 19 years ago
Several of the warnings emitted by gcc when increasing the warning level are considered errors by other compilers. Others are real bugs in the code, which go undetected because only -Wall is used. This is the reason I recommend and urge you to increase the default warning levels used when compiling mapserver. But I can live with the --with-warnings flag. The problem is that this will give more work for those enabling it, as they will try to get rid of all the problems detected when using it, and less work for the people introducing these problems without seeing the warnings. I'll soon try to compile mapserver using the compiler on other unix platforms, and report the bugs detected that way. I know for sure several of the signed/unsigned problems (using enums as ints) will trigger errors. More on this later.
comment:4 by , 19 years ago
Petter, I understand your point, but standard warnings level is something that really needs to be agreed upon by the developer community. Until 4.4 even -Wall reports loads of warnings and because there were so many warnings were pretty much ignored meaning that serious warnings were ignored under the assumption there were part of the background noise. The developers (especially Howard and I) put quite a bit of effort into cleaning up the warnings for -Wall for the 4.4 release with the intent that all warnings would be taken seriously and address promptly. I think this is progress. However, by introducing alot of new warnings, we now have rafts of warnings again. Many of which (like the unused parameters) I consider cruft not worth "fixing". If we just made these warnings standard, I think you are just going to see developers start ignoring warnings again. As a relative outsider to the MapServer development process, I would suggest that trying to push a very strict warning protocol is not the best place for you to contribute to MapServer. Of course, when you run into specific errors building MapServer on other platforms, we welcome your patches. We will try to incorporate them if practical - though support for compilers other than gcc, and MSVC++ is not really a priority for us (me at least). If you do wish to encourage more strict warnings policy, you might want to advocate on their behalf on the -dev list. For instance list each of the warnings options that you think we should have in our "standard" warnings, what the benefits are, and what sorts of warnings enabling it will trigger. If you get buy-in from several developers then it could be migrated into the standard list along with -Wall and we could fix up the associated warnings in the code.
Note:
See TracTickets
for help on using tickets.
Enable more warnings from GCC