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

Python: Fix wrapper build on MSVC not having __cplusplus for BitHacks #613

Merged
merged 1 commit into from Sep 7, 2023

Conversation

EchterAgo
Copy link
Contributor

The BitHacks.h header needs the __cplusplus define to determine whether to include <bit>. The MSVC compiler only defines this when it is passed the /Zc:__cplusplus flag. The main library was already setting this flag, but it not set when building the Python wrapper, causing compilation to fail because countr_zero and popcount are being used without <bit> being included.

Fixes #612

@axxel
Copy link
Collaborator

axxel commented Sep 7, 2023

Thanks for looking into this and providing a fix. Before merging it though, I'd like to ask if you could test an alternative approach that I would consider a more general solution:

diff --git a/core/CMakeLists.txt b/core/CMakeLists.txt
index a06cd6e3..b49d4150 100644
--- a/core/CMakeLists.txt
+++ b/core/CMakeLists.txt
@@ -22,6 +22,11 @@ if (WINRT)
         -DWINRT
     )
 endif()
+if (MSVC)
+    set (ZXING_CORE_DEFINES ${ZXING_CORE_DEFINES}
+        /Zc:__cplusplus
+    )
+else()
 
 set (ZXING_CORE_LOCAL_DEFINES
     $<$<BOOL:${BUILD_READERS}>:-DZXING_BUILD_READERS>
@@ -34,7 +39,6 @@ if (MSVC)
         -D_CRT_SECURE_NO_WARNINGS
         -D_CRT_NONSTDC_NO_WARNINGS
         -DNOMINMAX
-        /Zc:__cplusplus
     )
 else()
     set (ZXING_CORE_LOCAL_DEFINES ${ZXING_CORE_LOCAL_DEFINES}

If that also fixes your issue, I'd commit that instead as it would also make sure that other client code that ends up including BitHacks.h has this defined as needed.

The `BitHacks.h` header needs the `__cplusplus` define to determine
whether to include `<bit>`. The MSVC compiler only defines this when it
is passed the `/Zc:__cplusplus` flag. The main library was already
setting this flag, but it not set when building the Python wrapper,
causing compilation to fail because `countr_zero` and `popcount` are
being used without `<bit>` being included.

Fixes zxing-cpp#612
@EchterAgo
Copy link
Contributor Author

Updated, it builds on MSVC

@axxel axxel merged commit f92e87c into zxing-cpp:master Sep 7, 2023
16 checks passed
@axxel
Copy link
Collaborator

axxel commented Sep 7, 2023

Thanks.

@EchterAgo EchterAgo deleted the fix_msvc_pywrapper branch September 7, 2023 16:56
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.

MSVC build is broken, no countl_zero and popcount
2 participants