Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix build issue of external code including gdal_priv.h with MSVC (master only) #10762

Merged
merged 1 commit into from
Sep 11, 2024
Merged

Fix build issue of external code including gdal_priv.h with MSVC (master only) #10762

merged 1 commit into from
Sep 11, 2024

Conversation

rouault
Copy link
Member

@rouault rouault commented Sep 10, 2024

MSVC complains about "use of undefined type 'GDALDoublePointsCache'" since presumably it tries to locate the destructor of GDALDoublePointsCache, to which it doesn't have access

@rouault
Fix build issue of external code including gdal_priv.h with MSVC (mas…
515fbf0 
…ter only)

MSVC complains about "use of undefined type 'GDALDoublePointsCache'"
since presumably it tries to locate the destructor of
GDALDoublePointsCache, to which it doesn't have access
@elpaso
Copy link
Collaborator

elpaso commented Sep 11, 2024

Is there another option to fix this? To go back for managed to manual memory handling seems a step back to me.

@rouault
Copy link
Member Author

rouault commented Sep 11, 2024

Is there another option to fix this?

The easiest alternative would be to move the declaration GDALDoublePointsCache from internal / non-installed alg/gdal_interpolateatpoint.h to gdal_priv.h

A more advanced solution (I'm not totally sure though) would probably to not CPL_DLL export the GDALRasterBand class in the MSVC case, but tag each exported method as CPL_DLL, like we do in PROJ in https://github.com/OSGeo/PROJ/blob/0a407325fbb5bf42407a7dc5d4f948be9707e302/include/proj/crs.hpp#L90 where PROJ_GCC_DLL expands to __attribute__((visibility("default"))) for gcc/clang and nothing for MSVC. But it is also easier to mess it up and forget to export something, which wouldn't be caught if we for example have a plugin needed a particular method, and it happens the Windows builds in our CI don't build that driver.

For now, I'd prefer to stick with the solution of this PR, even if I fully agree this is not ideal.

@rouault rouault mentioned this pull request Sep 11, 2024
Open
@rouault
Copy link
Member Author

rouault commented Sep 11, 2024

I'm merging at it, to unbreak breakage that is hurting users following master. I've filed #10768 so we can possibly revisit that in the future

@rouault rouault merged commit 1e48b77 into OSGeo:master Sep 11, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rouault @elpaso