Fixes to format preprocessing for Clang on Windows#886
Fixes to format preprocessing for Clang on Windows#886HJfod wants to merge 6 commits intoquickjs-ng:masterfrom
Conversation
Clang on Windows defines _MSC_VER, but it doesn't understand _Printf_format_string_
|
Do you know how we could add a Clang on windows CI target? |
|
I believe just copying the action that uses clang-cl and making it use clang instead should work? Not entirely sure, can test it out on my fork |
|
Please add it to this PR so we test it there too going forward. |
| #ifndef JS_PRINTF_FORMAT | ||
| #ifdef _MSC_VER | ||
| /* Clang on Windows doesn't seem to support _Printf_format_string_ */ | ||
| #if defined(_MSC_VER) && !defined(__clang__) |
There was a problem hiding this comment.
Can you make this change also in quickjs.h, for consistency?
| #ifndef JS_PRINTF_FORMAT | ||
| #ifdef _MSC_VER | ||
| /* Clang on Windows doesn't seem to support _Printf_format_string_ */ | ||
| #if defined(_MSC_VER) && !defined(__clang__) |
There was a problem hiding this comment.
Can you make this change also in quickjs.h, for consistency?
| } | ||
|
|
||
| #ifdef __GNUC__ | ||
| #if defined(__GNUC__) || defined(__clang__) |
There was a problem hiding this comment.
I thought clang also defined GNUC, doesn't it?
There was a problem hiding this comment.
At least locally that seems to not be the case when compiling on Windows, though not sure why
|
I keep running into quirks with weird things not being compatible with Clang (for example the |
Clang on Windows defines
_MSC_VER, but it doesn't seem to understand_Printf_format_string_, leading to errors code using format string parameters as it fails to understand that the parameters are format strings. However, the__attribute__((format))syntax seems to work just fine, and so this PR enables that for Clang on Windows.For context, I am using clang (19.1.0) instead of clang-cl (because the latter doesn't work for some unknown reasons). I've checked that this still works on MSVC, though I don't know about the rest of the platforms because I didn't realize GitHub wouldn't run CI when I made the commit. Oh well, I'll see when CI runs for the PR, and push new commits to fix it if something goes wrong.