MSVC: Miscellaneous uncontroversial fixes #1422
Conversation
sethk
commented
Apr 1, 2025
sethk
commented
- Copy __builtin_clz wrapper into another source file.
- mode_t is not defined on _WIN32
- Avoid arithmetic on void *
- Try to thin out of use of <windows.h>
- Copy __builtin_clz wrapper into another source file. - mode_t is not defined on _WIN32 - Avoid arithmetic on void * - Try to thin out of use of <windows.h>
This avoids bringing in all of windows.h, but should work with both MSVC and MINGW64.
| #endif | ||
| #ifdef HAVE_UNISTD_H | ||
| #include <unistd.h> | ||
| #endif |
There was a problem hiding this comment.
You are guarding all of these headers, but if you don't have any of these the code will fail to compile. Is that right?
There was a problem hiding this comment.
Sorry about the massive delay getting back to this, had to ship a demo.
Different combinations of headers are required for different runtimes. Most systems put many things into unistd.h, and then there's MSVC/UCRT, which doesn't have unistd.h, but instead isatty() and mkdir() are provided by corecrt_io.h and direct.h, respectively. MINGW64 defines _WIN32 but has unistd.h and lacks corecrt_io.h, meaning that we can't just use #ifdef _WIN32 to guard that. These header tests seem like the simplest way to address the full build matrix.
I have another change that checks for malloc.h/alloca.h so it can use alloca() in place of variable-length arrays. I separated it from this one because I think it could be more controversial.
There was a problem hiding this comment.
Forgot to mention: I revised this patch with some comments to clarify why each header is used. It's true that I could have done:
#ifdef HAVE_OTHER_HEADER_H
#include <other_header.h> // foo()
#elif defined(HAVE_UNISTD_H)
#include <unistd.h>
#else
#error "Don't know which header defines foo() on this system"
#endif
But unistd.h also has a ton of other stuff, and the pattern I used is one I've seen in many autoconf-based projects.
2 participants
