wmarkham
Level: Master Delver

Rank Points: 125
Registered: 12-06-2004
IP: Logged

Within BackEndLib,

Types.h includes Wchar.h
Wchar.h includes Ports.h, for linux and Apple builds
Ports.h includes Types.h

I don't know why the Linux builds aren't affected as well...

In the OS X port, I broke the cycle between Types.h and Wchar.h. There were a few files that #included Types.h where I manually added #include "Wchar.h".

I just picked that somewhat arbitrarily, though, because it seemed easy. In what order should these headers depend on each other?

md5i
Level: Master Delver
Rank Points: 109
Registered: 03-25-2004
IP: Logged

wmarkham wrote:
Within BackEndLib,

Types.h includes Wchar.h
Wchar.h includes Ports.h, for linux and Apple builds
Ports.h includes Types.h

I don't know why the Linux builds aren't affected as well...

Although this may not be the best from a style point of view, this shouldn't break anything. Each of these three is wrapped in

#ifndef HEADER_H
#define HEADER_H
...
#endif

blocks, so although there may be a loop, it won't get past two, and the second include will be semantically empty.

[Edited by md5i at Local Time:04-23-2005 at 08:06 AM: Typo fix]

____________________________
Michael Welsh Duggan
(md5i@md5i.com)

wmarkham
Level: Master Delver

Rank Points: 125
Registered: 12-06-2004
IP: Logged

It is broken when GAME_BYTEORDER==GAME_BYTEORDER_BIG:

Assert.cpp includes Types.h, before any of the other headers
before any of its declarations, Types.h includes Wchar.h
before any of its declarations, Wchar.h includes Ports.h
at line 93 or so, Ports.h references USHORT, which has not been declared.

AlefBet
Level: Smitemaster
Rank Points: 979
Registered: 07-16-2003
IP: Logged

Include guards (#ifndef FNAME_H / #define FNAME_H / ... / #endif) do not actually solve the circular include problem. Usually (unless you really know what you're doing and have done it on purpose) a circular include is a problem waiting to happen, although there are situations where it will still work. These are usually situations where some or all of those includes are not actually needed by the module including them. In such a case, it can all work okay if everywhere else you #include "file1.h" but break if you #include "file2.h" somewhere. In that case, you can get lucky and the declarations will be in the right order if you start with the right file and in the wrong order otherwise.

So, I guess the bottom line is that this circular include may actually be a problem, but I haven't looked carefully at it so it may be that the includes are just done for convenience in the rest of the code, in which case it's fine.

By the way, the include guard idiom is actually to solve the multiple include situation where file1.h includes file2.h and file3.h does too, and then file4.h includes both file1.h and file3.h. In that case, the guards prevent file4.h from getting two copies of file2.h.

____________________________
I was charged with conspiracy to commit jay-walking, and accessory to changing lanes without signaling after the fact .

++Adam H. Peterson

ErikH2000
Level: Legendary Smitemaster

Rank Points: 2794
Registered: 02-04-2003
IP: Logged

It'd be preferable to not have the bizarre love triangle there. If somebody has a fix, we can try it out.

-Erik

____________________________
The Godkiller - Chapter 1 available now on Steam. It's a DROD-like puzzle adventure game.
dev journals | twitch stream | youtube archive (NSFW)

schep
Level: Smitemaster

Rank Points: 865
Registered: 03-01-2005
IP: Logged

I see no reason Types.h would need to include Wchar.h. And indeed, commenting out the #include "Wchar.h", a file that includes "Types.h" does compile (with and without _WINDOWS_ defined). There may be other headers or even sources that would need to add an #include "Wchar.h" to compensate for that change, though.

But speaking of _WINDOWS_, that part in Types.h with the #ifndef _WINDOWS_ bothers me. I can't tell whether it has to do with an assumption that something has been included before this file (eeeagh) or it's a test for whether <time.h> includes <windows.h> (does it?). Either way, it seems like testing the platform would be a better idea. Maybe there's some reason for this part I just didn't imagine.

wmarkham
Level: Master Delver

Rank Points: 125
Registered: 12-06-2004
IP: Logged

schep wrote:
But speaking of _WINDOWS_, that part in Types.h with the #ifndef _WINDOWS_ bothers me. I can't tell whether it has to do with an assumption that something has been included before this file (eeeagh) or it's a test for whether <time.h> includes <windows.h> (does it?). Either way, it seems like testing the platform would be a better idea. Maybe there's some reason for this part I just didn't imagine.

Yeah, that raised my eyebrow as well. My current guess is that it is a test to see if <time.h> includes <windows.h>. NULL, by the way, is in <stddef.h> (a.k.a. <cstddef>). (The comment next to the #include <time.h> says "Make sure I have the basics, like NULL. I don't know how portable this include is".)

Also, the solution you describe, schep, is what I did in the OS X patch I posted. I added #include <Wchar.h> or #include <BackEndLib/Wchar.h> to:

BackEndLib/MessageIDs.h
BackEndLib/Ports.h
FrontEndLib/ImageWidget.h
FrontEndLib/JpegHandler.h
FrontEndLib/PNGHandler.h


[Edited by wmarkham at Local Time:04-25-2005 at 06:27 PM: added info about the fix]

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

wmarkham wrote:
Also, the solution you describe, schep, is what I did in the OS X patch I posted. I added #include <Wchar.h> or #include <BackEndLib/Wchar.h> to:
...

Thank you, guys, for revising this. I've changed this in my source.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

wmarkham wrote:
Within BackEndLib,

Types.h includes Wchar.h
Wchar.h includes Ports.h, for linux and Apple builds
Ports.h includes Types.h

I don't know why the Linux builds aren't affected as well...

Just for the record, there's still an issue with this that I stumbled upon this weekend, so I'm breaking the cycle once and for all in 2.0.15. It's been committed to our cvs and will be released when 2.0.15 is official.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.