C++ class inheritance confusion

Question

Why do I get "error C2504: 'CEntity' : base class undefined" errors, when all the relevant headers are included?

I have a CMap which does most of the heavy lifting:

// CMap.h
#ifndef _CMAP_H_
#define _CMAP_H_
#include "CEntity.h"
class CMap {
    public:
        CMap(void);
        void OnLoad();
        void OnRender();
        std::vector<CTile*> TileList;
};
#endif

One of the things in the CMap is a list of Tiles:

// CTile.h
#ifndef _CTILE_H_
#define _CTILE_H_
class CEntity; // forward declaration

class CTile {
    public:
        CTile(void);
        std::vector<CEntity*> EntityList;
        char Label[0];
};
#endif

Each Tile contains a list of Entities:

// CEntity.h
#ifndef _CENTITY_H_
#define _CENTITY_H_
class CEntity {
    public:
        CEntity(void);
        char Label[0];
};
#endif

There are also various children of CEntity:

// CEntity_Buggy.h
#ifndef _CENTITY_BUGGY_H_
#define _CENTITY_BUGGY_H_
#include "CEntity.h"
class CEntity_Buggy : public CEntity {
    public:
        CEntity_Buggy(void);    
};
#endif

Now, my main map loading routine works fine, as does the rendering routing, which happens to need to peek into the Tiles for some info:

// CMap.cpp
#include "CMap.h"

void CMap::OnLoad() {
    ...
}

void CMap::OnRender() {
    /* here would be some rendering code ... */

    std::vector<CTile*>::iterator i;
    for (i=this->TileList.begin(); i!=this->TileList.end(); ++i) {
        CTile* tile = *i;

        for(unsigned int i = 0; i < tile->EntityList.size(); i++) {
            label[0] = tile->EntityList[i]->Label[0];
        }
    }
}

That works fine, and in the full app so far, it draws the cells, and adds the labels from the 'resident' entities. The problem comes when I want to put some specific entity subclasses into the system:

// CMap.h
#include "CEntity_Buggy.h" // add this to the header

// CMap.cpp
#include "CMap.h"

void CMap::OnLoad() {
    CEntity_Buggy buggy;
    buggy.OnLoad();
}

And now I get lots of \centity_buggy.h(18): error C2504: 'CEntity' : base class undefined errors, but I'm not sure why. My CEntity_Buggy.h includes CEntity.h.

The full current (and this is a first-C++ project, so it's quite flawed) is available on GitHub, if that helps.

Did you forget to put a semicolon at the end of your class definitions? — hatboyzero, Mar 08 '12 at 22:06
Are you writing your header file names in lower case, like `centity_buggy.h` but including them in upper case like `#include "CEntity_Buggy.h"`? It may not matter if this code is never ported off Windows but you will feel the pain on a case-sensitive filesystem. (This is likely not the problem; just a side note.) — Kaz, Mar 08 '12 at 22:19
Yeah, I'm on Windows at the moment, but from my black'n'white old web-dev days I picked up habits of case-sensitivity :) — Cylindric, Mar 08 '12 at 22:21
I commented out #include "CEntity_Buggy.h" from your CMap.h and it built fine for me. I'm going to have a closer look at it now. — Camford, Mar 08 '12 at 22:23
Yes, I got that too. The problem is that my CMap will need to instantiate Buggies when it loads the map file, in order to populate the map. — Cylindric, Mar 08 '12 at 22:29
@Cylindric. I added a comment below to OlduwanSteve's reply. The problem you are having is to do with how you manage header includes. — Camford, Mar 08 '12 at 22:50

score 3 · Accepted Answer · answered Mar 08 '12 at 22:25

3

The specific cause here is, I suspect, this chain of includes:

CEntity.h includes CCamera.h
CCamera.h includes CMap.h
CMap.h includes CEntity_Buggy.h

So CEntity.h is causing CEntity_Buggy.h to be included before CEntity has been defined. In this case it does not look like CMap.h really needs CEntity_Buggy.h - you could probably just include it in the cpp.

In general avoid including in .h files wherever humanly possible. Forward declaration is your friend :)

answered Mar 08 '12 at 22:25

OlduwanSteve

1,171
13
16

