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.

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

Extend msvc (DLL) configuration to clang-cl and mingw #739

Merged
merged 3 commits into from
Nov 23, 2022

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Nov 22, 2022

This PR is based on patching in vcpkg. Originally contributed to enable builds with clang-cl, I modified it to cover mingw builds. This change reduced the C++ import lib on mingw by one third.

@dg0yt
Copy link
Contributor Author

dg0yt commented Nov 22, 2022

2022-11-22T06:57:22.4721616Z In file included from D:/a/geos/geos/include/geos/algorithm/PointLocation.h:24,
2022-11-22T06:57:22.5346164Z                  from D:/a/geos/geos/src/algorithm/ConvexHull.cpp:22:
2022-11-22T06:57:22.6752542Z D:/a/geos/geos/include/geos/geom/Location.h:32:32: error: 'dllexport' attribute ignored [-Werror=attributes]
2022-11-22T06:57:22.7377914Z    32 | enum class GEOS_DLL Location : char {
2022-11-22T06:57:22.8897398Z       |                                ^~~~
2022-11-22T06:57:23.0675800Z cc1plus.exe: all warnings being treated as errors
2022-11-22T06:57:23.1946999Z make[2]: *** [CMakeFiles/geos.dir/build.make:146: CMakeFiles/geos.dir/src/algorithm/ConvexHull.cpp.obj] Error 1

Hm, I saw this warning while testing, but I didn't build with -Werror=attributes. Not sure how to properly DLL-export enum class with mingw.

@@ -187,7 +187,7 @@ target_compile_features(geos_cxx_flags INTERFACE cxx_std_11)
target_compile_options(geos_cxx_flags INTERFACE
"$<$<OR:$<CXX_COMPILER_ID:Clang>,$<CXX_COMPILER_ID:AppleClang>>:-ffp-contract=off>"
"$<$<CXX_COMPILER_ID:GNU>:-ffp-contract=off>"
"$<$<CXX_COMPILER_ID:MSVC>:/fp:precise>"
"$<$<BOOL:${MSVC}>:/fp:precise>"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the bool thing? Is the old way broken? (I find the old one more straightforward in my head.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, you're picking off mingw and msvc with one shot, got it. Add a comment to that effect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to mingw. AFAIU $<CXX_COMPILER_ID:MSVC> doesn't cover clang-cl, but ${MSVC} does.

@pramsey pramsey merged commit c1df685 into libgeos:main Nov 23, 2022
@dg0yt dg0yt deleted the dll-builds branch November 24, 2022 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants