Skip to content

Fixes to format preprocessing for Clang on Windows #886

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

Fixes to format preprocessing for Clang on Windows #886

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c7a9a92
Fixes to format preprocessing for Clang on Windows
HJfod Feb 5, 2025
2b1f713
Add Clang & Ninja workflow on Windows
HJfod Feb 8, 2025
51f5332
Fix Windows Clang + Ninja action
HJfod Feb 8, 2025
1607d47
Add JS_PRINTF_FORMAT to getopt_compat
HJfod Feb 8, 2025
bb6dfe0
Add missing JS_PRINTF_FORMAT
HJfod Feb 8, 2025
6731454
Fix warnings in quickjs-libc.c & update printf macro in quickjs.h
HJfod Feb 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,43 @@ jobs:
cl.exe /DJS_NAN_BOXING=0 /Zs cxxtest.cc
cl.exe /DJS_NAN_BOXING=1 /Zs cxxtest.cc

windows-clang:
windows-ninja-clang:
runs-on: windows-latest
strategy:
fail-fast: false
matrix:
buildType: [Debug, Release]
steps:
- uses: actions/checkout@v4
- name: install ninja
run: |
choco install ninja
ninja.exe --version
- name: build
run: |
git submodule update --init --checkout --depth 1
cmake -B build -DBUILD_EXAMPLES=ON -DCMAKE_BUILD_TYPE=${{matrix.buildType}} -DCMAKE_C_COMPILER=clang.exe -DCMAKE_CXX_COMPILER=clang++.exe -G "Ninja"
cmake --build build --config ${{matrix.buildType}}
- name: stats
run: |
build\${{matrix.buildType}}\qjs.exe -qd
- name: cxxtest
run: |
clang.exe /DJS_NAN_BOXING=0 /Zs cxxtest.cc
clang.exe /DJS_NAN_BOXING=1 /Zs cxxtest.cc
- name: test
run: |
cp build\${{matrix.buildType}}\fib.dll examples\
cp build\${{matrix.buildType}}\point.dll examples\
build\${{matrix.buildType}}\qjs.exe examples\test_fib.js
build\${{matrix.buildType}}\qjs.exe examples\test_point.js
build\${{matrix.buildType}}\run-test262.exe -c tests.conf
build\${{matrix.buildType}}\function_source.exe
- name: test interrupt
run: |
build\${{matrix.buildType}}\interrupt-test.exe

windows-clang-cl:
runs-on: windows-latest
strategy:
fail-fast: false
Expand Down
3 changes: 2 additions & 1 deletion cutils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ extern "C" {

/* Borrowed from Folly */
#ifndef JS_PRINTF_FORMAT
#ifdef _MSC_VER
/* Clang on Windows doesn't seem to support _Printf_format_string_ */
#if defined(_MSC_VER) && !defined(__clang__)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make this change also in quickjs.h, for consistency?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make this change also in quickjs.h, for consistency?

#include <sal.h>
#define JS_PRINTF_FORMAT _Printf_format_string_
#define JS_PRINTF_FORMAT_ATTR(format_param, dots_param)
Expand Down
6 changes: 4 additions & 2 deletions getopt_compat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@
#include <stdarg.h>
#include <stdio.h>
#include <windows.h>
/* Required for JS_PRINTF_FORMAT */
#include "cutils.h"

#ifdef __cplusplus
extern "C" {
Expand Down Expand Up @@ -145,7 +147,7 @@ static const char illoptchar[] = "unknown option -- %c";
static const char illoptstring[] = "unknown option -- %s";

static void
_vwarnx(const char *fmt,va_list ap)
JS_PRINTF_FORMAT_ATTR(1, 0) _vwarnx(JS_PRINTF_FORMAT const char *fmt,va_list ap)
{
(void)fprintf(stderr,"%s: ",__progname);
if (fmt != NULL)
Expand All @@ -154,7 +156,7 @@ _vwarnx(const char *fmt,va_list ap)
}

static void
warnx(const char *fmt,...)
JS_PRINTF_FORMAT_ATTR(1, 2) warnx(JS_PRINTF_FORMAT const char *fmt,...)
{
va_list ap;
va_start(ap,fmt);
Expand Down
8 changes: 4 additions & 4 deletions quickjs-libc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,10 @@ static void js_set_thread_state(JSRuntime *rt, JSThreadState *ts)
js_std_cmd(/*SetOpaque*/1, rt, ts);
}

#ifdef __GNUC__
#if defined(__GNUC__) || defined(__clang__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif // __GNUC__
#endif // __GNUC__ || __clang__
static JSValue js_printf_internal(JSContext *ctx,
int argc, JSValue *argv, FILE *fp)
{
Expand Down Expand Up @@ -409,9 +409,9 @@ static JSValue js_printf_internal(JSContext *ctx,
dbuf_free(&dbuf);
return JS_EXCEPTION;
}
#ifdef __GNUC__
#if defined(__GNUC__) || defined(__clang__)
#pragma GCC diagnostic pop // ignored "-Wformat-nonliteral"
#endif // __GNUC__
#endif // __GNUC__ || __clang__

uint8_t *js_load_file(JSContext *ctx, size_t *pbuf_len, const char *filename)
{
Expand Down
4 changes: 2 additions & 2 deletions quickjs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7048,7 +7048,7 @@ static int JS_PRINTF_FORMAT_ATTR(3, 4) JS_ThrowTypeErrorOrFalse(JSContext *ctx,
}
}

#ifdef __GNUC__
#if defined(__GNUC__) || defined(__clang__)
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought clang also defined GNUC, doesn't it?

Copy link
Author

Choose a reason for hiding this comment

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

At least locally that seems to not be the case when compiling on Windows, though not sure why

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif // __GNUC__
Expand All @@ -7065,7 +7065,7 @@ static JSValue JS_ThrowSyntaxErrorAtom(JSContext *ctx, const char *fmt, JSAtom a
JS_AtomGetStr(ctx, buf, sizeof(buf), atom);
return JS_ThrowSyntaxError(ctx, fmt, buf);
}
#ifdef __GNUC__
#if defined(__GNUC__) || defined(__clang__)
#pragma GCC diagnostic pop // ignored "-Wformat-nonliteral"
#endif // __GNUC__

Expand Down
2 changes: 1 addition & 1 deletion quickjs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ extern "C" {

/* Borrowed from Folly */
#ifndef JS_PRINTF_FORMAT
#ifdef _MSC_VER
#if defined(_MSC_VER) && !defined(__clang__)
#include <sal.h>
#define JS_PRINTF_FORMAT _Printf_format_string_
#define JS_PRINTF_FORMAT_ATTR(format_param, dots_param)
Expand Down
Loading