-
Notifications
You must be signed in to change notification settings - Fork 78
[CORE][CMAKE] Fix Linux compilation of WWLib #698
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ee80f88
to
0b7ef34
Compare
unsigned buffer_base=reinterpret_cast<unsigned>(m_Buffer-sizeof (StringClass::_HEADER)); | ||
unsigned temp_base=reinterpret_cast<unsigned>(m_TempStrings+MAX_TEMP_BYTES*MAX_TEMP_STRING); | ||
unsigned long buffer_base=reinterpret_cast<unsigned long>(m_Buffer-sizeof (StringClass::_HEADER)); | ||
unsigned long temp_base=reinterpret_cast<unsigned long>(m_TempStrings+MAX_TEMP_BYTES*MAX_TEMP_STRING); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this meant to take the size of pointer? long is 4 bytes on Windows x64. See:
https://en.cppreference.com/w/cpp/language/types
https://en.cppreference.com/w/cpp/language/types#Data_models
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, we have no integer type for pointer size yet. Will add one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All custom integer types are defined in BaseTypeCore.h
, however wwstring.cpp
and all of WWLib does not make use of BaseType.h
What shall we do here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps replicate the c++11 stdint types in a "Dependencies/Utility/stdint_adapter.h"
Then include that adapter in BaseTypeCore.h and always.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think i can do that in a separate PR? I didn't break anything here and this works fine for Linux. It didn't work for windows 64-bit before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes can be separate.
@@ -38,11 +38,25 @@ | |||
#ifndef _SYSTIMER_H | |||
|
|||
#include "always.h" | |||
#ifdef _WIN32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can add empty line before this.
#else | ||
#define DEFINE_AUTO_POOL(T,BLOCKSIZE) \ | ||
template<>\ | ||
ObjectPoolClass<T,BLOCKSIZE> AutoPoolClass<T,BLOCKSIZE>::Allocator = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this separation really necessary? Cannot just add template<>
for all compilers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No template<>
doesn't work when not using = {}
as the initializer here. VC6 doesn't support that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the compile error that this causes? Something about dependent name? It is odd that this requires a initializer because ObjectPoolClass
does have a default constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If i remember it was: missing implementation ....
.
Co-authored-by: Patrick Zacharias <[email protected]>
Partially implements #522
This PR does the following things to fix Linux compilation:
_UNIX
macro for non windows platformsosdep.h
. We replaced this with out compat library