Re: GOP-PROP 5: build system output (final)

[Wols Lists]

On 09/08/11 11:07, Phil Holmes wrote:
> ----- Original Message ----- From: "Reinhold Kainhofer"
> <address@hidden>
> 
> [snip very well argued case]
> 
> Reinhold,
> 
> I know you have many 10s of times more experience with lilypond than I
> do, and I agree with 99% of what you say...  But...
> 
> The truth is, no-one pays any attention to warnings during the build
> process.  If I grep the output of make for the word "warning" I get 380
> lines in the middle of 37,000 other lines of output.  Most of these
> warnings come from other parts of the build system, but no-one's looking
> at them. There are nine warnings from the code compiler:
> 
> /flower/file-name.cc:100: warning: ignoring return value of 'char*
> getcwd(char*, size_t)', declared with attribute warn_unused_result
> /lily/glissando-engraver.cc:124: warning: comparison between signed and
> unsigned integer expressions
> /lily/lily-parser-scheme.cc:85: warning: ignoring return value of 'int
> chdir(const char*)', declared with attribute warn_unused_result
> /lily/lyric-combine-music-iterator.cc:224: warning: unused parameter 'sev'
> /lily/skyline.cc:395: warning: unused parameter 'a'
> /lily/lexer.ll:634: warning, rule cannot be matched
> /lily/lexer.ll:637: warning, rule cannot be matched
> /lily/lexer.ll:706: warning, -s option given but default rule can be
> matched
> out/parser.cc:2392: warning: conversion to 'short int' from 'int' may
> alter its value
> 
> If warnings are there to prevent code errors, why have these not been
> fixed?

Because they're known about and discounted as not very important? Nine
warnings, imho, is not very many for a code base the size of lilypond.
> 
> In practice, displaying warnings on the console is a waste.  It's really
> far, far better to put them in a file, where concerned individuals can
> grep the file, open it in an editor and view it, etc.  I would have got
> nowhere understanding the build system unless I routinely redirected the
> build output to file.

I'm afraid I'm with Reinhold. As a *programmer*, I consider it very bad
practice to ignore warnings. For the system to hide them from me, well !!!

You can't always get rid of warnings, but when I was setting guidelines
for junior programmers I said

(a) The warning level should be jacked up to max
(b) if you don't understand a warning message, then you investigate
until you know whether it's safe to ignore or not, AND IF YOU CAN GET
RID OF IT, YOU DO.

Given (a), why bother turning *up* the warning level, if you're then
going to hide the warnings? As for (b), ime some warnings can't be
suppressed, I know we had a vicious circle where trying to fix one
warning just caused another one.

And backing up Reinhold about the other point of "warnings usually
indicate a problem" I couldn't agree more. I took over maintenance of a
program that "mostly worked but was flaky". It had no warning level set,
and so defaulted to almost none. Over a period of a few months (it was a
lot or work) I jacked that up to max and fixed all the warnings. Yes
they were nearly all bugs. And while I could never confirm that this
programming mistake caused that problem, the program became much more
reliable as a result. That "out/parser" is a perfect example - it *may*
be innocuous, or it *may* be a serious problem. It really ought to be
checked out and fixed, and if I were a boss, I would have no qualms
whatever about saying "fix it". In fact, looking closer, I would say
*most* of those warnings should be fixed - if they're innocuous they're
easy to fix the code to make them go away, and if they're not innocuous
- well, lily could easily blow up.

Cheers,
Wol