Re: [PATCH] Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic

[Carlo Arenas <carenas@gmail.com>]

On Wed, Sep 22, 2021 at 11:28 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jeff King <peff@peff.net> writes:
>
> >> I wonder if the attached (with clean-up to remove the tracing cruft)
> >> would show us a better direction.  It feeds a single line
> >>
> >>      int dummy_for_dep_check;
> >>
> >> C "program" from the standard input of the compiler to tackle the
> >> "you are not supposed to be compiling an empty compilation unit"
> >> problem in a more direct way.
> >
> > That feels a bit like we're playing a game of chicken with the compiler
> > in terms of what it may complain about. For example, sparse will
> > complain:
> >
> >   foo.c:1:5: warning: symbol 'dummy_for_dep_check' was not declared. Should it be static?
> >
> > Might compilers ever learn to warn of the same thing?
>
> Certainly.  That is the reason why I said "direction", not
> "solution", and I do not think it is beyond our capability to come
> up with a minimal "C program" that would be lint clean to make it
> as a part of the "solution".
>
> For example, would sparse or compilers complain about this?
>
>     extern int incr(int); int incr(int i) { return i + 1; }
>
> > So I'd argue we should go even simpler, like:
> >
> > diff --git a/Makefile b/Makefile
> > index 3628d14f16..4597a126d0 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1277,7 +1277,7 @@ COMPUTE_HEADER_DEPENDENCIES = auto
> >  endif
> >
> >  ifeq ($(COMPUTE_HEADER_DEPENDENCIES),auto)
> > -dep_check = $(shell $(CC) $(ALL_CFLAGS) \
> > +dep_check = $(shell $(CC) \
> >       -c -MF /dev/null -MQ /dev/null -MMD -MP \
> >       -x c /dev/null -o /dev/null 2>&1; \
> >       echo $$?)
>
> I am all for this simplification.  It takes us back to the state
> before 1816bf26 (Makefile: Improve compiler header dependency check,
> 2011-08-30).  But I think that is more or less orthogonal to the
> "you are not supposed to feed an empty compilation unit" issue.

the problem really is IMHO, that we are passing around CFLAGS that
indicate 2 things:

1) attributes that are relevant to how we build and what
2) which diagnostics we want to use

Ævar's approach addresses implicitly part of it (most of the
diagnostics are added in config.mak.dev), but I think it still feels
like a knee jerk reaction to the problem, while creating an
maintaining something like "DIAGNOSTIC_FLAGS" which we will only pass
to the compiler when we want to use diagnostic (including -pedantic,
-Wpedantic and -Werror) would be a better approach.

Carlo