-
-
Notifications
You must be signed in to change notification settings - Fork 384
Msvc support 3 #356
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
base: main
Are you sure you want to change the base?
Msvc support 3 #356
Conversation
This is not a search and replace of __MING32__ with _WIN32. There are places where __MINGW32__ is still used and all changes are tested with MSVC 2019 compiler. Although, this commit alone will not work on MSVC because there are plenty of other things to be done which are seperated into multiple PR. PR OSGeo#289 is the one that "works" on MSVC and unix as well. But that contains too many changes which shouldn't in a single PR. Even though this PR alone won't compile GRASS GIS on MSVC, it sure will not break existing compilers which I think is very important. Complete support for MSVC will be ready after 2/3 PRs OSGeo#289
static int initialized is not working for MSVC and must export this with __declspec so that it appears in a DLL correctly. This change has been tested on Ubuntu and MSVC 2019 and is working as expected.
Below is a code from filldepr.cpp which throws error on msvc compiler
function has G_fatal_error() so it is 'normal' to have no return type.
But MSVC complains that function's return type is elevation_type*
and does not return anything.
elevation_type*
ext_fill_depression(AMI_STREAM<boundaryType> *boundaryStr,
cclabel_type maxWatersheds) {
G_fatal_error(_("Fill_depressions do not fit in memory. Not implemented yet"));
}
The MSVC compiler only supports C90, it does not support C99 and variable length arrays are a feature of C99.
|
I don't know what are the relevant changes here (all these old msvc PRs were done from different places on a branch with previous changes accumulated - that's partially why we were never able to review them and merge them). I suggest closing this when CMake build works on Windows. |
echoix
left a comment
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.
This PR starts after f76f205, and has 10 commits (https://github.com/OSGeo/grass/pull/356/files/f76f205f4ab8d1fcdf260fea0b29b1889e4deeee..9714a9071aed297bed9e6f55bd41fdf81528d924).
I reviewed what was missing, already included, or conflicting. I think I found a manual conflict resolution error.
| #if defined(_MSC_VER) | ||
| #ifndef __attribute__ | ||
| #define __attribute__(x) | ||
| #endif | ||
| #endif | ||
|
|
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.
In #3621, this was added, that applies if _MSC_VER is defined, but does #undefs before.
#if !defined __GNUC__ || __GNUC__ < 2
#undef __attribute__
#define __attribute__(x)
#endif| #define EPSG_FILE "PROJ_EPSG" | ||
|
|
||
| #ifdef __MINGW32__ | ||
| #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.
Not applied yet
| #ifndef POINT2D_C | ||
| #define POINT2D_C 1 | ||
| #endif |
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.
Not included in main yet
| #ifdef _MSC_VER | ||
| #undef min | ||
| #undef max | ||
| #endif |
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.
Recently, r.sim got major changes in #5373 and this file and global variables have been removed.
To be merged after #355
msvc support 3rd and maybe last installation!