It's the circular deps that's been catching me, I think. CMap will need it I think, because I'm going to need to create CEntity_Buggies when I load the map. I suppose this is symptomatic of a poorly designed class hierarchy. – Cylindric Mar 08 '12 at 22:28
I was just about to say the same thing. Let this be a lesson for people who include too liberally. :p – Camford Mar 08 '12 at 22:30
I'm not sure how else to deal with global singleton objects - don't I have to include the headers in order to use them in other classes? `CCamera::CameraControl.GetX()` – Cylindric Mar 08 '12 at 22:32
NOt necessarily. Circular dependencies are useful and necessary. – Kaz Mar 08 '12 at 22:33
Caveat: I am guessing about what you're trying to do without looking too hard at the code! However probably entities are being dynamically created (i.e. using `new`) hence you will only be storing pointers to entities, not declaring entities as actual members of `CMap`.If you are only declaring a pointer to a type you can pre-declare the type in the .h file. You only need the actual class declaration (i.e. the header) when you want to call `new` which you will probably be doing in the cpp. – OlduwanSteve Mar 08 '12 at 22:37
@Cylindric In your particular case, it's largely not a design problem. It's a header #include's management problem. I'm looking at your code right now and you are including everything in the header file that defines your class. This is a bad idea. Think about what's happening when the preprocessor goes through each of your cpp file. This is how you end up with a system where every cpp file includes every .h file. – Camford Mar 08 '12 at 22:40
@OlduwanSteve But how do I do anything with the created pointer, without including the header? If I forward-declare the CEntity_Buggy class in CMap.h, I can create a pointer to a new buggy when I load the map, but I can't do anything with that pointer, such as set properties, without an "incomplete type is not allowed" error. – Cylindric Mar 08 '12 at 22:49
@Camford: Yeah, I'm seeing this now, but I'm not sure how else to access the objects I need. If my map needs to create buggys on load, and my buggys need to get the camera, it all gets a bit linked. Should I be passing stuff around in parameters rather than singletons? – Cylindric Mar 08 '12 at 22:51
@Cylindric Perhaps you are not fully understanding the distinction between .h and .cpp files? If you want to use a particular class in your _implementation_ of CMap (in the cpp) that does not mean you have to include the header in CMap.h - you can include it only in the cpp. As a rule of thumb the only times you ever include one class header in another are because you need to inherit from the class, or you need to use the class as a full (non-reference, non-pointer) member. – OlduwanSteve Mar 09 '12 at 07:40
Oh. I see. You ever had that thing where you seem to have missed something really basic? :D – Cylindric Mar 09 '12 at 09:39
It only seems basic once you know it. You just have to remember that unlike java or C# the compiler starts from (more or less) nothing every time it looks at the next .cpp file. That has good and bad sides, but you get used to it. – OlduwanSteve Mar 09 '12 at 13:46
Thanks to all the input on SO, it's working now. And I think I've learned a few important bits about C++ that I didn't know yet too :) – Cylindric Mar 09 '12 at 15:30

Kaz · Answer 2 · 2012-03-08T22:16:57.283

2

Take out the nonstandard #pragma once and do a full rebuild.

Do you have precompiled headers turned on in Visual Studio? That's a build-time bugtrap.

The portable method for once-only inclusion is

#ifndef IDENTIFIER
#define IDENTIFIER

// header contents

#endif

IDENTIFIER is chosen based on the header name, and if you're smart, some additional characters to decrease the likelihood that it clashes with anything. E.g. CENTITY_H_4D59_3FC4 (randomly chosen hex digits).

A decent compiler (e.g. gcc) recognizes this "ifdef ritual" and actually will not read the header, so it is as efficient as the #pragma once.

I'm writing this because, obviously, the centity.h header defines the CEntity class, and it is being included right above the definition of derived class CEntityBuggy. So why would it not define the class? Maybe #pragma once is buggy and has eaten the header, or the compiler is regurgitating some stale header material from a precompiled header cache.

Build-time stuff like this will have you scratching your head over correct-looking code.

edited Mar 08 '12 at 22:16

answered Mar 08 '12 at 22:10

Kaz

48,579
8
85
132

Compilers must read the code inside ifdef even if the condition is false because it may contain syntax errors. – Dani Mar 08 '12 at 22:18
I've checked the settings, and "Precompiled Headers" seems to be switched off. Changing all the `#pragma once` to "standard" guards makes no difference - although I didn't really expect it too, `#pragma once` isn't exactly obscure voodoo. – Cylindric Mar 08 '12 at 22:19
[This question](http://stackoverflow.com/questions/787533/is-pragma-once-a-safe-include-guard) covers pros and cons of `#pragma once`, but in this case I think you'd see the same results with a `#define` based include guard. – OlduwanSteve Mar 08 '12 at 22:29
@Dani: of course `gcc` reads the header once. It is fair to assume that during the same invocation of the compiler, the header is not being edited! The compiler knows not only that the condition is false but it knows that it's false because that header was included already; the trick is not based purely off the condition. What's optimized away is not only scanning the stuff between the `#ifndef ... #endif` but actually opening the file at all. – Kaz Mar 08 '12 at 22:32

C++ class inheritance confusion

2 Answers2