Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Support for non-CL Clang on Windows #981

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Support for non-CL Clang on Windows #981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 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
3073f11
Merge branch 'master' of https://github.com/quickjs-ng/quickjs
HJfod Mar 15, 2025
deb2196
Fix Windows Clang build by suppressing CRT warnings
HJfod Mar 15, 2025
f08401e
Fix building with non-CL Clang on Windows
HJfod Mar 15, 2025
de85fe6
More fixes to Windows Clang workflow
HJfod Mar 15, 2025
0ccbeae
Fix outdated name of QJS_BUILD_EXAMPLES option in Windows Clang
HJfod Mar 15, 2025
9529a1b
Remove outdated interrupt test from Windows Clang action
HJfod Mar 15, 2025
e311961
Merge branch 'master' of https://github.com/quickjs-ng/quickjs
HJfod Mar 16, 2025
833f67a
Make changes according to PR code review
HJfod Mar 18, 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
33 changes: 33 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,39 @@ jobs:
cl.exe /DJS_NAN_BOXING=1 /Zs cxxtest.cc

windows-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 -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: |
build\qjs.exe -qd
- name: cxxtest
run: |
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\
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

windows-clang-cl:
runs-on: windows-latest
strategy:
fail-fast: false
Expand Down
11 changes: 11 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 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.
if(MSVC)
xcheck_add_c_compiler_flag(-Wno-unsafe-buffer-usage)
Expand Down Expand Up @@ -77,6 +84,10 @@ endif()
if(WIN32)
if(MSVC)
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /STACK:8388608")
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()
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__)
#include <sal.h>
#define JS_PRINTF_FORMAT _Printf_format_string_
#define JS_PRINTF_FORMAT_ATTR(format_param, dots_param)
Expand Down
8 changes: 8 additions & 0 deletions getopt_compat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ 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
_vwarnx(const char *fmt,va_list ap)
{
Expand All @@ -152,6 +157,9 @@ _vwarnx(const char *fmt,va_list ap)
(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
warnx(const char *fmt,...)
Expand Down
9 changes: 5 additions & 4 deletions quickjs-libc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,11 @@ static void js_set_thread_state(JSRuntime *rt, JSThreadState *ts)
js_std_cmd(/*SetOpaque*/1, rt, ts);
}

#ifdef __GNUC__
// Non-CL Clang on Windows does not define __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.

Doesn't clang-on-windows define __GNUC__? Because clang-on-linux definitely does:

$ clang -E -dM - < /dev/null | grep GNUC
#define __GNUC_MINOR__ 2
#define __GNUC_PATCHLEVEL__ 1
#define __GNUC_STDC_INLINE__ 1
#define __GNUC__ 4

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it does too. What are you trying to fix here @HJfod ?

Copy link
Author

Choose a reason for hiding this comment

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

image
Removing the defined(__clang__) check makes it not work, so seems like Clang doesn't define __GNUC__

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif // __GNUC__
#endif // __GNUC__ || __clang__
static JSValue js_printf_internal(JSContext *ctx,
int argc, JSValueConst *argv, FILE *fp)
{
Expand Down Expand Up @@ -417,9 +418,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 @@ -6985,7 +6985,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__
Expand All @@ -7002,7 +7002,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
3 changes: 2 additions & 1 deletion quickjs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,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 <sal.h>
#define JS_PRINTF_FORMAT _Printf_format_string_
#define JS_PRINTF_FORMAT_ATTR(format_param, dots_param)
Expand Down