News:

Simutrans.com Portal
Our Simutrans site. You can find everything about Simutrans from here.

'timespec': 'struct' error compiling with msvs 2015

Started by Ves, October 02, 2016, 09:09:39 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

Ves

This is a continuation from this thread:
http://forum.simutrans.com/index.php?topic=14705.msg154781#msg154781

Severity Code Description Project File Line Suppression State
Error C2011 'timespec': 'struct' type redefinition Simutrans-Experimental C:\Users\Victor\Documents\GitHub\OpenTTD\shared\include\pthread.h 320


I am trying to compile Simutrans Experimental on msvs2015 and I get the above error.
It was asked if the simthread.h file was included in the project, and I confirm it does here: https://github.com/jamespetts/simutrans-experimental/blob/devel-new-2/utils/simthread.h and it also shows up inside msvs2015.

I have also tried to follow the steps in this post about recompiling my own version of the libbz2.lib: http://forum.simutrans.com/index.php?topic=14705.msg154718#msg154718
Again, without any change to the above error.

How can I resolve this error?

DrSuperGood

QuoteSimthread.h is located here: https://github.com/jamespetts/simutrans-experimental/blob/devel-new-2/utils/simthread.h
Strange, I googled simutrans experimental on github and it was not there... Anyway it does have the fix so does work.

That is if all multi-threaded use correctly includes simthread.h rather than incorrectly including pthread.h directly.

QuoteHow can I resolve this error?
It should be resolved. Make sure to clean build.

Junna

Quote from: DrSuperGood on October 02, 2016, 09:26:10 PM
That is if all multi-threaded use correctly includes simthread.h rather than incorrectly including pthread.h directly.
It should be resolved. Make sure to clean build.

That does not work, so I take it that the previous suggestion, that it does not include simthread.h, might be the issue.

"Severity   Code   Description   Project   File   Line   Suppression State
Error   C2011   'timespec': 'struct' type redefinition (compiling source file simdepot.cc)   Simutrans-Experimental   C:\Ny mapp (2)\Pre-built.2\include\pthread.h   320   "

refers to simdepot.cc, which presumably incorrectly refers pthread

Ves

I also just made a clean build (downloaded the devel-new-2 branch of Experimental as a zip), and the result is still the same error.

When trying to recompile after a fail, it appears to want to compile simdepot.cc. This applies when trying to compile both the clean build and the one directly from Github. Could it be that the error lies somewhere there?

DrSuperGood

Quote
#ifdef MULTI_THREAD

#include <pthread.h>

static pthread_mutex_t sync_mutex = PTHREAD_MUTEX_INITIALIZER;

static pthread_mutex_t add_to_world_list_mutex = PTHREAD_MUTEX_INITIALIZER;

#endif
This is an experimental fault? It should not be including pthread.h directly?

jamespetts

#5
Ahh, after some searching, I think that I have an idea what the problem is: apologies for not realising this sooner. For some reason, when I first tried to compile this, Dr. Supergood's suggestion/patch did not work: I still got the timespec error. I ended up modifying pthread.h, which I have just realised is not actually staged, so this would not have been downloaded.  I had to add the following to somewhere (anywhere) near the top of the file):


#ifdef _MSC_VER
#define HAVE_STRUCT_TIMESPEC
#endif


Given that this file is not staged, this is a very unsatisfactory way of doing things, but I am not sure why it is necessary. I wonder whether Dr. Supergood has any ideas?

Edit: I think that I have fixed this based on Dr. Supergood's advice above, by referring to simthread.h rather than pthread.h. I should be grateful if people could let me know whether this works.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ves


DrSuperGood

Quote
Given that this file is not staged, this is a very unsatisfactory way of doing things, but I am not sure why it is necessary. I wonder whether Dr. Supergood has any ideas?
It was explained when I patched in the solution to simthread.h. Old versions of MSVC (when pthread was last updated) did not have the standard timespec structure, so pthread had to explicitly declare one for its API to work. However MSVC2015 made huge advancements on C++ conformance and so now does have the standard timespec structure which is used by the pthread API (identical declarations). However pthread.h still tries to explicitly declare a timespec structure on MSVC even though it already exists resulting in the error. The solution is to instruct the compiler before including pthread that it has a timespec structure with the appropriate macros.

There are two ways to do this. The one I used with hacky compiler macros. The other to add the macro to the build properties on MSVC2015 projects (possibly a better solution). As Simutrans standard has no MSVC2015 project and lots of people rely on older MSVC for building I went with the hacky approach. If experimental is choosen to only support MSVC2015 and later then feel free to add the macro to the project build properties.

jamespetts

Splendid - thank you for the clarification, and for finding the underlying problem here.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Junna

Quote from: Ves on October 03, 2016, 10:17:02 PM
It works! Thank you James and DrSupergood!

You going to put it on the server? I still can't compile it 'cause of bz2lib.

Ves

Yes, i did not have time yesterday, but will today.

edit:
QuoteYou going to put it on the server? I still can't compile it 'cause of bz2lib.
Done!

However, the makeobj are now complaining about this:

