-
-
Notifications
You must be signed in to change notification settings - Fork 383
Support msvc 2 #351
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?
Support msvc 2 #351
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"));
}
|
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. |
|
It's the last 6 commits that follow #350, on a computer, on files changed, you can filter by commit and use shift+click to select a range of commits to show the changed files of |
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.
I reviewed the changes that are after the branch mentioned (last 6 commits). There's only one include in raster/r.viewshed/statusstructure.cpp that isn't in main already
| #ifdef _MSC_VER | ||
| #pragma warning(default:4716) | ||
| #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.
Included in #3621, but with #pragma warning(default : 4716) instead.
| #include <stdlib.h> | ||
| #include <assert.h> | ||
|
|
||
| #include <algorithm> |
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 branch as of 2025-04-26
to be merged after #350