Current BZR version leads to compiler errors on Windows

[Andreas Breitschopp]

As discussed here
https://code.launchpad.net/~ab-tools/widelands/msvs2010-compilewarnings/+merge/109787
current BZR version leads to compiler errors on Windows using Visual Studio 2010 which worked fine with older BZR versions.

Most compiler errors (but not all) are referring to a "Syntax error: '('" in these two lines (see also attached compiler log):
 - "panel.h": line 250: "void do_draw(RenderTarget &) __attribute__((hot));"
 - "button.h": line 72: "void draw(RenderTarget &)__attribute__((hot));"

Any ideas on that?

[Hans Joachim Desserud (hjd)]

As indicated by the merge request, this did work at some point in June last year. Using binary search to go through the revisions added since then it should be possible to pin down the commit which broke compilation in VS.

[Andreas Breitschopp (ab-tools)]

Indeed the problem cannot be too long ago, because I tried to compile Widelands the last time about one months ago.

As you can read in this forum thread I got only one compiler error at this point which I was able to fix myself:
http://wl.widelands.org/forum/topic/1064/?page=3

Now there are a lot of compiler errors more in the current BZR version.

[SirVer (sirver)]

I Bet the__attribute is the problem. Try removing it and see what happens.

[Kiscsirke (csirkeee)]

What you actually mean is that it can't be built with Visual Studio. It builds happily with MinGW on Windows :)

Anyway, yes, probably the attribute is the problem. What is surprising is that it worked before, the attribute was introduced in rev 4206 in the year 2009. I'm guessing maybe the recently introduced stricter warnings could make this a problem, if it also included warnings for Visual Studio? __attribute__ is a GCC compiler specific directive, VS directives are in the form __declspec.

Anyway, if it is the attribute that is the problem, probably we should introduce a conditional macro that is different for different compilers. Something like
#ifdef __GNUC__
 #define GCC_HOT __attribute__((hot))
#else
 #define GCC_HOT
#endif
(A quick search didn't turn up anything analogous in VS for me, so I just define it to nothing there.) This should be placed in a header named like "platform.h" maybe, that is included in these files.

And then with these functions you can replace "__attribute__((hot))" with "GCC_HOT" like
void do_draw(RenderTarget &) GCC_HOT;
(I tested this on MinGW-GCC, it worked there at least.)

[SirVer (sirver)]

Just delete those attributes. For one, these functions are not really hot (they do not show up in the profile runs I did recently), secondly, the compiler should figure out for itself what to optimize and third they are not portable. Just delete them and do not bother anymore.

[Andreas Breitschopp (ab-tools)]

Yes, removing these attributes indeed fixes the compiler errors for Visual Studio 2010 for both lines.

Additionally the other compiler error I mentioned in the forum thread already needs to be fixed as well.

I've attached a patch now that fixes all of them. After applying that compilation works correctly again for me with Visual Studio 2010.

[Nicolai Hähnle (nha)]

I would suggest to replace the assert by throwing an exception. width() is INFINITE_WIDTH, so it's not a good idea to try to create a surface of that size...

[Andreas Breitschopp (ab-tools)]

Yes, I agree, it would be a very large surface. ;-)

I changed the patch to throw an exception.

[SirVer (sirver)]

I merged your patch in r6532. I only changed a tiny bit: the call to format() was not really necessary as you were passing a plain string to it anyways - so I removed it.

[Andreas Breitschopp (ab-tools)]

Great, thanks for committing the fix!

[SirVer (sirver)]

Released in build-18 rc1.