Add C Closures#1228
Conversation
|
Thank you! Could you please add a test to api-test.c? |
| JSValue JS_NewCClosure(JSContext *ctx, JSCClosure *func, | ||
| int length, int magic, void *opaque, | ||
| void (*opaque_finalize)(void*)) |
There was a problem hiding this comment.
For consistency with e.g. JS_SetModuleLoaderFunc:
| JSValue JS_NewCClosure(JSContext *ctx, JSCClosure *func, | |
| int length, int magic, void *opaque, | |
| void (*opaque_finalize)(void*)) | |
| JSValue JS_NewCClosure(JSContext *ctx, JSCClosure *func, | |
| void (*opaque_finalize)(void*), | |
| int length, int magic, void *opaque) |
And I'd add a typedef for the finalizer callback.
There was a problem hiding this comment.
I'll defer to your judgement here, but after implementing it, it feels awkward to separate the finalizer from the data pointer. Other apis which accept opaque pointers do not have to manage the lifecycle of that pointer. Keeping the opaque_finalize argument last keeps the first 5 arguments consistent with the related JS_CFunction* apis.
There was a problem hiding this comment.
JS_EXTERN JSValue JS_NewCFunctionData2(JSContext *ctx, JSCFunctionData *func,
const char *name,
int length, int magic, int data_len,
JSValueConst *data);
typedef void JSCClosureFinalizerFunc(void*);
JS_EXTERN JSValue JS_NewCClosure(JSContext *ctx, JSCClosure *func,
const char* name,
int length, int magic,
void* opaque, JSCClosureFinalizerFunc *opaque_finalize);
|
Oops forgot the test. |
|
Hello. Anything I can do to help this be reviewed? |
This allows C objects to be attached to functions, and it also allows C code to be notified when these functions are finalized. Co-Authored-By: Andrew Krieger <akrieger@users.noreply.github.com>
|
There is a failing test, which seems relevant: https://github.com/quickjs-ng/quickjs/actions/runs/19453323256/job/55673631387#step:3:1 |
8a416e5 to
9da2fc4
Compare
|
I really should find a way to build these myself with a more strictly conforming compiler... apologies for the back and forth. |
- Add typedef for JSCClosureFinalizerFunc - Add `name` parameter to JS_NewCClosure - Wrap closure invocation with temporary JS stack frame for debugging. - Fix finalizer signature to be const correct.
bnoordhuis
left a comment
There was a problem hiding this comment.
Style nits but otherwise LGTM.
Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl>
|
Apologies for the delay, the github web UI was throwing errors attempting to commit the suggestions so I had to wait until I had full git access again. |
|
Cheers! |
This allows C objects to be attached to functions, and it also allows C code to be notified when these functions are finalized.
Import of https://github.com/tbluemel/quickjscpp/blob/98bf1ddb00e39d93840c4b67211c9260943b5a83/patches/2024-01-13/0001-Add-C-Closures.patch. Trivial renaming of functions and function annotations to match local style.
Closes #1212. Closes tbluemel/quickjscpp#3.