[patch 0/3] Header file reduction.

Jeff Law law@redhat.com
Tue Oct 6 21:55:00 GMT 2015


On 10/05/2015 03:11 PM, Andrew MacLeod wrote:
>
> In any case, a direct include of obstack.h in coretypes.h was considered
> earlier in the aggregation process and it didn't show up as something
> that would be a win.  It is included a couple of common places that we
> have no control over..  in particular libcpp/include/symtab.h includes
> obstack.h and  is included by tree-core.h.  A very significant number of
> files bring that in.  If we included obstack.h in coretypes.h then those
> files would be including it again for a second time for no particularly
> good reason.  So I made the judgement call to not put it in coretypes.h.
And just as important, we can revisit the aggregators and when we do so, 
we ought to be able to answer the question, "if obstack.h is put into 
coretypes.h" does that clean things up elsewhere and re-run the tools to 
clean things up.


>
>> And it's one example, but it does point out a problem with this sort
>> of automated approach: realistically no one is going to check the
>> whole patch, and it may contain changes that could be done better.
>
> The point being that the aggregation *wasn't* automated... and has
> nothing to do with this patch set.    I analyzed and preformed all that
> sort of thing  earlier. Sure judgment calls were made, but it wasn't
> automated in the slightest.   There are certainly further aggregation
> improvements that could be made... and either I or someone else could do
> more down the road.,  The heavy lifting has all been done now.
Agreed.

>
> So  the *only* thing that is automated is removing include files which
> are not needed so that we can get an idea of what the true dependencies
> in the source base are.
Also agreed.

>>> the reduction on all those files will take the better part of a week.
>>
>> That's a little concerning due to the possibility of intervening
>> commits. I'd like to make one requirement for checkin, that you take
>> the revision at which you're committing and then run the script again,
>> verifying that the process produces the same changes as the patch you
>> committed. (Or do things in smaller chunks.).
>>
>
> Well, sure there are intervening commits.. the only ones that actually
> matter are the ones which fail to compile because someone made a code
> change which now requires a header that wasn't needed before.  which is
> really a state we're looking for I think.   I fix those up before
> committing.   Its *possible* a conditional compilation issue could creep
> in, but highly unlikely.
More likely is conditional compilation will be removed :-)  We're trying 
to get away from conditional compilation as a general direction.

Intervening commits are always a problem with this kind of large patch 
that hits many places.   But IMHO, they're an easily managed problem.

jeff




More information about the Gcc-patches mailing list