[squid-dev] RFC: policy change for header #includes

[Alex Rousskov]

On 3/7/23 00:38, Amos Jeffries wrote:
> 
> Current Policy 
> https://wiki.squid-cache.org/DeveloperResources/SquidCodingGuidelines#file-include-guidelines:
>   4. system C headers (with a .h suffix):
>      * mandatory HAVE_FOO_H wrapper


> I propose using the C++17 "__has_include(<foo.h>)" instead of HAVE_FOO_H 
> whenever we can. Which is:
>   * all .cc and .cci files
>   * any .h files in the src/ and tools/ areas


We should not use __has_include() instead of HAVE_FOO_H for most system 
include files because HAVE_FOO_H essentially means "can compile" while 
__has_include() means "exists". Essentially, __has_include() has the 
same problems that old AC_CHECK_HEADER and AC_CHECK_HEADERS macros had, 
before Autoconf changed them to check whether the header is not just 
present but can be compiled. We should not go back to unreliable 
existence checks IMO!

The semantic difference detailed above is sufficient to avoid the 
proposed change IMO, but I can also mention another significant problem: 
The increased difficulty in deciding when to use __has_include() and 
when HAVE_FOO_H. We already suffer from this problem (a little) in some 
special "Do not use HAVE_X_H for x.h!" cases, but unreliable 
environment-dependent __has_include() checks are likely to make it 
significantly worse IMO.

If we should not use __has_include() instead of HAVE_FOO_H for _most_ 
include files, then we should not use it at all: Code consistency and 
less bickering during reviews are worth the extra ./configure 
changes/time IMO, especially since we do not add new system headers that 
often.


HTH,

Alex.
P.S. To save time, I am not detailing my partial disagreement with the 
pros/cons statements quoted below.


> Pros:
>   * less configure.ac logic, smaller build logs
>   * removes need to do a full bootstrap + re-configure when a 
> third-party library or system header is added/remove/changed
>   * conversion can be semi-scripted
> 
> Cons AFAIK are all about how it cannot be a blanket requirement like 
> HAVE_FOO_H is:
>   * sometimes headers need to be forbidden include by ./configure logic 
> checks
>   * C language does not support the __has_include