Angband.oook.cz
Angband.oook.cz
AboutVariantsLadderForumCompetitionComicScreenshotsFunniesLinks

Go Back   Angband Forums > Angband > Development

Reply
 
Thread Tools Display Modes
Old December 18, 2019, 17:10   #1
eastwind
Apprentice
 
Join Date: Dec 2019
Location: Mexico, undisclosed location
Posts: 79
eastwind is on a distinguished road
Compilation errors building 4.2.0 w/ VS 2019

I got 4.2.0 to build under VS 2019, but not without a few code fixes. FWIW I was building on win 10 with the community (free) edition of VS, no cygwin installed with my own solution & vcxproj.

Per the FAQ, I'm reporting them in a thread (I think the idea is they aren't 'bugs' until someone else hits them). Since they're all compilation errors I'm going to bundle them in one thread rather than spam 4 threads.

****** Bug #1).

C1189: #error: Macro definition of snprintf conflicts with Standard Library function declaration

Lots of .c files have this error, any that include h-basic.h. Line 29 defines snprintf because old versions of MSC needed a definition. But at some point (*) they fixed the compiler, and now defining the macro causes both a C4005 macro redefinition warning and a C1189 error. The problem lines in h-basic.c are these:

/**
* Native MSVC compiler doesn't understand inline or snprintf
*/
#ifdef _MSC_VER
# define inline __inline
# define snprintf _snprintf
#endif

My fix was to restrict those lines to early versions of Visual C. I *think* the change happened with VS 2015, so I changed the #ifdef to:

#if (defined(_MSC_VER) && _MSC_VER < 1900L)

1900L is supposed to be VS 2015, but I don't have VS 2015 or an earlier version to test this fix. This change should only affect anybody compiling with VS 2015 or later.

An alternate fix would be to officially decide that only recent versions of MS C are supported, and get rid of the whole snippet. I believe the issue is that for a long time MS C did not support C99, but finally added some support in VS 2015, and the Angband code assumes C99 and uses snprintf and the inline keyword. The snippet was a fix for MS C not supporting that portion of C99, but now it does, so the snippet will cause an error on C99-supporting versions of MS C.

**** Bug #2):

load.c(852,36): error C2057: expected constant expression
load.c(852,36): error C2466: cannot allocate an array of constant size 0
load.c(852,37): error C2133: 'itypes': unknown size

Line 852 is the declaration:

bitflag flags, itypes[itype_size];

itype_size is a local variable. You can't allocate dynamic-sized arrays on the stack in C like you can in C++. My fix was to allocate the max size that itype_size is allowed to have all the time, which is the #define constant
ITYPE_SIZE - it's not very large, and this array only sits on the stack for a short time anyway.

bitflag flags, itypes[ITYPE_SIZE];

An alternative fix would be to use the C++ compiler - but that might have a lot of consequences.

***** Bug #3):

scrnshot.c(105): error C4703: potentially uninitialized local pointer variable 'info_ptr' used

I mentioned this in another thread. I believe it's actually a compiler bug, not an Angband bug, but I know from experience that reporting it to Microsoft will result in them replying "well don't write screwy code", because the C-comipiler front end is a different code base for them than their C++ front end, and the C front end is very old code (perhaps dating from the 80's) and they touch it as little as possible.

My workaround is to initialize info_ptr to null in its declaration on line 40:

png_infop info_ptr = NULL;

Of course rewriting the routine wouldn't be a bad idea, but adding '= NULL' is the minimum to get the code to compile.

***** Bug #4):

z-file.c(389,1): error C4996: 'open': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _open. See online help for details.
corecrt_io.h(516): message : see declaration of 'open'

I think this is another compiler change that came in with VS 2015 and C99 support. In this case the fix is to add some lines for versions 2015 and after. I added these lines to z-file.c after line 55:

/* Suppress MSC C4996 error */
#if defined(_MSC_VER) && (_MSC_VER >= 1900L)
#define open _open
#define fdopen _fdopen
#define mkdir _mkdir
#endif

(I only pasted one C4996 error above, but z-file.c gets several, for the three functions redefined, so all 3 defines are needed).

*****

That's it - I get a build with those fixes, plus making sure not to define _UNICODE and adding a couple other defines on the command line. I've tested my build for a couple hours and it is stable and shows no issues, but for completeness somebody with an early MS C should compile these fixes, and somebody with cygwin should make sure they don't have any effect for cygwin.
eastwind is offline   Reply With Quote
Old December 27, 2019, 02:16   #2
eastwind
Apprentice
 
Join Date: Dec 2019
Location: Mexico, undisclosed location
Posts: 79
eastwind is on a distinguished road
Is there any interest in having these fixes pushed? I can try to figure out how to use Git if there is interest, but if no one else is trying to us VS I won't bother.

