Add C-API helpers for embedders: property accessors, typed setters, arg validation, exception/promise convenience#1475
Closed
andreasrosdal wants to merge 5 commits into
Conversation
…e convenience)
Adds header-only inline helpers to quickjs.h, motivated by patterns
repeated throughout the Nordstjernen browser's QuickJS binding:
JS_FreeCStringSafe no-op on a NULL pointer
JS_GetPropertyStrInt32 fetch+convert+free in one call
JS_GetPropertyStrInt64 "
JS_GetPropertyStrFloat64 "
JS_GetPropertyStrBool "
JS_GetExceptionAsString consume pending exception, format as
"<msg>" or "<msg>\n<stack>"
JS_ResolvePromise settle a JS_NewPromiseCapability pair
JS_RejectPromise "
Each helper consolidates a multi-line dance (get + null/undefined
check + convert + handle conversion error + free intermediate) into
one expression, and silently clears any pending exception raised by
the conversion so callers don't accidentally propagate it.
All additions are static inline; no ABI change, no quickjs.c changes.
api-test.c gains coverage for the new helpers.
JS_ToCString does not take ownership of its JSValue argument, so the new string from JS_NewString was leaked. ASan/LSan flagged it as a 30-byte direct leak in api-test when CI builds with QJS_ENABLE_ASAN. Bind the JSValue to a local and JS_FreeValue it after JS_ToCString, matching the convention used elsewhere in this file.
…ers, argument validation
Extends the embedder-convenience helpers in quickjs.h with three new
families, all header-only static inlines (no ABI change, no quickjs.c
changes):
JS_GetPropertyStrString fetch + convert-to-string + free in one
call; returns NULL on missing/null/error
with any pending exception cleared
JS_SetPropertyStrInt32 typed setters mirroring the typed getters,
JS_SetPropertyStrInt64 so embeddings can read and write numeric/
JS_SetPropertyStrFloat64 boolean/string properties without hand-
JS_SetPropertyStrBool writing the JS_New*/JS_SetPropertyStr
JS_SetPropertyStrString pair each time
JS_CheckArgc argument-validation helpers for native
JS_ArgInt32 callbacks: range-check argc, then bounds-
JS_ArgInt64 check + convert argv[idx]. On failure all
JS_ArgFloat64 throw a TypeError and return -1 so the
JS_ArgBool callback can simply propagate JS_EXCEPTION
JS_ArgCString
The argument helpers preserve the underlying conversion exception (so
callers can let it bubble up), unlike the property accessors which
intentionally swallow errors and return a fallback. JS_CheckArgc treats
a negative max as "no upper bound".
api-test.c gains coverage for all three families, including:
- getter that throws yields NULL with no leftover exception
- round-trip through eval to confirm setters store the JS-typed value
they advertise (typeof === number/string/boolean)
- a real native callback driven from JS that exercises CheckArgc,
ArgInt32, ArgFloat64, ArgCString, ArgBool and all four error paths
Verified clean under -DQJS_ENABLE_ASAN=ON.
https://claude.ai/code/session_011v7x9Nm8RCHQsVVfeP1ryY
andreasrosdal
changed the title
bnoordhuis
reviewed
May 17, 2026
Contributor
bnoordhuis
left a comment
bnoordhuis
left a comment
There was a problem hiding this comment.
Most of these are a little too inflexible / not generic enough to make the cut, IMO.
Comment on lines
+916
to
+919
| static inline void JS_FreeCStringSafe(JSContext *ctx, const char *ptr) | ||
| { | ||
| if (ptr) JS_FreeCString(ctx, ptr); | ||
| } |
Contributor
There was a problem hiding this comment.
Not needed, JS_FreeCString/js_free_cstring does an early return if ptr == NULL.
| { | ||
| JSValue v = JS_GetPropertyStr(ctx, this_obj, prop); | ||
| int32_t out = fallback; | ||
| if (!JS_IsUndefined(v) && !JS_IsNull(v) && !JS_IsException(v)) { |
Contributor
There was a problem hiding this comment.
JS semantics are that null and undefined get coerced to 0, making this surprising behavior (violates the principle of least surprise.)
Swallowing exceptions is also generally a no-go. If you want to do that in your own code, fine, but not in quickjs's API.
Most of the functions you add are arguably too big to be inline, also because it means bug fixes won't end up in user programs when they link against the shared library.
| Convenience for the common JS_GetException + JS_ToCString + | ||
| JS_GetPropertyStr("stack") dance. */ | ||
| static inline const char *JS_GetExceptionAsString(JSContext *ctx, | ||
| bool with_stack) |
Contributor
There was a problem hiding this comment.
Extensibility: second arg should be a flags argument.
| JS_FreeCStringSafe(ctx, stack_s); | ||
| return msg; | ||
| } | ||
| char *tmp = (char *)js_malloc(ctx, mlen + 1 + slen + 1); |
Contributor
There was a problem hiding this comment.
Superfluous cast (not necessary when you move it to a .c file.)
Contributor
Author
|
Ok,. Thank you for reviewing. |
Contributor
|
I'm not saying everything is unsalvageable. Something like JS_GetExceptionAsString is common1 and generic enough that it could make a good candidate. 1 Pretty much every program has something like it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds header-only
static inlinehelpers toquickjs.hthat consolidatemulti-line patterns repeated throughout embedder code (in our case, the
~10k-line binding layer of the Nordstjernen browser). No changes to
quickjs.c, no ABI impact.