C/C++ include directives and explicitness

[PeterBowman]

From #12 (comment):

I just want to leave this here to illustrate a case where minimizing #includes is hazardous (which is vaguely related to this issue, but still...). Compare these two CI weekly cron builds of the tools repository on consecutive commits, no changes in between:

• https://travis-ci.org/roboticslab-uc3m/tools/builds/286195407
• https://travis-ci.org/roboticslab-uc3m/tools/builds/289191356

One cannot depend on headers that may or may not include another header, which in turn happens to be a must for your code to compile, and that's why I tend to prefer explicitness. This was the fix: roboticslab-uc3m/tools@da3ee49.

I think this is worth a mention in our guides. Two more situations I recently ran into:

• missing <sys/ioctl.h>: another transitive inclusion during a major refactorization effort which had nothing to do with the Jr3 device
• <yarp/dev/CanBusInterface.h> is (surprise!) not included in <yarp/dev/all.h>

The latter means that we may want to avoid omnibus headers of the sort of xxx/all.h. Is this acceptable?

Note: also reflect conclusions of #12.

[jgvictores]

I totally agree on advising against omnibus headers of the sort of xxx/all.h. 👍

[PeterBowman]

More refs, now focusing on self-sufficient headers:

• Self-sufficient header files in C/C++ (SO)
• Self-Sufficient Headers (Herb Sutter)
• Order of header inclusion (yarp interfaces) affects functionality robotology/yarp#1641

[PeterBowman]

<yarp/dev/CanBusInterface.h> is (surprise!) not included in <yarp/dev/all.h>

I presume this could be treated as an upstream bug. Nevertheless, good practice (and common sense?) dictates avoidance of omnibus header abuse. Another example is the IControlLimits2.h header (deprecated in YARP 3.0), which looks odd just beside the <yarp/dev/all.h> inclusion: roboticslab-uc3m/yarp-devices@96003c6.

[PeterBowman]

Looking at this the other way around, i.e. redundant/unnecessary included headers, the include-what-you-use tool seems an interesting approach: SO question.

[PeterBowman]

Someone may be tempted to give for granted the transitiveness of included headers under the assumption that usage requirements, once being reflected in the public API, are automatically propagated. Example:

// file "FancyInterface.hpp"
#include <SomeFancyDependency.hpp>

class FancyInterface
{
public:
    void doSomething(SomeFancyDependency & obj);
};

Let's say I'm an user of this interface and I want to invoke an implementation of FancyInterface::doSomething. The signature tells me it introduces the SomeFancyDependency. I am pretty safe to include FancyInterface.hpp and assume everything will work.

Now, see what happens in the following example:

// file "FancyInterface.hpp"
class SomeFancyDependency; // forward declaration

class FancyInterface
{
public:
    void doSomething(SomeFancyDependency * p_obj);
};

Since SomeFancyDependency is referred to by pointer, there is no need to provide further hints to the compiler apart from a forward declaration. This is a so-called incomplete type and can be found e.g. in yarp::os::PortReader. Now, if you implement FancyInterface::doSomething and fail to include SomeFancyDependency.hpp by yourself, the compiler will complain loudly.

Edit: actually, SomeFancyDependency needs not to be declared as a pointer.

[PeterBowman]

Also affects yarp::os::TypedReaderCallback. Implementors need to include yarp::os::TypedReader.