Severity Code Description Project File Line Suppression State
Error C2956 sized deallocation function 'operator delete(void*, size_t)' would be chosen as placement deallocation function. Makeobj-experimental C:\Users\Victor\Documents\GitHub\simutrans-experimental\besch\bild_besch.cc 334
Error C2956 sized deallocation function 'operator delete(void*, size_t)' would be chosen as placement deallocation function. Makeobj-experimental C:\Users\Victor\Documents\GitHub\simutrans-experimental\besch\bild_besch.cc 383
Error C2956 sized deallocation function 'operator delete(void*, size_t)' would be chosen as placement deallocation function. Makeobj-experimental C:\Users\Victor\Documents\GitHub\simutrans-experimental\besch\bild_besch.cc 411

DrSuperGood

Quote
However, the makeobj are now complaining about this:
Use the solution I posted for standard for MSVC2015. Until the source code is updated to conform with modern versions of the C++ standard one has to use a hacky compiler flag to disable the sized allocator/deallocator feature. The problem is that the placement allocation function used by simutrans to append an array on the end of a class conflicts with the sized allocator and deallocator declerations.

Specifically the following MSVC++ compiler flag.

/Zc:sizedDealloc-

Ters

Quote from: DrSuperGood on October 04, 2016, 03:05:05 PM
Use the solution I posted for standard for MSVC2015. Until the source code is updated to conform with modern versions of the C++ standard one has to use a hacky compiler flag to disable the sized allocator/deallocator feature. The problem is that the placement allocation function used by simutrans to append an array on the end of a class conflicts with the sized allocator and deallocator declerations.

Specifically the following MSVC++ compiler flag.

/Zc:sizedDealloc-


Or just commit a matching deallocator member function. That way, everybody gets their problems fixed. Maybe that counts as "conform with modern versions of the C++ standard", but it doesn't take much effort.

Ves

QuoteUse the solution I posted for standard for MSVC2015. Until the source code is updated to conform with modern versions of the C++ standard one has to use a hacky compiler flag to disable the sized allocator/deallocator feature. The problem is that the placement allocation function used by simutrans to append an array on the end of a class conflicts with the sized allocator and deallocator declerations.

Specifically the following MSVC++ compiler flag.
Thank you, that worked!

jamespetts

I have now committed a change representing this fix to Github.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

DrSuperGood

#15
Quote
Or just commit a matching deallocator member function. That way, everybody gets their problems fixed. Maybe that counts as "conform with modern versions of the C++ standard", but it doesn't take much effort.
Does not work like that I am afraid. Sized deallocators are not placement allocators/deallocators and they are reserved.

To make it clear (was not thinking earlier, sorry), there is no physical new sized allocator since types always have a know size so the standard allocator works (which is sized internally). There is now implicitly a sized deallocator since the compiler can then pass the type size for final types which can make deallocation more efficient (no dynamic resolution of size). The problem is that in order for C++ to support the sized deallocator which is implicitly used, it means that the placement allocator for type "size_t" cannot be used as the placement deallocator for "size_t" (only called during new failure as clean up) has the same signature as the sized deallocator. Simutrans uses a placement allocator of "size_t" (or near enough) to append a variable length array at the end of a class. As such the error is produced because such placement allocators are no longer legal C++. This applies to both MSVC2015 and new versions of GCC which apparently also generate the same error.

The solution is to use a complex type placement allocator such as...

struct class_append_array_t {
    size_t length;
};

Since this is not size_t it has an optional placement deallocator taking class_append_array_t which does not conflict with the sized deallocator.

However placement allocators of types other than size_t were only supported by newer revisions of C++ than then minimum required support of Simutrans standard allows. Hence there currently is no solution other than hacky compiler flags or reverting to older compilers.

Experiential is not standard though. If James wishes to raise the minimum supported C++ standard he can which would mean a proper language fix could be applied. Personally I have nothing against doing so (MSVC2015 and newer are free for use by individual persons and small companies and GCC is GNU licenced so free) and think that projects should only target the latest versions of GCC and MSVC to keep up to date. Apparently it causes problems with MinGW which uses an old version of GCC lacking modern C++ support which is why standard has an old C++ requirement.

Ters

Quote from: DrSuperGood on October 04, 2016, 10:47:21 PM
Does not work like that I am afraid. Sized deallocators are not placement allocators/deallocators and they are reserved.

I tested it and it worked. You did not protest back then. The (draft for the) standard is explicit that sized deallocator functions are only used if they are defined. The global ones are apparently always defined, but it only uses global functions if there are no class-specific functions. The standard (draft) was unclear on whether it would use a global deallocator function for a local allocator function, which compilers apparently will, but it will not use a global sized deallocator function if there exists any local deallocator function.

It might not be an ideal solution, I never presented it as such, but at least we don't need to deal with people having problems compiling all the time until someone manages to pull together such an ideal solution. The problem affects the newest versions of GCC as well, so it can only be expected to get worse. The inventor of radar, an early version of which was put in service just in time to probably save Britain from defeat in the war has stated "Give them the third best to go on with; the second best comes too late, the best never comes."