Re: [Pio-temporary] Re: [ pio-Patches-1293678 ] Enable compiler warnings without debugging support

[Roland C. <rc...@us...>]

On Monday 19 September 2005 18:20, Brian Wellington wrote:
> On Mon, 19 Sep 2005, Bas Wijnen wrote:
> 
> >>> Comment By: Brian Wellington (bwelling)
> >> Date: 2005-09-18 20:56
> >>
> >> Roland - I don't think your patch will work, because the
> >> specific warnings supported by gcc vary greatly by version
> >> (and since many distributors shipped patched gcc, even
> >> version checking isn't enough).
> >
> > Good point.  However, the warnings are not new: They are currently 
in
> > rules.make, and can be enabled with --enable-debug.  I would like 
to compile
> > with lots of warnings, but of course it shouldn't break the 
compilation.  Is
> > there a solution for this?
> 
> It looked like the attached patch added more warnings than were 
currently 
> in rules.make, but I could be wrong.

That is correct. I was reading the man page of gcc to see if there are 
more options that eventually could be added.

If --enable-warnings=yes is used, it will result in the same situation 
as the current, except that the warnings will not be enabled when the 
compiler is not called gcc (perhaps not a good idea, since it can 
also be e.g. gcc-3.3)

But I was experimenting with what would happen if I enabled nearly all 
compiler warnings, --enable-warnings=full. It turns out that a few 
useful additional warnings were given.

I've looked in gnome-compiler-flags.m4 (part of the gnome-common 
package) for ideas.  It contains an AM_macro that enables compiler 
warnings, but they are less strict than the collection of warnings we 
currently use. It think we should at least keep the current list of 
warning checks.

However, gnome-compiler-flags.m4 contains a nice for loop that will 
check the capability of gcc to use a certain -Wflag. Perhaps that can 
be used to determine the available gcc warning flags.

> >> I'm a bit confused as to the intent of the original patch -
> >> having debugging symbols in a binary doesn't really cause
> >> any burden other than increased disk usage.  Given that hard
> >> drives are effectively free (and a pioneers build with
> >> debugging symbols is still going to use way less space than,
> >> say, gnome) and debugging symbols are useful if there are
> >> bugs, I don't know why they should be split.
> >
> > The difference is more than a factor of 3 for the client (I didn't 
check the
> > others).  I don't think it is reasonable to ask people who aren't 
going to
> > submit bugs to install that.  At least for Debian they will be 
stripped
> > anyway, so that's no problem.  I don't know what other 
distributions do
> > though.
> 
> It might be a factor of 3, but on my system (FC4/x86), the total 
size of 
> the 6 installed binaries is only about 2.3M, and the total size of 
the 
> other installed files is about 1.4M (and wouldn't be affected by 
this).
> That said, I don't have any problem making this configurable.
> 
> > I have been building debugging versions of the debian packages, it 
might be a
> > good idea to put them on the web somewhere to ask people to use 
them if we
> > want more info.  Of course I can only build for my own 
architecture (although
> > I can provide a package which people can use to build it on their 
machine),
> > which is currently i386.
> 
> OK.
> 
> >> In my opinion, what should be split is the debug/warning
> >> code and the deprecation flags.
> >
> > Sounds good.
> >
> >> Splitting off -DDEBUG from warnings could also be useful,
> >> but I don't see any good reason why -DDEBUG and -g should be
> >> tied together - they're completely unrelated.
> >
> > The reason they're together (and called --enable-debug) is that 
you'll want
> > both when debugging.  I agree though that it makes sense to split 
them.
> 
> That's true, but I've also found cases where I've been trying to 
debug 
> something in gdb, and the huge amount of data printed when DEBUG is 
set 
> actually makes it harder to follow.
> 
> >> I'd suggest something like:
> >>   - --enable-gcc-warnings (-Wall, etc)
> >>   - --enable-debug (-DDEBUG)
> >>   - --enable-deprecation-checks (or
> >> --enable-glib-deprecation checks, etc)
> >>   - possibly a way to set all of these with one flag
> >
> > Sounds good.

--enable-warnings: for compiler warnings
--enable-debug: for -ggdb3 and -O
--enable-logging: for -DDEBUG
--enable-deprecation-checks -D*_DISABLE_DEPRECATED
--enable-developer-only

developer-only would turn on all four. 
./configure --enable-developer-only --disable-logging should then be 
able have all warnings and debug information, but without the 
logging.

> >>   - always build with -g unless CFLAGS is set, like most
> >> other autoconf-based software.
> >
> > I don't know about this.  Perhaps it makes sense that people who 
compile from
> > source have enough disk space anyway.  Distributions will strip 
the files
> > before installing (at least I know I will do that for Debian), so 
that should
> > be ok.
> 
> I'm not sure about debian, but a lot of RH/Fedora packages set 
CFLAGS 
> before invoking configure as part of the build process.  I know that 
> setting CFLAGS doesn't currently work for pioneers (or was that 
fixed? 
> I'm a bit behind), but that's a fairly common solution.

Using CFLAGS should work again.
 
> >> I could try to come up with a patch to do this if there's
> >> interest, but I'm not going to fight with autoconf unless
> >> there's consensus.

I've made many changes to the autoconf script already. It would not be 
hard for me to implement the mentioned changes.

> > In cases like these, I think it is a good idea to apply a patch 
which makes
> > the thing work, and move the report to the feature requests.  
While my patch
> > may not be the final solution, it does fix a problem (it's even 
more relevant
> > for the AI-empty-bank-maritime-trade bug).  How about the default 
policy of
> > fixing the bug first, and changing the thing later after we 
decided what's the
> > Right Thing(tm) to do in that situation.  In particular for bugs, 
it's not
> > acceptable to leave them open just because we don't know how we 
want to
> > restructure the code, IMO.
> 
> Agreed.  Your patch is definitely an improvement over the current 
code, 
> and should be applied.  The we can figure out what the best way to 
move 
> forward from there.

Agreed.
But since I've put some time into this, and I might as well finish my 
patch, I would like to propose the following:
In about 18 hours from now I can spend some time for Pioneers again. 
If I don't have a patch posted in 24 hours from now, let's apply the 
patch from Bas, otherwise look at the new patch.

Regards,
Roland