Subject: Re: [PATCH 3/3] Makefile: use -Wdeclaration-after-statement if supported

From: Adam Spiers <[email-masked]>
Date: Mon, 17 Dec 2012 02:15:01 +0000

On Sun, Dec 16, 2012 at 05:52:05PM -0800, Junio C Hamano wrote:
> Adam Spiers <[email-masked]> writes:
> 
> > If we adopt this approach,...
> > diff --git a/Makefile b/Makefile
> > index a49d1db..aae70d4 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -331,8 +331,13 @@ endif
> >  # CFLAGS and LDFLAGS are for the users to override from the command line.
> >  
> >  CFLAGS = -g -O2 -Wall
> > +GCC_DECL_AFTER_STATEMENT = \
> > +	$(shell $(CC) --help -v 2>&1 | \
> > +		grep -q -- -Wdeclaration-after-statement && \
> > +	  echo -Wdeclaration-after-statement)
> > +GCC_FLAGS = $(GCC_DECL_AFTER_STATEMENT)
> > +ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) $(GCC_FLAGS)
> >  LDFLAGS =
> > -ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
> >  ALL_LDFLAGS = $(LDFLAGS)
> 
> Please do not do this.
> 
> People cannot disable it from the command line, like:
> 
>     $ make V=1 CFLAGS='-g -O0 -Wall'
> 
> If anything, this should be part of the default CFLAGS.
> 
> More importantly, this will run the $(shell ...) struct once for
> every *.o file we produce, I think, in addition to running it twice
> for the whole build.

[snipped]

OK; I expect these issues with the implementation are all
surmountable.  I did not necessarily expect this to be the final
implementation anyhow, as indicated by my comments below the divider
line.  However it's not clear to me what you think about the idea in
principle, and whether other compiler flags would merit inclusion.

(And also, please don't let this discussion hold up acceptance of the
two prior patches in the series.  Even though they are independent,
they are somewhat logically related so I grouped them into the same
series, although I'm not sure if that was the right thing to do.)