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

From: Adam Spiers <git@adamspiers.org>
Date: Dec 17, 2012, 02:15 UTC
Message-ID: <20121217021501.GA13745@gmail.com>
In-Reply-To: <7vk3shphru.fsf@alter.siamese.dyndns.org>

On Sun, Dec 16, 2012 at 05:52:05PM -0800, Junio C Hamano wrote:

Show 32 quoted lines

> Adam Spiers <git@adamspiers.org> 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.)