0.9 Build fixes for OSX

[grrussel]

Here is a diff to allow OpenXCOM 0.9 (or at least, todays current git source) to build on OSX 10.7 via Makefile.simple as was possible before in 0.5

Fixes are a) not including headers not present (malloc.h) and b) OpenGL includes, use of -framework, and not redefining glCreateProgram etc

Since now there is a linker command line difference e.g. OSX adds -framework OpenGL to link step, there is now a named OSX target in the makefile (As was there previously for Dingoo)

[grrussel]

Hi, the OSX git build has broken again since
Commit 6024bedf3cca3855a5b00b315b5a24d8530964e9 by neo_nihilist
rewrite of unitOpensDoor() function.

Undefined symbols for architecture x86_64:
  "OpenXcom::MapData::O_WESTWALL", referenced from:
      OpenXcom::TileEngine::unitOpensDoor(OpenXcom::BattleUnit*, bool) in TileEngine.o
  "OpenXcom::MapData::O_NORTHWALL", referenced from:
      OpenXcom::TileEngine::unitOpensDoor(OpenXcom::BattleUnit*, bool) in TileEngine.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [../bin/openxcom] Error 1

The fix patch is attached,

[Warboy1982]

please try a clean make/try a fresh source dir/flush your .obj cache and get back to me with results.

[grrussel]

Clean builds make no difference to the presence of the linker error.

It looks like an instance of the following: https://stackoverflow.com/questions/1312241/using-a-static-const-int-in-a-struct-class/1312267#1312267

[DarkDefender]

Are you sure this is not a bug with clang/llvm? It complies fine with gcc and MSVS.

[Warboy1982]

but... pathfinding.cpp has been using those defines for years now and never had a problem...

[grrussel]

Mm. For whatever reason, that last change made it break.

It may be a bug in clang/LLVM, or a happy portability problem, or just laxity in other compilers?

At any rate, it is breaking the default C++ compiler on OSX 10.7 , and the fix is trivial to patch in.

[grrussel]

A little more digging suggests its a language extension. https://msdn.microsoft.com/en-us/library/34h23df8(v=vs.80).aspx

Under the standard (/Za), you need to make an out-of-class definition for data members. For example,
class CMyClass  {
   static const int max = 5;
   int m_array[max];
}
...
const int CMyClass::max;   https:// out of class definition
Under /Ze, the out-of-class definition is optional for static, const integral, and const enum data members. Only integrals and enums that are static and const can have initializers inside a class; the initializing expression must be a const expression.

[DarkDefender]

It might be that VS and gcc is more loose in their c++ standard implementation. But the problem we have is the following:
1. The code compiled fine with clang before the commit
2. There are other variables that are defined just like O_WESTWALL & co is. However those doesn't break clang it seems
3. If we just accept a quick fix for this without knowing why and what went wrong then there will probably be breakage further down the line.

I installed clang 3.2 my self to see if I could compile it. It doesn't work here either. But I get a better error message:

Code: [Select]

openxcom-TileEngine.o: In function `pair':
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.2/include/g++-v4/bits/stl_pair.h:105: undefined reference to `OpenXcom::MapData::O_WESTWALL'
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.2/include/g++-v4/bits/stl_pair.h:105: undefined reference to `OpenXcom::MapData::O_NORTHWALL'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
If we take a look at what changed in that commit https://github.com/SupSuper/OpenXcom/commit/6024bedf3cca3855a5b00b315b5a24d8530964e9 we can see that it is now using "std::pair" to store the O_* stuff. And it wasn't using a pair datatype before. So that is probably what broke it and not the definition of the O_* ints.
So now the question is why clang doesn't like pairs in this situation

[DarkDefender]

I think I solved the problem. I replaced every new pair declaration in the push back function with make_pair because the pair type is already defined in the vector and voila, it now compiles for me with clang!
Can you try it out and see if it solves it for you also?

Not only does this cut down on some unnecessary code it also solves the compilation problem. However I don't know exactly why clang had a problem with the old code. I will rule it out as a bug in clang for the time being. Or perhaps this is deliberate to force the code to be cleaner I guess? However the error message suggest otherwise...  

[grrussel]

So, pair vs make_pair

std::make_pair deduces its argument types, and from its function arguments, creates a std::pair
https://www.cplusplus.com/reference/utility/make_pair/

std::pair (src at https://gcc.gnu.org/onlinedocs/libstdc++/libstdc++-html-USERS-4.3/a02030.html) takes its constructor arguments by reference. Thus, it is needed to take the address of the thing passed in (e.g.  MapData::O_WESTWALL). This has no address - the variable is declared and initialised, inside the class, but not defined outside the class. Therefore, no storage was allocated, and it has no address.

For std::make_pair, the arguments to the internal call to std::pair(a,b) use the address of the arguments in the call to std::make_pair, and so that works. The compiler can see
declaration of  MapData::O_WESTWALL specified a constant int value, so that can be used in the call to std::make_pair

Why did it only break on clang? No idea.  The problem is that the static consts in the class are not addressable, so the problem may recur if any other code attempts to take their address (e.g. ever using those values to a function taking an int& operand). The real solution imo is to add the out of class definitions.

The code as is with the make_pair change is back up and building in the OSX buildbot as before ;-)