Re: [RFC PATCH] Makefile: create externcheck target

[Junio C Hamano <gitster@pobox.com>]

Denton Liu <liu.denton@gmail.com> writes:

> +externcheck: $(filter-out $(THIRD_PARTY_SOURCES),$(filter %.c %.h,$(shell $(FIND_SOURCE_FILES))))
> +	sed -i 's/^\(\s*\)extern \([^(]*([^*]\)/\1\2/' $^

I am not enthused by this particular patch for a few reasons, and I
am moderately negative on the whole idea.

 - We don't aim to support "only GNU and some BSD"; let's not do
   "-i" which as far as I know is used only in config.mak.uname for
   vcxproj target (which is OK as we know that is run only on a very
   narrow target, but probably is a bad idea as it would be another
   source of copy-and-paste for those who do not even think why it
   is acceptable there but not in all other places).

 - Same for \s.  If it is easy enough to write [        ]*, why
   risk breaking for somebody you don't even know?

 - The initial [^*] may be an attempt to avoid triggering on a
   global pointer-to-function, but doesn't it also make the pattern
   fail to trigger on a global function whose return type is a
   pointer-to-function?

 - If this is a "check" target, we shouldn't apply a wholesale
   transformation that is potentially buggy to user's files.  Using
   "grep" to just point out the places where your opinion differ
   from user's (and to fail the "make foocheck" operation) would be
   more appropriate.

Quite honestly, I suspect that the "push" that b199d714 (*.[ch]:
remove extern from function declarations using sed, 2019-04-29)
talks about was misguided in the first place.

Sure, we can write these external function declarations without
'extern' in front, because the language allows it and without
'static' in front, it by default is 'extern'.  It however does
not automatically mean we _should_ drop 'extern'.

Sure, for function decls, it may not make a difference to have or
not have "extern" in front, but for decls of data (including
pointers to functions), it makes a whole lot of difference.  Not
standardising to the rule "our external declarations always are
marked with leading 'extern', regardless of the type of the
identifier being declared" forces us to spend our brain cycles to
think if we should or should not write 'extern' in front.  And is
that a good thing to spend our brain cycles on, or just waste of our
effort?  I am moderately in favor of saying that it is a waste.

In addition, seeing these 'extern ' in header files will train our
eyes to spot the same in the source files more easily.  External
decls in the source (as opposed to inclusion of a header that does
the decls) can happen but they ought to be exceptions and it is good
to make them stand out.