Before I do submit them, I would need some feedback on a couple of the #ifdefs - whether I've done them right so as to not mess up other builds.
eastwind is offline   Reply With Quote
Old December 27, 2019, 03:34   #3
Nick
Vanilla maintainer
 
Nick's Avatar
 
Join Date: Apr 2007
Location: Canberra, Australia
Age: 55
Posts: 8,396
Donated: $60
Nick will become famous soon enough
Quote:
Originally Posted by eastwind View Post
Is there any interest in having these fixes pushed? I can try to figure out how to use Git if there is interest, but if no one else is trying to us VS I won't bother.

Before I do submit them, I would need some feedback on a couple of the #ifdefs - whether I've done them right so as to not mess up other builds.
Yes, there definitely is, and just listing your changes would be fine (although obviously if you want to learn git go right ahead).
__________________
One for the Dark Lord on his dark throne
In the Land of Mordor where the Shadows lie.
Nick is offline   Reply With Quote
Old January 25, 2020, 05:59   #4
eastwind
Apprentice
 
Join Date: Dec 2019
Location: Mexico, undisclosed location
Posts: 79
eastwind is on a distinguished road
Bug number 3 is already fixed in master by the fix for #4236, I couldn't find an issue for the other 3 so I opened #4266.

Nick, sorry if #4266 is a dup. I am ready to put up a pull request with these fixes once I have the right issue to reference.
eastwind is offline   Reply With Quote
Old January 25, 2020, 11:08   #5
Nick
Vanilla maintainer
 
Nick's Avatar
 
Join Date: Apr 2007
Location: Canberra, Australia
Age: 55
Posts: 8,396
Donated: $60
Nick will become famous soon enough
No it's not - I missed this thread altogether when I did #4236.
__________________
One for the Dark Lord on his dark throne
In the Land of Mordor where the Shadows lie.
Nick is offline   Reply With Quote
Old January 25, 2020, 14:48   #6
eastwind
Apprentice
 
Join Date: Dec 2019
Location: Mexico, undisclosed location
Posts: 79
eastwind is on a distinguished road
Ok, I think I stumbled my way through all the Git malarky and there's a pull request waiting your approval. It looks kind of like creating the pull request created a new issue #4267 referencing #4266 that I thought I was fixing.

Anyway, see the comments for a description of a simpler version of the fix that deprecates VS compilers prior to 2015. The fixes I pushed don't make that decision.
eastwind is offline   Reply With Quote
Old January 25, 2020, 20:39   #7
Nick
Vanilla maintainer
 
Nick's Avatar
 
Join Date: Apr 2007
Location: Canberra, Australia
Age: 55
Posts: 8,396
Donated: $60
Nick will become famous soon enough
Malarky indeed. Apparently issues and pull requests use the same numbering system; and I knew if you types "Fix #nnn" or "Fixes #nnn" in a commit message it would close issue #nnn, but apparently this also works in the title of a pull request.
__________________
One for the Dark Lord on his dark throne
In the Land of Mordor where the Shadows lie.
Nick is offline   Reply With Quote
Old January 26, 2020, 00:42   #8
takkaria
Veteran
 
takkaria's Avatar
 
Join Date: Apr 2007
Posts: 1,947
Donated: $40
takkaria is on a distinguished road
Quote:
Originally Posted by eastwind View Post
You can't allocate dynamic-sized arrays on the stack in C like you can in C++.
FWIW that isn't true. C99 allows variable-sized arrays allocated on the stack. It's incredibly frustrating that Microsoft can't implement full support for a 20 year old standard!
__________________
takkaria whispers something about options. -more-
takkaria is offline   Reply With Quote
Old January 26, 2020, 01:13   #9
Pete Mack
Prophet
 
Join Date: Apr 2007
Location: Seattle, WA
Posts: 5,752
Donated: $40
Pete Mack is on a distinguished road
And yeah, MS doesn't want to bother with C, but it isn't that big a deal to make minor changes to a compiler front-end. (The back end is shared across languages.)
Pete Mack is offline   Reply With Quote
Old January 26, 2020, 07:24   #10
eastwind
Apprentice
 
Join Date: Dec 2019
Location: Mexico, undisclosed location
Posts: 79
eastwind is on a distinguished road
The changes in the OP are now all in master.

I'll punt on the language feature support discussion.
eastwind is offline   Reply With Quote
Reply


Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
watchmen (2019) DrWho42 Idle chatter 0 October 23, 2019 07:29
bootrogue (2019) DrWho42 Idle chatter 0 September 30, 2019 15:52
Memory errors associated with quak traps Pete Mack Development 0 October 1, 2016 17:34
Graphic errors in gd08de4a build CyclopsSlayer v4 6 May 14, 2012 02:52
r1353 compile errors PaulBlay Vanilla 10 April 13, 2009 08:19


All times are GMT +1. The time now is 09:27.


Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2020, vBulletin Solutions Inc.