From c7a9a92d71621049ad990e3d6e881ca0c57b3ad8 Mon Sep 17 00:00:00 2001 From: HJfod <60038575+HJfod@users.noreply.github.com> Date: Wed, 5 Feb 2025 22:45:13 +0200 Subject: [PATCH 01/12] Fixes to format preprocessing for Clang on Windows Clang on Windows defines _MSC_VER, but it doesn't understand _Printf_format_string_ --- cutils.h | 3 ++- quickjs.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cutils.h b/cutils.h index 4639a6b93..70480df43 100644 --- a/cutils.h +++ b/cutils.h @@ -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__) #include #define JS_PRINTF_FORMAT _Printf_format_string_ #define JS_PRINTF_FORMAT_ATTR(format_param, dots_param) diff --git a/quickjs.c b/quickjs.c index faf5431f9..92f02ba1e 100644 --- a/quickjs.c +++ b/quickjs.c @@ -7048,7 +7048,7 @@ static int JS_PRINTF_FORMAT_ATTR(3, 4) JS_ThrowTypeErrorOrFalse(JSContext *ctx, } } -#ifdef __GNUC__ +#if defined(__GNUC__) || defined(__clang__) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wformat-nonliteral" #endif // __GNUC__ @@ -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__ From 2b1f7134ef3f5b8e9821c0c1c6b7f38ed7390963 Mon Sep 17 00:00:00 2001 From: HJfod <60038575+HJfod@users.noreply.github.com> Date: Sat, 8 Feb 2025 12:45:53 +0200 Subject: [PATCH 02/12] Add Clang & Ninja workflow on Windows --- .github/workflows/ci.yml | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8a1417bd6..7b0c19f89 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 -G "Ninja" -T Clang + 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 From 51f53322b5530b1c03f895cf707735c4565e6fc7 Mon Sep 17 00:00:00 2001 From: HJfod <60038575+HJfod@users.noreply.github.com> Date: Sat, 8 Feb 2025 12:50:20 +0200 Subject: [PATCH 03/12] Fix Windows Clang + Ninja action --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7b0c19f89..41b2a4807 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -255,7 +255,7 @@ jobs: - name: build run: | git submodule update --init --checkout --depth 1 - cmake -B build -DBUILD_EXAMPLES=ON -G "Ninja" -T Clang + 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: | From 1607d47fd83abe2ccc6643184acc6f4068de790c Mon Sep 17 00:00:00 2001 From: HJfod <60038575+HJfod@users.noreply.github.com> Date: Sat, 8 Feb 2025 13:38:11 +0200 Subject: [PATCH 04/12] Add JS_PRINTF_FORMAT to getopt_compat --- getopt_compat.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/getopt_compat.h b/getopt_compat.h index f98ba2739..c295d0801 100644 --- a/getopt_compat.h +++ b/getopt_compat.h @@ -68,6 +68,8 @@ #include #include #include +/* Required for JS_PRINTF_FORMAT */ +#include "cutils.h" #ifdef __cplusplus extern "C" { @@ -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) From bb6dfe084ce69d215c1a767e92757631c7eb61b3 Mon Sep 17 00:00:00 2001 From: HJfod <60038575+HJfod@users.noreply.github.com> Date: Sat, 8 Feb 2025 13:41:34 +0200 Subject: [PATCH 05/12] Add missing JS_PRINTF_FORMAT --- getopt_compat.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/getopt_compat.h b/getopt_compat.h index c295d0801..0a92b8fe6 100644 --- a/getopt_compat.h +++ b/getopt_compat.h @@ -156,7 +156,7 @@ JS_PRINTF_FORMAT_ATTR(1, 0) _vwarnx(JS_PRINTF_FORMAT 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); From 6731454c063d7b5c809e82ae56e9f94a5c11a22a Mon Sep 17 00:00:00 2001 From: HJfod <60038575+HJfod@users.noreply.github.com> Date: Sat, 8 Feb 2025 17:26:20 +0200 Subject: [PATCH 06/12] Fix warnings in quickjs-libc.c & update printf macro in quickjs.h --- quickjs-libc.c | 8 ++++---- quickjs.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/quickjs-libc.c b/quickjs-libc.c index e78487896..cade9acfc 100644 --- a/quickjs-libc.c +++ b/quickjs-libc.c @@ -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) { @@ -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) { diff --git a/quickjs.h b/quickjs.h index bcdd87165..47245e7a1 100644 --- a/quickjs.h +++ b/quickjs.h @@ -49,7 +49,7 @@ extern "C" { /* Borrowed from Folly */ #ifndef JS_PRINTF_FORMAT -#ifdef _MSC_VER +#if defined(_MSC_VER) && !defined(__clang__) #include #define JS_PRINTF_FORMAT _Printf_format_string_ #define JS_PRINTF_FORMAT_ATTR(format_param, dots_param) From deb219611efb713b328aa59dbbc8eba2e2c47570 Mon Sep 17 00:00:00 2001 From: HJfod <60038575+HJfod@users.noreply.github.com> Date: Sat, 15 Mar 2025 22:30:40 +0200 Subject: [PATCH 07/12] Fix Windows Clang build by suppressing CRT warnings --- CMakeLists.txt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 133de63f5..c59b4cafb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -42,6 +42,13 @@ xcheck_add_c_compiler_flag(-Wno-stringop-truncation) xcheck_add_c_compiler_flag(-Wno-array-bounds) xcheck_add_c_compiler_flag(-funsigned-char) +# Clang on Windows without MSVC command line fails because the codebase uses +# functions like strcpy over strcpy_s +if(CMAKE_C_COMPILER_ID STREQUAL "Clang" AND WIN32) +add_compile_definitions(_CRT_SECURE_NO_WARNINGS) +add_compile_definitions(_CRT_NONSTDC_NO_DEPRECATE) +endif() + # ClangCL is command line compatible with MSVC, so 'MSVC' is set. if(MSVC) xcheck_add_c_compiler_flag(-Wno-unsafe-buffer-usage) @@ -77,7 +84,7 @@ endif() if(WIN32) if(MSVC) set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /STACK:8388608") - else() + elseif(MINGW) set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--stack,8388608") endif() endif() From f08401eded4f8b67f57d2268d7d80fe0ba16a1b4 Mon Sep 17 00:00:00 2001 From: HJfod <60038575+HJfod@users.noreply.github.com> Date: Sat, 15 Mar 2025 22:37:20 +0200 Subject: [PATCH 08/12] Fix building with non-CL Clang on Windows It doesn't generate separate Debug and Release directories and instead puts the executable directly under build/ --- .github/workflows/ci.yml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f127b6ccd..11175d1c2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -249,26 +249,26 @@ jobs: - 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 -B build -DBUILD_EXAMPLES=ON -DCMAKE_BUILD_TYPE=${{matrix.buildType}} -DCMAKE_C_COMPILER=clang.exe -G "Ninja" cmake --build build --config ${{matrix.buildType}} - name: stats run: | - build\${{matrix.buildType}}\qjs.exe -qd + build\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 + cp build\fib.dll examples\ + cp build\point.dll examples\ + build\qjs.exe examples\test_fib.js + build\qjs.exe examples\test_point.js + build\run-test262.exe -c tests.conf + build\function_source.exe - name: test interrupt run: | - build\${{matrix.buildType}}\interrupt-test.exe + build\interrupt-test.exe windows-clang-cl: runs-on: windows-latest From de85fe6c0c661d1adc2f7d2755a722241fd6cb90 Mon Sep 17 00:00:00 2001 From: HJfod <60038575+HJfod@users.noreply.github.com> Date: Sat, 15 Mar 2025 23:12:58 +0200 Subject: [PATCH 09/12] More fixes to Windows Clang workflow --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 11175d1c2..1413b1022 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -256,8 +256,8 @@ jobs: build\qjs.exe -qd - name: cxxtest run: | - clang.exe /DJS_NAN_BOXING=0 /Zs cxxtest.cc - clang.exe /DJS_NAN_BOXING=1 /Zs cxxtest.cc + clang.exe -D JS_NAN_BOXING=0 -fsyntax-only cxxtest.cc + clang.exe -D JS_NAN_BOXING=1 -fsyntax-only cxxtest.cc - name: test run: | cp build\fib.dll examples\ From 0ccbeaef4f8d650d37419e3e8c2faa54b5b2b7a3 Mon Sep 17 00:00:00 2001 From: HJfod <60038575+HJfod@users.noreply.github.com> Date: Sun, 16 Mar 2025 00:03:46 +0200 Subject: [PATCH 10/12] Fix outdated name of QJS_BUILD_EXAMPLES option in Windows Clang --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1413b1022..04cdfa27d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -249,7 +249,7 @@ jobs: - 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 -G "Ninja" + cmake -B build -DQJS_BUILD_EXAMPLES=ON -DCMAKE_BUILD_TYPE=${{matrix.buildType}} -DCMAKE_C_COMPILER=clang.exe -G "Ninja" cmake --build build --config ${{matrix.buildType}} - name: stats run: | From 9529a1b9a7ada26457d6af4db81dc26281228c54 Mon Sep 17 00:00:00 2001 From: HJfod <60038575+HJfod@users.noreply.github.com> Date: Sun, 16 Mar 2025 00:08:09 +0200 Subject: [PATCH 11/12] Remove outdated interrupt test from Windows Clang action --- .github/workflows/ci.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 04cdfa27d..4dd5a3386 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -266,9 +266,6 @@ jobs: build\qjs.exe examples\test_point.js build\run-test262.exe -c tests.conf build\function_source.exe - - name: test interrupt - run: | - build\interrupt-test.exe windows-clang-cl: runs-on: windows-latest From 833f67aee0c0771007166902d9a06d40398b6e3b Mon Sep 17 00:00:00 2001 From: HJfod <60038575+HJfod@users.noreply.github.com> Date: Tue, 18 Mar 2025 22:09:35 +0200 Subject: [PATCH 12/12] Make changes according to PR code review * CI action renamed to just `windows-clang` * Remove inclusion of `cutils.h` from `getopt_compat.h` and just suppress format string literal warnings * Add more comments to `quickjs-libc.c` and `quickjs.h` * Update `CMakeLists.txt` to also set 8 MB stack size for Windows Clang --- .github/workflows/ci.yml | 2 +- CMakeLists.txt | 12 ++++++++---- getopt_compat.h | 14 ++++++++++---- quickjs-libc.c | 1 + quickjs.h | 1 + 5 files changed, 21 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4dd5a3386..501ced519 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -234,7 +234,7 @@ jobs: cl.exe /DJS_NAN_BOXING=0 /Zs cxxtest.cc cl.exe /DJS_NAN_BOXING=1 /Zs cxxtest.cc - windows-ninja-clang: + windows-clang: runs-on: windows-latest strategy: fail-fast: false diff --git a/CMakeLists.txt b/CMakeLists.txt index c59b4cafb..f7a29dcd9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -44,9 +44,9 @@ xcheck_add_c_compiler_flag(-funsigned-char) # Clang on Windows without MSVC command line fails because the codebase uses # functions like strcpy over strcpy_s -if(CMAKE_C_COMPILER_ID STREQUAL "Clang" AND WIN32) -add_compile_definitions(_CRT_SECURE_NO_WARNINGS) -add_compile_definitions(_CRT_NONSTDC_NO_DEPRECATE) +if(CMAKE_C_COMPILER_ID STREQUAL "Clang" AND WIN32 AND NOT MSVC) + add_compile_definitions(_CRT_SECURE_NO_WARNINGS) + add_compile_definitions(_CRT_NONSTDC_NO_DEPRECATE) endif() # ClangCL is command line compatible with MSVC, so 'MSVC' is set. @@ -84,7 +84,11 @@ endif() if(WIN32) if(MSVC) set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /STACK:8388608") - elseif(MINGW) + elseif(CMAKE_C_COMPILER_ID STREQUAL "Clang") + # Clang on Windows uses Windows' default linker, but requires GCC-style + # -Wl to specify a linker argument + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,/STACK:8388608") + else() set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--stack,8388608") endif() endif() diff --git a/getopt_compat.h b/getopt_compat.h index 0a92b8fe6..97d7f3c02 100644 --- a/getopt_compat.h +++ b/getopt_compat.h @@ -68,8 +68,6 @@ #include #include #include -/* Required for JS_PRINTF_FORMAT */ -#include "cutils.h" #ifdef __cplusplus extern "C" { @@ -146,17 +144,25 @@ static const char noarg[] = "option doesn't take an argument -- %.*s"; static const char illoptchar[] = "unknown option -- %c"; static const char illoptstring[] = "unknown option -- %s"; +// Non-CL Clang on Windows does not define __GNUC__ +#if defined(__GNUC__) || defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat-nonliteral" +#endif // __GNUC__ || __clang__ static void -JS_PRINTF_FORMAT_ATTR(1, 0) _vwarnx(JS_PRINTF_FORMAT const char *fmt,va_list ap) +_vwarnx(const char *fmt,va_list ap) { (void)fprintf(stderr,"%s: ",__progname); if (fmt != NULL) (void)vfprintf(stderr,fmt,ap); (void)fprintf(stderr,"\n"); } +#if defined(__GNUC__) || defined(__clang__) +#pragma GCC diagnostic pop // ignored "-Wformat-nonliteral" +#endif // __GNUC__ || __clang__ static void -JS_PRINTF_FORMAT_ATTR(1, 2) warnx(JS_PRINTF_FORMAT const char *fmt,...) +warnx(const char *fmt,...) { va_list ap; va_start(ap,fmt); diff --git a/quickjs-libc.c b/quickjs-libc.c index 5977c527b..1e95a7710 100644 --- a/quickjs-libc.c +++ b/quickjs-libc.c @@ -198,6 +198,7 @@ static void js_set_thread_state(JSRuntime *rt, JSThreadState *ts) js_std_cmd(/*SetOpaque*/1, rt, ts); } +// Non-CL Clang on Windows does not define __GNUC__ #if defined(__GNUC__) || defined(__clang__) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wformat-nonliteral" diff --git a/quickjs.h b/quickjs.h index 3798f49de..4d7903a5f 100644 --- a/quickjs.h +++ b/quickjs.h @@ -49,6 +49,7 @@ extern "C" { /* Borrowed from Folly */ #ifndef JS_PRINTF_FORMAT +/* Clang on Windows doesn't seem to support _Printf_format_string_ */ #if defined(_MSC_VER) && !defined(__clang__) #include #define JS_PRINTF_FORMAT _Printf_format_string_