Using templates with forward-declared types - safe?

Question

I'm building an header-only library, and I've resolved some circular dependency issues by doing something similar to what the code shows.

Basically, I create a private template implementation that allows me to use forward-declared types as if they were included and not forward-declared.

Is there anything dangerous about my approach?

Is there any performance loss? (the library's main focus is performance - the real code has explicit inline suggestions)

Bonus question: is there an impact (positive or negative) on compilation time?

---

// Entity.h
#include "Component.h"
struct Entity { void a() { ... } } 

---

// Component.h
struct Entity; // forward-declaration
class Component 
{        
    Entity& entity;
    template<class T = Entity> void doAImpl() { static_cast<T&>(entity).a(); } 

    public:
        // void notWorking() { entity.a(); } <- this does not compile
        void doA() { doAImpl(); }
}

Answer 1

It was sufficient to declare doA in Component.h, and define it in Entity.h.

---

// Component.h

class Entity;

class Component
{
    void doA();
}

---

// Entity.h

class Entity { ... }

// still in Entity.h
void Component::doA() { entity.a(); }

Answer 2

Is there anything dangerous about my approach?

As long as the template instantiation is, in fact, deferred, not much can go wrong. It could maybe declare intent a bit better if you prohibited an incorrect instantiation:

typename std::enable_if< std::is_same< T, Entity >::value
    && sizeof ( T ) /* Ensure that Entity is not incomplete. */ >::type

On second reading of your code, it looks like the non-template doA function will immediately and prematurely instantiate doAImpl, defeating the templating. I don't think the public interface can be a non-template, since it must cause instantiation of whatever Impl ultimately does the work, but only when the function is actually used. Unless there's another layer of templating protecting the user, it's probably better to do away with the private part and do everything in doA.

Is there any performance loss?

Nope. The function is sure to be inlined either way.

Is there an impact (positive or negative) on compilation time?

The minuscule added complexity will certainly not make a difference.

---

The only reason not to do this is obvious: it's ugly. And quite likely a violation of separation of concerns.

One workaround would be to use a non-member, free function instead.

struct Entity;
void a( Entity & );

    void doA() { a( entity ); }

Another would be to simply treat Entity.h or whatever as a dependency and include it. I think this would be the most popular solution.

If Component is really not dependent on Entity, then maybe doA belongs in a derived class which should have its own new header, which includes both the existing ones.

Answer 3

The code in the header Component.h will fail to compile unless you also #include Entity.h. This will result in mysterious errors in Component.h if you were ever to try to #include Component.h separately; to change Entity.h so that it does not contain a complete definition of Entity; or to change Library.h so that it no longer contains #include Entity.h. This is generally considered bad practice as this error would be difficult to understand for a future maintainer of the code.

clang gives error: member access into incomplete type 'Entity'

Here's a live example demonstrating the error: http://coliru.stacked-crooked.com/view?id=d6737c6f710992cce8a3f28217562da2-25dabfc2c190f5ef027f31d968947336

The function doAImpl() is instantiated if it is called in a context that does not depend on a template parameter. At the point of instantiation, Entity is used in a class-member-access and therefore required to be complete. If you do not #include Entity.h, the type Entity will not be complete at the point of instantiation.

A much simpler (and prettier) way to achieve what you want is to do:

template<class Entity>
class Component 
{        
    Entity& entity;

    public:
        void doA() { entity.a(); } // this compiles fine
};

In general, (even in a header-only library) you can avoid a lot of headaches by following this simple rule: every header name.h must have a matching name.cpp which contains #include name.h before any other #include directive. This guarantees that name.h can be safely included anywhere without resulting in this kind of error.

This is the canonical reference, from John Lakos in Large-Scale C++ Software Design, quoted by Bruce Eckel in Thinking in C++: http://bruce-eckel.developpez.com/livres/cpp/ticpp/v1/?page=page_18

Latent usage errors can be avoided by ensuring that the .h file of a component parses by itself - without externally-provided declarations or definitions... Including the .h file as the very first line of the .c file ensures that no critical piece of information intrinsic to the physical interface of the component is missing from the .h file (or, if there is, that you will find out about it as soon as you try to compile the .c file).

Answer 4

I would suggest to put implementation in "*.inl"

// Entity.h

#ifndef ENTITY_H
#define ENTITY_H

class Component; // forward-declaration
struct Entity { void a(); };

#include "Entity.inl" 

#endif

// Entity.inl

#ifndef ENTITY_INL
#define ENTITY_INL

#include "Component.h";
inline void Entity::a() { /* implementation using Component and Entity */}

#endif

// Component.h

#ifndef COMPONENT_H
#define COMPONENT_H

struct Entity; // forward-declaration
class Component 
{        
    Entity& entity;
    public:
        void doA();
};

#include "Component.inl"

#endif

// Component.inl

#ifndef COMPONENT_INL
#define COMPONENT_INL

#include "Entity.h";
inline void Component::doA() { entity.a(); }

#endif