Fix Windows build compatibility (MSVC cl.exe and clang-cl)#53
Open
lorisercole wants to merge 5 commits intowavefunction91:masterfrom
Open
Fix Windows build compatibility (MSVC cl.exe and clang-cl)#53lorisercole wants to merge 5 commits intowavefunction91:masterfrom
lorisercole wants to merge 5 commits intowavefunction91:masterfrom
Conversation
- Remove dead `exx_coeff` static constexpr in `deorbitalized.hpp` that referenced non-existent members in kernel_traits (MSVC eagerly instantiates it; GCC/Clang deferred since it was never ODR-used) - Replace C++ alternative operator tokens (`not`, `and`, `or`) with standard `!`, `&&`, `||` across headers and source files (MSVC cl.exe does not recognize them without /permissive-) - Add `_USE_MATH_DEFINES` compile definition for MSVC (M_PI) - Suppress MSVC warning C4800 (implicit double-to-bool conversion in auto-generated kernel code) - Replace `sed` PATCH_COMMAND for libxc with portable CMake -P script so FetchContent works on Windows without Unix tools
Co-authored-by: Copilot <copilot@github.com>
warning C4996: 'strdup': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _strdup.
This was referenced May 7, 2026
There was a problem hiding this comment.
Pull request overview
This PR targets Windows toolchain compatibility (MSVC cl.exe and clang-cl) by removing MSVC-sensitive template instantiation triggers, avoiding alternative operator tokens, and making the libxc FetchContent patch step portable.
Changes:
- Replaced
not/and/ortokens with!/&&/||in several translation units and headers. - Updated CMake to add MSVC/clang-cl-specific definitions and warning suppressions; replaced a
sed-based libxc patch with a CMake-Pscript. - Removed a dead
exx_coeffconstexpr indeorbitalized.hppand adjusted exception code to address MSVC deprecation warnings.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/xc_functional.cxx |
Replaces alternative operator tokens to improve MSVC compatibility. |
src/libxc.cxx |
Replaces or with ` |
src/CMakeLists.txt |
Adds MSVC/clang-cl compile definitions and warning suppression flags. |
src/builtin_interface.cxx |
Replaces or with ` |
include/exchcxx/xc_functional.hpp |
Replaces not/and with !/&& in sanity/type checks. |
include/exchcxx/impl/builtin/kernels/screening_interface.hpp |
Replaces not in if constexpr to satisfy MSVC parsing/compat. |
include/exchcxx/impl/builtin/kernels/deorbitalized.hpp |
Removes a dead constexpr that referenced non-existent traits members. |
include/exchcxx/exceptions/exchcxx_exception.hpp |
Switches to _strdup and updates macro to avoid not. |
CMakeLists.txt |
Replaces a sed PATCH_COMMAND with a portable CMake script invocation. |
cmake/patch_libxc_work_mgga.cmake |
New CMake script to patch libxc sources without Unix tools. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
61
to
70
| const char* what() const noexcept override { | ||
| std::stringstream ss; | ||
| ss << "EXCHCXX Exception (" << msg_prefix_ << ")" << std::endl | ||
| << " File " << file_ << std::endl | ||
| << " Line " << line_ << std::endl; | ||
|
|
||
| auto msg = ss.str(); | ||
|
|
||
| return strdup( msg.c_str() ); | ||
| return _strdup( msg.c_str() ); | ||
| } |
| /wd4267 # conversion from 'size_t' to 'type', possible loss of data | ||
| /wd4365 # conversion from 'type1' to 'type2', signed/unsigned mismatch | ||
| /wd4514 # unreferenced inline function has been removed | ||
| /wd4267 # conversion from 'size_t' to 'type', possible loss of data |
Comment on lines
+19
to
+49
| if(MSVC) | ||
| target_compile_definitions( exchcxx PUBLIC _USE_MATH_DEFINES ) | ||
| if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") # clang-cl | ||
| target_compile_options( exchcxx PUBLIC | ||
| -Wno-c++98-compat | ||
| -Wno-c++98-compat-local-type-template-args | ||
| -Wno-c++98-compat-pedantic | ||
| -Wno-deprecated-declarations | ||
| -Wno-exit-time-destructors | ||
| -Wno-extra-semi | ||
| -Wno-extra-semi-stmt | ||
| -Wno-float-conversion | ||
| -Wno-float-equal | ||
| -Wno-global-constructors | ||
| -Wno-inconsistent-missing-destructor-override | ||
| -Wno-missing-prototypes | ||
| -Wno-missing-variable-declarations | ||
| -Wno-old-style-cast | ||
| -Wno-pre-c++14-compat | ||
| -Wno-pre-c++17-compat | ||
| -Wno-reserved-macro-identifier | ||
| -Wno-sign-conversion | ||
| -Wno-suggest-destructor-override | ||
| -Wno-switch-enum | ||
| -Wno-undefined-func-template | ||
| -Wno-unsafe-buffer-usage | ||
| -Wno-unsafe-buffer-usage-in-libc-call | ||
| -Wno-unused-macros | ||
| -Wno-unused-parameter | ||
| -Wno-unused-variable | ||
| ) |
Comment on lines
+11
to
+13
| file(READ "${_file}" _content) | ||
| string(REPLACE "p->info->family != XC_KINETIC" "p->info->kind != XC_KINETIC" _content "${_content}") | ||
| file(WRITE "${_file}" "${_content}") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
exx_coeffstatic constexpr indeorbitalized.hppthat referenced non-existentkernel_traitsmembers (MSVC eagerly instantiates; GCC/Clang defer since never ODR-used)not,and,or) with!,&&,||across headers and source files (MSVC cl.exe requires/permissive-otherwise)_USE_MATH_DEFINEScompile definition forM_PIsedPATCH_COMMAND for libxc with portable CMake-Pscript so FetchContent works on Windows without Unix toolsstrdupwith_strdupto fix MSVC C4996 warning