[FFmpeg-devel] [PATCH] fix compilation (was Re: r12489 broke all builds)

[Måns Rullgård]

The Wanderer <inverseparadox at comcast.net> writes:

> M?ns Rullg?rd wrote:
>
>> Reimar D?ffinger wrote:
>
>>> Well, I do not like headers including everything too much either,
>>> because it makes it less clear which headers one is supposed to
>>> include.
>> 
>> Every file should include all headers it explicitly uses symbols
>> from. The only exception is when another header is documented to
>> include a required header, e.g. inttypes.h includes stdint.h.
>
> By "every file" here, do you include header files, or only non-header
> files?

Any file seen by the preprocessor.

> As I understand it, the principle being championed by those who support
> having headers routinely include other headers is that every header
> should provide everything which it requires - either by defining those
> things directly, or by #including another header which provides them.

Yes, that is one way of stating the idea.

> I am having a hard time seeing how this is an unreasonable expectation.
> (I would, in fact, be inclined towards it myself if I ever did much
> coding anymore.)
>
> Since constantly redefining symbols in multiple locations is obviously a
> bad thing for a variety of reasons, the only way to achieve this goal
> would appear to be to have headers frequently include one another.
>
>>> As an example (that probably can not happen in exactly this way, so
>>> just to give you an idea of what I think of) a file might include
>>> bswap.h, then later adds endian dependent code under #ifdef
>>> WORDS_BIGENDIAN. Later then someone removes the bswap.h include and
>>> suddenly WORDS_BIGENDIAN never gets defined since nobody included
>>> any other header - since it was not necessary, due to bswap.h
>>> including everything, stdint.h, common.h, config.h,...
>> 
>> By the above rule, any file that uses WORDS_BIGENDIAN should
>> explicitly include config.h.  It so happens, that common.h is (or
>> should be) documented to include config.h (when building FFmpeg), and
>> avcodec.h can be relied on to include common.h.  However, there is no
>> promise that bswap.h include any additional headers.
>> 
>> A more realistic example is a file, let's call it mpeg12data.c, that
>> includes, say, rational.h, and everything is fine.  One fine day,
>> someone changes rational.h to require stuff from common.h, without
>> adding the #include line.  Suddenly, mpeg12data.c fails to compile
>> because it doesn't include common.h, and has no reason to do so, not
>> directly using any of its symbols.
>
> If you are operating under the paradigm that headers do not #include
> other headers except as explicitly documented, then your argument makes
> sense - but it is exactly that paradigm which appears to be being argued
> against.

Allow me to clarify.  Headers should #include whatever they need, but
need not document precisely what they #include.  *If* a header is
documented as #including another header, only then may users of the
first header omit #including the second.

> If you are operating under the paradigm that headers should provide
> everything which they need (whether directly or by #include), then I do
> not see how this breakage would not be considered to be the fault of the
> person who changed what rational.h requires, for not also making the
> necessary change so that it also provides the things it now requires.
>
> Am I missing something?

Someone changed what rational.h required, without making sure it
somehow provided it.  That broke compilation of files that needed
things from rational.h but not from common.h, and hence did not
#include the latter.  Had rational.h been updated with the necessary
#include, as it was later, there would have been no problem.

>> It is not reasonable that backwards compatible changes to a header
>> file should require updating every source file that uses.  Just think
>> for a moment about public API headers.
>
> If you assume that any change to the header such that it would now
> require a new symbol should be accompanied by a change to the header so
> that it somehow provides that symbol, I don't see how this is a problem.

We seem to be in complete agreement.

-- 
M?ns Rullg?rd
mans at mansr.com