From bbe9a688312b155c000a8d36520d66d3e435ee78 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Wed, 17 Jul 2024 15:59:00 +0300 Subject: [PATCH 1/7] Use RAII for runtime/context --- R/JSContext.R | 35 ++++--- inst/include/quickjsr.hpp | 1 + inst/include/quickjsr/JS_RtCtxContainer.hpp | 28 +++++ src/quickjsr.cpp | 107 ++++++++------------ 4 files changed, 89 insertions(+), 82 deletions(-) create mode 100644 inst/include/quickjsr/JS_RtCtxContainer.hpp diff --git a/R/JSContext.R b/R/JSContext.R index 2c7e258..d2330c0 100644 --- a/R/JSContext.R +++ b/R/JSContext.R @@ -66,18 +66,18 @@ source <- NULL call <- NULL #' Get a variable from the current context -#' +#' #' @name JSContext-method-get #' @aliases get -#' +#' #' @usage get(var_name) -#' +#' #' @description #' Get the value of a variable from the current context -#' +#' #' @param var_name The name of the variable to retrieve #' @return The value of the variable -#' +#' #' @examples #' \dontrun{ #' ctx <- JSContext$new() @@ -87,19 +87,19 @@ call <- NULL get <- NULL #' Assign a value to a variable in the current context -#' +#' #' @name JSContext-method-assign #' @aliases assign -#' +#' #' @usage assign(var_name, value) #' #' @description #' Assign a value to a variable in the current context -#' +#' #' @param var_name The name of the variable to assign #' @param value The value to assign to the variable #' @return No return value, called for side effects -#' +#' #' @examples #' \dontrun{ #' ctx <- JSContext$new() @@ -110,14 +110,13 @@ assign <- NULL new_JSContext <- function(stack_size = NULL) { stack_size_int = ifelse(is.null(stack_size), -1, stack_size) - rt_and_ctx = qjs_context(stack_size_int) + rt_ctx = qjs_context(stack_size_int) ContextList = list( - runtime = rt_and_ctx$runtime_ptr, - context = rt_and_ctx$context_ptr + runtime_context_ptr = qjs_context(stack_size_int) ) ContextList$validate <- function(code_string) { - qjs_validate(ContextList$context, code_string) + qjs_validate(ContextList$runtime_context_ptr, code_string) } ContextList$source <- function(file = NULL, code = NULL) { @@ -127,10 +126,10 @@ new_JSContext <- function(stack_size = NULL) { warning("Both a filepath and code string cannot be provided,", " code will be ignored!", call. = FALSE) } - eval_success <- qjs_source(ContextList$context, + eval_success <- qjs_source(ContextList$runtime_context_ptr, input = normalizePath(file), is_file = TRUE) } else if (!is.null(code)) { - eval_success <- qjs_source(ContextList$context, input = code, is_file = FALSE) + eval_success <- qjs_source(ContextList$runtime_context_ptr, input = code, is_file = FALSE) } else { stop("No JS code provided!", call. = FALSE) } @@ -141,13 +140,13 @@ new_JSContext <- function(stack_size = NULL) { invisible(NULL) } ContextList$call <- function(function_name, ...) { - qjs_call(ContextList$context, function_name, ...) + qjs_call(ContextList$runtime_context_ptr, function_name, ...) } ContextList$get <- function(var_name) { - qjs_get(ContextList$context, var_name) + qjs_get(ContextList$runtime_context_ptr, var_name) } ContextList$assign <- function(var_name, value) { - qjs_assign(ContextList$context, var_name, value) + qjs_assign(ContextList$runtime_context_ptr, var_name, value) } structure( class = "JSContext", diff --git a/inst/include/quickjsr.hpp b/inst/include/quickjsr.hpp index 05b626e..5d2d7b1 100644 --- a/inst/include/quickjsr.hpp +++ b/inst/include/quickjsr.hpp @@ -7,5 +7,6 @@ #include #include #include +#include #endif diff --git a/inst/include/quickjsr/JS_RtCtxContainer.hpp b/inst/include/quickjsr/JS_RtCtxContainer.hpp new file mode 100644 index 0000000..7b6d5b1 --- /dev/null +++ b/inst/include/quickjsr/JS_RtCtxContainer.hpp @@ -0,0 +1,28 @@ +#ifndef QUICKJSR_JS_RTCTXCONTAINER_HPP +#define QUICKJSR_JS_RTCTXCONTAINER_HPP + +#include +#include + +namespace quickjsr { + +class JS_RtCtxContainer { + public: + JSRuntime* rt; + JSContext* ctx; + + JS_RtCtxContainer(int stack_size = 0) { + rt = quickjsr::JS_NewCustomRuntime(stack_size); + ctx = quickjsr::JS_NewCustomContext(rt); + } + + ~JS_RtCtxContainer() { + JS_FreeContext(ctx); + js_std_free_handlers(rt); + JS_FreeRuntime(rt); + } +}; + +} // namespace quickjsr + +#endif diff --git a/src/quickjsr.cpp b/src/quickjsr.cpp index c4c8442..08fa85e 100644 --- a/src/quickjsr.cpp +++ b/src/quickjsr.cpp @@ -4,14 +4,8 @@ #include #include -void JS_FreeRuntimeStdHandlers(JSRuntime* rt) { - js_std_free_handlers(rt); - JS_FreeRuntime(rt); -} - -// Register the cpp11 external pointer types with the correct cleanup/finaliser functions -using ContextXPtr = cpp11::external_pointer; -using RuntimeXPtr = cpp11::external_pointer; +using quickjsr::JS_RtCtxContainer; +using RtCtxXPtr = cpp11::external_pointer; extern "C" { SEXP qjs_context_(SEXP stack_size_) { @@ -19,27 +13,21 @@ extern "C" { int stack_size = Rf_isInteger(stack_size_) ? INTEGER_ELT(stack_size_, 0) : static_cast(REAL_ELT(stack_size_, 0)); - RuntimeXPtr rt(quickjsr::JS_NewCustomRuntime(stack_size)); - ContextXPtr ctx(quickjsr::JS_NewCustomContext(rt.get())); + RtCtxXPtr rt(new JS_RtCtxContainer(stack_size)); - cpp11::writable::list result; - using cpp11::literals::operator""_nm; - result.push_back({"runtime_ptr"_nm = rt}); - result.push_back({"context_ptr"_nm = ctx}); - - return cpp11::as_sexp(result); + return cpp11::as_sexp(rt); END_CPP11 } SEXP qjs_source_(SEXP ctx_ptr_, SEXP input_, SEXP is_file_) { BEGIN_CPP11 - JSContext* ctx = ContextXPtr(ctx_ptr_).get(); + RtCtxXPtr rt_ctx(ctx_ptr_); int ret; const char* input = Rf_translateCharUTF8(STRING_ELT(input_, 0)); if (LOGICAL_ELT(is_file_, 0)) { - ret = quickjsr::eval_file(ctx, input, -1); + ret = quickjsr::eval_file(rt_ctx->ctx, input, -1); } else { - ret = quickjsr::eval_buf(ctx, input, strlen(input), "", JS_EVAL_TYPE_GLOBAL); + ret = quickjsr::eval_buf(rt_ctx->ctx, input, strlen(input), "", JS_EVAL_TYPE_GLOBAL); } return cpp11::as_sexp(!ret); END_CPP11 @@ -47,37 +35,37 @@ extern "C" { SEXP qjs_validate_(SEXP ctx_ptr_, SEXP code_string_) { BEGIN_CPP11 - JSContext* ctx = ContextXPtr(ctx_ptr_).get(); + RtCtxXPtr rt_ctx(ctx_ptr_); const char* code_string = Rf_translateCharUTF8(STRING_ELT(code_string_, 0)); - JSValue val = JS_Eval(ctx, code_string, strlen(code_string), "", JS_EVAL_FLAG_COMPILE_ONLY); + JSValue val = JS_Eval(rt_ctx->ctx, code_string, strlen(code_string), "", JS_EVAL_FLAG_COMPILE_ONLY); bool failed = JS_IsException(val); - JS_FreeValue(ctx, val); + JS_FreeValue(rt_ctx->ctx, val); return cpp11::as_sexp(!failed); END_CPP11 } SEXP qjs_call_(SEXP ctx_ptr_, SEXP fun_name_, SEXP args_list_) { BEGIN_CPP11 - JSContext* ctx = ContextXPtr(ctx_ptr_).get(); + RtCtxXPtr rt_ctx(ctx_ptr_); int64_t n_args = Rf_xlength(args_list_); std::vector args(n_args); for (int64_t i = 0; i < n_args; i++) { - args[i] = quickjsr::SEXP_to_JSValue(ctx, VECTOR_ELT(args_list_, i), true); + args[i] = quickjsr::SEXP_to_JSValue(rt_ctx->ctx, VECTOR_ELT(args_list_, i), true); } - JSValue global = JS_GetGlobalObject(ctx); - JSValue fun = quickjsr::JS_GetPropertyRecursive(ctx, global, Rf_translateCharUTF8(STRING_ELT(fun_name_, 0))); - JSValue result_js = JS_Call(ctx, fun, global, args.size(), args.data()); + JSValue global = JS_GetGlobalObject(rt_ctx->ctx); + JSValue fun = quickjsr::JS_GetPropertyRecursive(rt_ctx->ctx, global, Rf_translateCharUTF8(STRING_ELT(fun_name_, 0))); + JSValue result_js = JS_Call(rt_ctx->ctx, fun, global, args.size(), args.data()); - SEXP result = quickjsr::JSValue_to_SEXP(ctx, result_js); + SEXP result = quickjsr::JSValue_to_SEXP(rt_ctx->ctx, result_js); - JS_FreeValue(ctx, result_js); + JS_FreeValue(rt_ctx->ctx, result_js); for (auto&& arg : args) { - JS_FreeValue(ctx, arg); + JS_FreeValue(rt_ctx->ctx, arg); } - JS_FreeValue(ctx, fun); - JS_FreeValue(ctx, global); + JS_FreeValue(rt_ctx->ctx, fun); + JS_FreeValue(rt_ctx->ctx, global); return result; END_CPP11 @@ -85,13 +73,13 @@ extern "C" { SEXP qjs_get_(SEXP ctx_ptr_, SEXP js_obj_name) { BEGIN_CPP11 - JSContext* ctx = ContextXPtr(ctx_ptr_).get(); - JSValue global = JS_GetGlobalObject(ctx); - JSValue result = quickjsr::JS_GetPropertyRecursive(ctx, global, Rf_translateCharUTF8(STRING_ELT(js_obj_name, 0))); - SEXP rtn = quickjsr::JSValue_to_SEXP(ctx, result); + RtCtxXPtr rt_ctx(ctx_ptr_); + JSValue global = JS_GetGlobalObject(rt_ctx->ctx); + JSValue result = quickjsr::JS_GetPropertyRecursive(rt_ctx->ctx, global, Rf_translateCharUTF8(STRING_ELT(js_obj_name, 0))); + SEXP rtn = quickjsr::JSValue_to_SEXP(rt_ctx->ctx, result); - JS_FreeValue(ctx, result); - JS_FreeValue(ctx, global); + JS_FreeValue(rt_ctx->ctx, result); + JS_FreeValue(rt_ctx->ctx, global); return rtn; END_CPP11 @@ -99,11 +87,11 @@ extern "C" { SEXP qjs_assign_(SEXP ctx_ptr_, SEXP js_obj_name_, SEXP value_) { BEGIN_CPP11 - JSContext* ctx = ContextXPtr(ctx_ptr_).get(); - JSValue global = JS_GetGlobalObject(ctx); - JSValue value = quickjsr::SEXP_to_JSValue(ctx, value_, true); - int result = quickjsr::JS_SetPropertyRecursive(ctx, global, Rf_translateCharUTF8(STRING_ELT(js_obj_name_, 0)), value); - JS_FreeValue(ctx, global); + RtCtxXPtr rt_ctx(ctx_ptr_); + JSValue global = JS_GetGlobalObject(rt_ctx->ctx); + JSValue value = quickjsr::SEXP_to_JSValue(rt_ctx->ctx, value_, true); + int result = quickjsr::JS_SetPropertyRecursive(rt_ctx->ctx, global, Rf_translateCharUTF8(STRING_ELT(js_obj_name_, 0)), value); + JS_FreeValue(rt_ctx->ctx, global); return cpp11::as_sexp(result); END_CPP11 @@ -112,15 +100,12 @@ extern "C" { SEXP qjs_eval_(SEXP eval_string_) { BEGIN_CPP11 const char* eval_string = Rf_translateCharUTF8(STRING_ELT(eval_string_, 0)); - JSRuntime* rt = quickjsr::JS_NewCustomRuntime(0); - JSContext* ctx = quickjsr::JS_NewCustomContext(rt); + JS_RtCtxContainer rt_ctx; - JSValue val = JS_Eval(ctx, eval_string, strlen(eval_string), "", JS_EVAL_TYPE_GLOBAL); - SEXP result = quickjsr::JSValue_to_SEXP(ctx, val); + JSValue val = JS_Eval(rt_ctx.ctx, eval_string, strlen(eval_string), "", JS_EVAL_TYPE_GLOBAL); + SEXP result = quickjsr::JSValue_to_SEXP(rt_ctx.ctx, val); - JS_FreeValue(ctx, val); - JS_FreeContext(ctx); - JS_FreeRuntimeStdHandlers(rt); + JS_FreeValue(rt_ctx.ctx, val); return result; END_CPP11 @@ -128,15 +113,12 @@ extern "C" { SEXP to_json_(SEXP arg_, SEXP auto_unbox_) { BEGIN_CPP11 - JSRuntime* rt = quickjsr::JS_NewCustomRuntime(0); - JSContext* ctx = quickjsr::JS_NewCustomContext(rt); + JS_RtCtxContainer rt_ctx; - JSValue arg = quickjsr::SEXP_to_JSValue(ctx, arg_, LOGICAL_ELT(auto_unbox_, 0)); - std::string result = quickjsr::JSValue_to_JSON(ctx, arg); + JSValue arg = quickjsr::SEXP_to_JSValue(rt_ctx.ctx, arg_, LOGICAL_ELT(auto_unbox_, 0)); + std::string result = quickjsr::JSValue_to_JSON(rt_ctx.ctx, arg); - JS_FreeValue(ctx, arg); - JS_FreeContext(ctx); - JS_FreeRuntimeStdHandlers(rt); + JS_FreeValue(rt_ctx.ctx, arg); return cpp11::as_sexp(result); END_CPP11 @@ -144,16 +126,13 @@ extern "C" { SEXP from_json_(SEXP json_) { BEGIN_CPP11 - JSRuntime* rt = quickjsr::JS_NewCustomRuntime(0); - JSContext* ctx = quickjsr::JS_NewCustomContext(rt); + JS_RtCtxContainer rt_ctx; const char* json = Rf_translateCharUTF8(STRING_ELT(json_, 0)); - JSValue result = JS_ParseJSON(ctx, json, strlen(json), ""); - SEXP rtn = quickjsr::JSValue_to_SEXP(ctx, result); + JSValue result = JS_ParseJSON(rt_ctx.ctx, json, strlen(json), ""); + SEXP rtn = quickjsr::JSValue_to_SEXP(rt_ctx.ctx, result); - JS_FreeValue(ctx, result); - JS_FreeContext(ctx); - JS_FreeRuntimeStdHandlers(rt); + JS_FreeValue(rt_ctx.ctx, result); return rtn; END_CPP11 From 1e54b41b4c5b94b41064120e1f1a650deda640fe Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Wed, 17 Jul 2024 16:00:37 +0300 Subject: [PATCH 2/7] Stray --- R/JSContext.R | 1 - 1 file changed, 1 deletion(-) diff --git a/R/JSContext.R b/R/JSContext.R index d2330c0..cf34470 100644 --- a/R/JSContext.R +++ b/R/JSContext.R @@ -110,7 +110,6 @@ assign <- NULL new_JSContext <- function(stack_size = NULL) { stack_size_int = ifelse(is.null(stack_size), -1, stack_size) - rt_ctx = qjs_context(stack_size_int) ContextList = list( runtime_context_ptr = qjs_context(stack_size_int) ) From e981ea0b6f5a65e3acb8702369dffb3353fd0fd0 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Wed, 17 Jul 2024 16:30:59 +0300 Subject: [PATCH 3/7] RAII wrapper for JSValue initial --- inst/include/quickjsr.hpp | 2 +- inst/include/quickjsr/JS_Containers.hpp | 42 +++++++++++++++++++++ inst/include/quickjsr/JS_RtCtxContainer.hpp | 28 -------------- src/quickjsr.cpp | 6 +-- 4 files changed, 46 insertions(+), 32 deletions(-) create mode 100644 inst/include/quickjsr/JS_Containers.hpp delete mode 100644 inst/include/quickjsr/JS_RtCtxContainer.hpp diff --git a/inst/include/quickjsr.hpp b/inst/include/quickjsr.hpp index 5d2d7b1..809e8f5 100644 --- a/inst/include/quickjsr.hpp +++ b/inst/include/quickjsr.hpp @@ -7,6 +7,6 @@ #include #include #include -#include +#include #endif diff --git a/inst/include/quickjsr/JS_Containers.hpp b/inst/include/quickjsr/JS_Containers.hpp new file mode 100644 index 0000000..725dec8 --- /dev/null +++ b/inst/include/quickjsr/JS_Containers.hpp @@ -0,0 +1,42 @@ +#ifndef QUICKJSR_JS_CONTAINERS_HPP +#define QUICKJSR_JS_CONTAINERS_HPP + +#include +#include +#include + +namespace quickjsr { + +struct JS_RtCtxContainer { + public: + JSRuntime* rt; + JSContext* ctx; + + JS_RtCtxContainer(int stack_size = 0) + : rt(JS_NewCustomRuntime(stack_size)), ctx(JS_NewCustomContext(rt)) {} + + ~JS_RtCtxContainer() { + JS_FreeContext(ctx); + js_std_free_handlers(rt); + JS_FreeRuntime(rt); + } +}; + +template +struct JS_ValContainer { + using RtCtxXPtr = cpp11::external_pointer; + public: + RtCtxXPtr rt_ctx; + JSValT val; + + JS_ValContainer(RtCtxXPtr in_rt_ctx, JSValT&& in_val) + : rt_ctx(in_rt_ctx), val(std::forward(in_val)) {} + + ~JS_ValContainer() { + JS_FreeValue(rt_ctx->ctx, val); + } +}; + +} // namespace quickjsr + +#endif diff --git a/inst/include/quickjsr/JS_RtCtxContainer.hpp b/inst/include/quickjsr/JS_RtCtxContainer.hpp deleted file mode 100644 index 7b6d5b1..0000000 --- a/inst/include/quickjsr/JS_RtCtxContainer.hpp +++ /dev/null @@ -1,28 +0,0 @@ -#ifndef QUICKJSR_JS_RTCTXCONTAINER_HPP -#define QUICKJSR_JS_RTCTXCONTAINER_HPP - -#include -#include - -namespace quickjsr { - -class JS_RtCtxContainer { - public: - JSRuntime* rt; - JSContext* ctx; - - JS_RtCtxContainer(int stack_size = 0) { - rt = quickjsr::JS_NewCustomRuntime(stack_size); - ctx = quickjsr::JS_NewCustomContext(rt); - } - - ~JS_RtCtxContainer() { - JS_FreeContext(ctx); - js_std_free_handlers(rt); - JS_FreeRuntime(rt); - } -}; - -} // namespace quickjsr - -#endif diff --git a/src/quickjsr.cpp b/src/quickjsr.cpp index 08fa85e..5577caa 100644 --- a/src/quickjsr.cpp +++ b/src/quickjsr.cpp @@ -5,6 +5,7 @@ #include using quickjsr::JS_RtCtxContainer; +using quickjsr::JS_ValContainer; using RtCtxXPtr = cpp11::external_pointer; extern "C" { @@ -56,11 +57,10 @@ extern "C" { JSValue global = JS_GetGlobalObject(rt_ctx->ctx); JSValue fun = quickjsr::JS_GetPropertyRecursive(rt_ctx->ctx, global, Rf_translateCharUTF8(STRING_ELT(fun_name_, 0))); - JSValue result_js = JS_Call(rt_ctx->ctx, fun, global, args.size(), args.data()); + JS_ValContainer result_js(rt_ctx, JS_Call(rt_ctx->ctx, fun, global, args.size(), args.data())); - SEXP result = quickjsr::JSValue_to_SEXP(rt_ctx->ctx, result_js); + SEXP result = quickjsr::JSValue_to_SEXP(rt_ctx->ctx, result_js.val); - JS_FreeValue(rt_ctx->ctx, result_js); for (auto&& arg : args) { JS_FreeValue(rt_ctx->ctx, arg); } From 70157615f20b1391dfd07216589d87e46d7b15cd Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Wed, 17 Jul 2024 16:53:48 +0300 Subject: [PATCH 4/7] Wrap more calls --- inst/include/quickjsr/JS_Containers.hpp | 4 +- src/quickjsr.cpp | 77 +++++++------------------ 2 files changed, 25 insertions(+), 56 deletions(-) diff --git a/inst/include/quickjsr/JS_Containers.hpp b/inst/include/quickjsr/JS_Containers.hpp index 725dec8..e9866d1 100644 --- a/inst/include/quickjsr/JS_Containers.hpp +++ b/inst/include/quickjsr/JS_Containers.hpp @@ -22,9 +22,11 @@ struct JS_RtCtxContainer { } }; + +using RtCtxXPtr = cpp11::external_pointer; + template struct JS_ValContainer { - using RtCtxXPtr = cpp11::external_pointer; public: RtCtxXPtr rt_ctx; JSValT val; diff --git a/src/quickjsr.cpp b/src/quickjsr.cpp index 5577caa..05e4ddd 100644 --- a/src/quickjsr.cpp +++ b/src/quickjsr.cpp @@ -6,17 +6,14 @@ using quickjsr::JS_RtCtxContainer; using quickjsr::JS_ValContainer; -using RtCtxXPtr = cpp11::external_pointer; +using quickjsr::RtCtxXPtr; extern "C" { SEXP qjs_context_(SEXP stack_size_) { BEGIN_CPP11 int stack_size = Rf_isInteger(stack_size_) ? INTEGER_ELT(stack_size_, 0) : static_cast(REAL_ELT(stack_size_, 0)); - - RtCtxXPtr rt(new JS_RtCtxContainer(stack_size)); - - return cpp11::as_sexp(rt); + return RtCtxXPtr(new JS_RtCtxContainer(stack_size)); END_CPP11 } @@ -38,10 +35,8 @@ extern "C" { BEGIN_CPP11 RtCtxXPtr rt_ctx(ctx_ptr_); const char* code_string = Rf_translateCharUTF8(STRING_ELT(code_string_, 0)); - JSValue val = JS_Eval(rt_ctx->ctx, code_string, strlen(code_string), "", JS_EVAL_FLAG_COMPILE_ONLY); - bool failed = JS_IsException(val); - JS_FreeValue(rt_ctx->ctx, val); - return cpp11::as_sexp(!failed); + JS_ValContainer val(rt_ctx, JS_Eval(rt_ctx->ctx, code_string, strlen(code_string), "", JS_EVAL_FLAG_COMPILE_ONLY)); + return cpp11::as_sexp(!JS_IsException(val.val)); END_CPP11 } @@ -55,43 +50,29 @@ extern "C" { args[i] = quickjsr::SEXP_to_JSValue(rt_ctx->ctx, VECTOR_ELT(args_list_, i), true); } - JSValue global = JS_GetGlobalObject(rt_ctx->ctx); - JSValue fun = quickjsr::JS_GetPropertyRecursive(rt_ctx->ctx, global, Rf_translateCharUTF8(STRING_ELT(fun_name_, 0))); - JS_ValContainer result_js(rt_ctx, JS_Call(rt_ctx->ctx, fun, global, args.size(), args.data())); - - SEXP result = quickjsr::JSValue_to_SEXP(rt_ctx->ctx, result_js.val); - - for (auto&& arg : args) { - JS_FreeValue(rt_ctx->ctx, arg); - } - JS_FreeValue(rt_ctx->ctx, fun); - JS_FreeValue(rt_ctx->ctx, global); + JS_ValContainer global(rt_ctx, JS_GetGlobalObject(rt_ctx->ctx)); + JS_ValContainer fun(rt_ctx, quickjsr::JS_GetPropertyRecursive(rt_ctx->ctx, global.val, Rf_translateCharUTF8(STRING_ELT(fun_name_, 0)))); + JS_ValContainer result_js(rt_ctx, JS_Call(rt_ctx->ctx, fun.val, global.val, args.size(), args.data())); - return result; + return quickjsr::JSValue_to_SEXP(rt_ctx->ctx, result_js.val); END_CPP11 } SEXP qjs_get_(SEXP ctx_ptr_, SEXP js_obj_name) { BEGIN_CPP11 RtCtxXPtr rt_ctx(ctx_ptr_); - JSValue global = JS_GetGlobalObject(rt_ctx->ctx); - JSValue result = quickjsr::JS_GetPropertyRecursive(rt_ctx->ctx, global, Rf_translateCharUTF8(STRING_ELT(js_obj_name, 0))); - SEXP rtn = quickjsr::JSValue_to_SEXP(rt_ctx->ctx, result); - - JS_FreeValue(rt_ctx->ctx, result); - JS_FreeValue(rt_ctx->ctx, global); - - return rtn; + JS_ValContainer global(rt_ctx, JS_GetGlobalObject(rt_ctx->ctx)); + JS_ValContainer result(rt_ctx, quickjsr::JS_GetPropertyRecursive(rt_ctx->ctx, global.val, Rf_translateCharUTF8(STRING_ELT(js_obj_name, 0)))); + return quickjsr::JSValue_to_SEXP(rt_ctx->ctx, result.val); END_CPP11 } SEXP qjs_assign_(SEXP ctx_ptr_, SEXP js_obj_name_, SEXP value_) { BEGIN_CPP11 RtCtxXPtr rt_ctx(ctx_ptr_); - JSValue global = JS_GetGlobalObject(rt_ctx->ctx); + JS_ValContainer global(rt_ctx, JS_GetGlobalObject(rt_ctx->ctx)); JSValue value = quickjsr::SEXP_to_JSValue(rt_ctx->ctx, value_, true); - int result = quickjsr::JS_SetPropertyRecursive(rt_ctx->ctx, global, Rf_translateCharUTF8(STRING_ELT(js_obj_name_, 0)), value); - JS_FreeValue(rt_ctx->ctx, global); + int result = quickjsr::JS_SetPropertyRecursive(rt_ctx->ctx, global.val, Rf_translateCharUTF8(STRING_ELT(js_obj_name_, 0)), value); return cpp11::as_sexp(result); END_CPP11 @@ -100,41 +81,27 @@ extern "C" { SEXP qjs_eval_(SEXP eval_string_) { BEGIN_CPP11 const char* eval_string = Rf_translateCharUTF8(STRING_ELT(eval_string_, 0)); - JS_RtCtxContainer rt_ctx; - - JSValue val = JS_Eval(rt_ctx.ctx, eval_string, strlen(eval_string), "", JS_EVAL_TYPE_GLOBAL); - SEXP result = quickjsr::JSValue_to_SEXP(rt_ctx.ctx, val); - - JS_FreeValue(rt_ctx.ctx, val); - - return result; + RtCtxXPtr rt_ctx(new JS_RtCtxContainer()); + JS_ValContainer val(rt_ctx, JS_Eval(rt_ctx->ctx, eval_string, strlen(eval_string), "", JS_EVAL_TYPE_GLOBAL)); + return quickjsr::JSValue_to_SEXP(rt_ctx->ctx, val.val); END_CPP11 } SEXP to_json_(SEXP arg_, SEXP auto_unbox_) { BEGIN_CPP11 - JS_RtCtxContainer rt_ctx; - - JSValue arg = quickjsr::SEXP_to_JSValue(rt_ctx.ctx, arg_, LOGICAL_ELT(auto_unbox_, 0)); - std::string result = quickjsr::JSValue_to_JSON(rt_ctx.ctx, arg); - - JS_FreeValue(rt_ctx.ctx, arg); - - return cpp11::as_sexp(result); + RtCtxXPtr rt_ctx(new JS_RtCtxContainer()); + JS_ValContainer arg(rt_ctx, quickjsr::SEXP_to_JSValue(rt_ctx->ctx, arg_, LOGICAL_ELT(auto_unbox_, 0))); + return cpp11::as_sexp(quickjsr::JSValue_to_JSON(rt_ctx->ctx, arg.val)); END_CPP11 } SEXP from_json_(SEXP json_) { BEGIN_CPP11 - JS_RtCtxContainer rt_ctx; + RtCtxXPtr rt_ctx(new JS_RtCtxContainer()); const char* json = Rf_translateCharUTF8(STRING_ELT(json_, 0)); - JSValue result = JS_ParseJSON(rt_ctx.ctx, json, strlen(json), ""); - SEXP rtn = quickjsr::JSValue_to_SEXP(rt_ctx.ctx, result); - - JS_FreeValue(rt_ctx.ctx, result); - - return rtn; + JS_ValContainer result(rt_ctx, JS_ParseJSON(rt_ctx->ctx, json, strlen(json), "")); + return quickjsr::JSValue_to_SEXP(rt_ctx->ctx, result.val); END_CPP11 } From 945f1d302fff058830138aa8348d63565c52205b Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Wed, 17 Jul 2024 17:01:56 +0300 Subject: [PATCH 5/7] Test fixes valgrind --- .github/workflows/R-CMD-check-cross.yaml | 4 ++-- inst/include/quickjsr/JS_Containers.hpp | 5 ++--- src/quickjsr.cpp | 3 ++- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/R-CMD-check-cross.yaml b/.github/workflows/R-CMD-check-cross.yaml index 383ef99..9e706a6 100644 --- a/.github/workflows/R-CMD-check-cross.yaml +++ b/.github/workflows/R-CMD-check-cross.yaml @@ -1,8 +1,8 @@ on: push: branches: [main, master] - pull_request: - branches: [main, master] + #pull_request: + #branches: [main, master] name: R-CMD-check-crossplatform diff --git a/inst/include/quickjsr/JS_Containers.hpp b/inst/include/quickjsr/JS_Containers.hpp index e9866d1..3021650 100644 --- a/inst/include/quickjsr/JS_Containers.hpp +++ b/inst/include/quickjsr/JS_Containers.hpp @@ -22,15 +22,14 @@ struct JS_RtCtxContainer { } }; - using RtCtxXPtr = cpp11::external_pointer; -template struct JS_ValContainer { public: RtCtxXPtr rt_ctx; - JSValT val; + JSValue val; + template JS_ValContainer(RtCtxXPtr in_rt_ctx, JSValT&& in_val) : rt_ctx(in_rt_ctx), val(std::forward(in_val)) {} diff --git a/src/quickjsr.cpp b/src/quickjsr.cpp index 05e4ddd..b9c3f9c 100644 --- a/src/quickjsr.cpp +++ b/src/quickjsr.cpp @@ -13,7 +13,8 @@ extern "C" { BEGIN_CPP11 int stack_size = Rf_isInteger(stack_size_) ? INTEGER_ELT(stack_size_, 0) : static_cast(REAL_ELT(stack_size_, 0)); - return RtCtxXPtr(new JS_RtCtxContainer(stack_size)); + RtCtxXPtr rt(new JS_RtCtxContainer(stack_size)); + return cpp11::as_sexp(rt); END_CPP11 } From 6bfc3693e643bb6671bca68934409fb170944d04 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Wed, 17 Jul 2024 17:06:34 +0300 Subject: [PATCH 6/7] Test fixes valgrind --- src/quickjsr.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/quickjsr.cpp b/src/quickjsr.cpp index b9c3f9c..fcc1472 100644 --- a/src/quickjsr.cpp +++ b/src/quickjsr.cpp @@ -55,6 +55,10 @@ extern "C" { JS_ValContainer fun(rt_ctx, quickjsr::JS_GetPropertyRecursive(rt_ctx->ctx, global.val, Rf_translateCharUTF8(STRING_ELT(fun_name_, 0)))); JS_ValContainer result_js(rt_ctx, JS_Call(rt_ctx->ctx, fun.val, global.val, args.size(), args.data())); + for (auto&& arg : args) { + JS_FreeValue(rt_ctx->ctx, arg); + } + return quickjsr::JSValue_to_SEXP(rt_ctx->ctx, result_js.val); END_CPP11 } @@ -72,8 +76,8 @@ extern "C" { BEGIN_CPP11 RtCtxXPtr rt_ctx(ctx_ptr_); JS_ValContainer global(rt_ctx, JS_GetGlobalObject(rt_ctx->ctx)); - JSValue value = quickjsr::SEXP_to_JSValue(rt_ctx->ctx, value_, true); - int result = quickjsr::JS_SetPropertyRecursive(rt_ctx->ctx, global.val, Rf_translateCharUTF8(STRING_ELT(js_obj_name_, 0)), value); + JS_ValContainer value(rt_ctx, quickjsr::SEXP_to_JSValue(rt_ctx->ctx, value_, true)); + int result = quickjsr::JS_SetPropertyRecursive(rt_ctx->ctx, global.val, Rf_translateCharUTF8(STRING_ELT(js_obj_name_, 0)), value.val); return cpp11::as_sexp(result); END_CPP11 From 528173a6541df6ebd0f5cd0c9b91f104e566c3a6 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Wed, 17 Jul 2024 17:11:52 +0300 Subject: [PATCH 7/7] Final cross check --- .github/workflows/R-CMD-check-cross.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/R-CMD-check-cross.yaml b/.github/workflows/R-CMD-check-cross.yaml index 9e706a6..383ef99 100644 --- a/.github/workflows/R-CMD-check-cross.yaml +++ b/.github/workflows/R-CMD-check-cross.yaml @@ -1,8 +1,8 @@ on: push: branches: [main, master] - #pull_request: - #branches: [main, master] + pull_request: + branches: [main, master] name: R-CMD-check-crossplatform