Re: [PATCH 13/13] Installed header hygiene (BZ#20366): Test of installed headers.

[Carlos O'Donell <carlos at redhat dot com>]

On 08/29/2016 09:16 PM, Zack Weinberg wrote:
> As the final act in this patchset, this adds a test to make sure that
> the bugs I've been fixing stay fixed.  Each subdirectory checks each
> of the headers that it installs for two properties:
> 
>  * A source file that includes that header as (nearly) its first
>    action, includes no other headers, and makes one global definition
>    ("int avoid_empty_translation_unit;") will compile successfully, as
>    both C and C++, at a variety of language and library conformance
>    levels.  (I did not go for the full cross product mainly because
>    that would take ages, and also I'd have to know more about which
>    -std= options the compiler understands.  I think the set I picked
>    makes sense, but am happy to take feedback about what's
>    appropriate.)
> 
>  * None of the headers transitively included by that source file
>    contain any tokens that match a regexp-based blacklist: this is
>    currently enforcing non-use of the obsolete BSD u_* types.
> 
> In order for this test to work correctly, every wrapper header
> that actually defines something must guard those definitions with
>  #ifndef _ISOMAC.  This is the existing mechanism used by the conform/
> tests to tell wrapper headers not to define anything that the public
> header wouldn't, and not to use anything from libc-symbols.h.  conform/
> only cares for headers that we need to check for standards conformance,
> whereas this test applies to *every* header.  (Headers in include/ that
> are either installed directly, or are internal-use-only and do *not*
> correspond to any installed header, are not affected.)
> 
> The wrapper header adjustments were semi-mechanically generated, but I
> had to correct enough problems by hand that I don't think posting the
> script is worth it.

We minimally require gcc 4.7 to build glibc, and 4.7 was the fist to
includes support for -std=c11, so your set of tested modes looks good.
Similarly by gcc 4.7 the -std=c++11 support was almost complete, so
this is also OK.

As a whole this patch looks good to me. I appreciate that you included
testing in this patch set to prevent the changes from regressing.

I don't have a strong opinion about how this testing should be
architected, and what you have seems sensible at a high level.

I looked over the patches from a correctness perspective and I don't
see anything that jumps out as obviously wrong.

I would like someone else like Joseph to comment on the design though
since his experience maintaining conform tests would be useful here.

-- 
Cheers,
Carlos.