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
