Skip to content

Commit 847c58b

Browse files
andrjohnsandrjohns
authored
Merge pull request #79 from andrjohns/null-handling
Fixes for null conversions
2 parents d046ce8 + 29c72f9 commit 847c58b

File tree

9 files changed

+124
-41
lines changed

9 files changed

+124
-41
lines changed

‎.github/workflows/R-CMD-check-cross.yaml‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,6 @@ jobs:
2525
- { platform: armhf, cc: gcc-14, cxx: g++-14 }
2626
- { platform: armhf, cc: clang-18, cxx: clang++-18 }
2727

28-
- { platform: arm64, cc: gcc-14, cxx: g++-14 }
29-
- { platform: arm64, cc: clang-18, cxx: clang++-18 }
30-
31-
- { platform: ppc64le, cc: gcc-14, cxx: g++-14 }
32-
- { platform: ppc64le, cc: clang-18, cxx: clang++-18 }
33-
34-
- { platform: riscv64, cc: gcc-14, cxx: g++-14 }
35-
- { platform: riscv64, cc: clang-18, cxx: clang++-18 }
36-
37-
#- { platform: s390x, cc: gcc-14, cxx: g++-14 }
38-
#- { platform: s390x, cc: clang-18, cxx: clang++-18 }
3928
- { platform: FreeBSD, cc: default, cxx: default }
4029

4130
env:

‎.github/workflows/R-CMD-check-special.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ jobs:
2525
- { label: "rchk",
2626
name: "rchk",
2727
container: "ghcr.io/r-hub/containers/rchk:latest"}
28+
- { label: "c23",
29+
name: "c23",
30+
container: "ghcr.io/r-hub/containers/c23:latest"}
31+
- { label: "gcc14",
32+
name: "gcc14",
33+
container: "ghcr.io/r-hub/containers/gcc14:latest"}
34+
- { label: "clang20",
35+
name: "clang20",
36+
container: "ghcr.io/r-hub/containers/clang20:latest"}
2837

2938
env:
3039
MAKEFLAGS: -j4

‎.github/workflows/R-CMD-check.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ jobs:
2626
- {os: windows-latest, r: 'release'}
2727
- {os: windows-latest, r: 'oldrel'}
2828

29+
- {os: ubuntu-24.04-arm, r: 'devel'}
2930
- {os: ubuntu-latest, r: 'devel', http-user-agent: 'release'}
3031
- {os: ubuntu-latest, r: 'release'}
3132
- {os: ubuntu-latest, r: 'oldrel'}

‎DESCRIPTION‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
Package: QuickJSR
22
Title: Interface for the 'QuickJS' Lightweight 'JavaScript' Engine
3-
Version: 1.5.1
3+
Version: 1.5.2
44
Authors@R: c(
55
person(c("Andrew", "R."), "Johnson", , "andrew.johnson@arjohnsonau.com", role = c("aut", "cre"),
66
comment = c(ORCID = "0000-0001-7000-8065")),
7-
person("Fabrice", "Bellard", role = c("cph"),
8-
comment = "Author of QuickJS sources and headers"),
9-
person("Charlie", "Gordon", role = c("cph"),
10-
comment = "Author of QuickJS sources and headers"),
7+
person("QuickJS", "Authors", role = c("cph"),
8+
comment = "QuickJS sources and headers"),
119
person("QuickJS-NG", "Authors", role = c("cph"),
1210
comment = "QuickJS-NG sources and headers")
1311
)

‎NEWS.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
# QuickJSR 1.5.2
2+
* Fix conversions of NULL/NA/undefined values between R and JS
3+
14
# QuickJSR 1.5.1
25
* Fix compilation under emscripten/WASM
36

‎inst/include/quickjsr/JSCommonType.hpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ enum JSCommonType {
2121
JSCommonType JS_ArrayCommonType(JSContext* ctx, const JSValue& val);
2222

2323
JSCommonType JS_GetCommonType(JSContext* ctx, const JSValue& val) {
24-
if (JS_IsUndefined(val)) {
24+
if (JS_IsUndefined(val) || JS_IsNull(val)) {
2525
return Undefined;
2626
}
2727
if (JS_IsBool(val)) {
@@ -63,6 +63,9 @@ JSCommonType JS_UpdateCommonType(JSCommonType current, JSContext* ctx, const JSV
6363
if (new_type == Undefined) {
6464
return current;
6565
}
66+
if (current == Undefined) {
67+
return new_type;
68+
}
6669
// If one, but not both, types are NumberArray or Date (checked above), return Object
6770
if (new_type == NumberArray || current == NumberArray || new_type == Object
6871
|| new_type == Date || current == Date) {

‎inst/include/quickjsr/JSValue_to_SEXP.hpp‎

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ namespace quickjsr {
1313
SEXP JSValue_to_SEXP(JSContext* ctx, const JSValue& val);
1414

1515
SEXP JSValue_to_SEXP_scalar(JSContext* ctx, const JSValue& val) {
16+
if (JS_IsNull(val)) {
17+
return R_NilValue;
18+
}
1619
if (JS_IsUndefined(val)) {
1720
return R_NilValue;
1821
}
@@ -37,19 +40,59 @@ SEXP JSValue_to_SEXP_scalar(JSContext* ctx, const JSValue& val) {
3740
}
3841

3942
SEXP JSValue_to_SEXP_vector(JSContext* ctx, const JSValue& val) {
43+
// Very inefficient workaround to identify null/undefined in array
44+
// so that they can be replaced with NA of the appropriate type in SEXP conversion
45+
std::vector<int64_t> null_idxs;
46+
int64_t len;
47+
JS_GetLength(ctx, val, &len);
48+
for (int64_t i = 0; i < len; i++) {
49+
JSValue elem = JS_GetPropertyInt64(ctx, val, i);
50+
if (JS_IsNull(elem) || JS_IsUndefined(elem)) {
51+
null_idxs.push_back(i);
52+
}
53+
JS_FreeValue(ctx, elem);
54+
}
4055
switch (JS_ArrayCommonType(ctx, val)) {
41-
case Integer:
42-
return cpp11::as_sexp(JSValue_to_Cpp<std::vector<int>>(ctx, val));
43-
case Double:
44-
return cpp11::as_sexp(JSValue_to_Cpp<std::vector<double>>(ctx, val));
45-
case Logical:
46-
return cpp11::as_sexp(JSValue_to_Cpp<std::vector<bool>>(ctx, val));
47-
case Character:
48-
return cpp11::as_sexp(JSValue_to_Cpp<std::vector<std::string>>(ctx, val));
49-
case Undefined:
50-
return R_NilValue;
56+
case Integer: {
57+
SEXP int_sexp(cpp11::as_sexp(JSValue_to_Cpp<std::vector<int>>(ctx, val)));
58+
for (int64_t idx : null_idxs) {
59+
SET_INTEGER_ELT(int_sexp, idx, NA_INTEGER);
60+
}
61+
return int_sexp;
62+
}
63+
case Double: {
64+
SEXP dbl_sexp(cpp11::as_sexp(JSValue_to_Cpp<std::vector<double>>(ctx, val)));
65+
for (int64_t idx : null_idxs) {
66+
SET_REAL_ELT(dbl_sexp, idx, NA_REAL);
67+
}
68+
return dbl_sexp;
69+
}
70+
case Logical: {
71+
SEXP lgl_sexp(cpp11::as_sexp(JSValue_to_Cpp<std::vector<bool>>(ctx, val)));
72+
for (int64_t idx : null_idxs) {
73+
SET_LOGICAL_ELT(lgl_sexp, idx, NA_LOGICAL);
74+
}
75+
return lgl_sexp;
76+
}
77+
case Character: {
78+
SEXP chr_sexp(cpp11::as_sexp(JSValue_to_Cpp<std::vector<std::string>>(ctx, val)));
79+
for (int64_t idx : null_idxs) {
80+
SET_STRING_ELT(chr_sexp, idx, NA_STRING);
81+
}
82+
return chr_sexp;
83+
}
84+
case Undefined: {
85+
cpp11::writable::logicals res(len);
86+
for (int64_t i = 0; i < len; i++) {
87+
res[static_cast<R_xlen_t>(i)] = NA_LOGICAL;
88+
}
89+
return res;
90+
}
5191
case Date: {
5292
cpp11::writable::doubles res = cpp11::as_sexp(JSValue_to_Cpp<std::vector<double>>(ctx, val));
93+
for (int64_t idx : null_idxs) {
94+
SET_REAL_ELT(res, idx, NA_REAL);
95+
}
5396
res.attr("class") = "POSIXct";
5497
return res;
5598
}
@@ -125,7 +168,7 @@ SEXP JSValue_to_SEXP(JSContext* ctx, const JSValue& val) {
125168
js_std_dump_error(ctx);
126169
return cpp11::as_sexp("Error!");
127170
}
128-
if (JS_IsUndefined(val)) {
171+
if (JS_IsUndefined(val) || JS_IsNull(val)) {
129172
return R_NilValue;
130173
}
131174
if (JS_IsArray(ctx, val)) {

‎inst/include/quickjsr/SEXP_to_JSValue.hpp‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -138,18 +138,28 @@ namespace quickjsr {
138138
return SEXP_to_JSValue(ctx, VECTOR_ELT(x, index), auto_unbox, auto_unbox_curr);
139139
}
140140
switch (TYPEOF(x)) {
141-
case LGLSXP:
141+
case NILSXP:
142+
return JS_NULL;
143+
case LGLSXP: {
144+
if (LOGICAL_ELT(x, index) == NA_LOGICAL) {
145+
return JS_NULL;
146+
}
142147
return JS_NewBool(ctx, LOGICAL_ELT(x, index));
148+
}
143149
case INTSXP: {
144-
if (Rf_inherits(x, "factor")) {
150+
if (INTEGER_ELT(x, index) == NA_INTEGER) {
151+
return JS_NULL;
152+
} else if (Rf_inherits(x, "factor")) {
145153
SEXP levels = Rf_getAttrib(x, R_LevelsSymbol);
146154
return JS_NewString(ctx, to_cstring(levels, INTEGER_ELT(x, index) - 1));
147155
} else {
148156
return JS_NewInt32(ctx, INTEGER_ELT(x, index));
149157
}
150158
}
151159
case REALSXP: {
152-
if (Rf_inherits(x, "POSIXct")) {
160+
if (ISNA(REAL_ELT(x, index))) {
161+
return JS_NULL;
162+
} else if (Rf_inherits(x, "POSIXct")) {
153163
static constexpr double milliseconds_second = 1000;
154164
double tz_offset_seconds = get_tz_offset_seconds();
155165
return JS_NewDate(ctx, (REAL_ELT(x, index) + tz_offset_seconds) * milliseconds_second);
@@ -160,26 +170,42 @@ namespace quickjsr {
160170
return JS_NewFloat64(ctx, REAL_ELT(x, index));
161171
}
162172
}
163-
case STRSXP:
173+
case STRSXP: {
174+
if (STRING_ELT(x, index) == NA_STRING) {
175+
return JS_NULL;
176+
}
164177
return JS_NewString(ctx, to_cstring(x, index));
178+
}
165179
case VECSXP:
166180
return SEXP_to_JSValue(ctx, VECTOR_ELT(x, index), auto_unbox, auto_unbox_curr);
167181
case CLOSXP:
168182
return SEXP_to_JSValue_function(ctx, x, auto_unbox, auto_unbox_curr);
169183
case ENVSXP:
170184
return SEXP_to_JSValue_env(ctx, x);
171-
case NILSXP:
172-
return JS_UNDEFINED;
173185
default:
174186
cpp11::stop("Conversions for type %s to JSValue are not yet implemented",
175187
Rf_type2char(TYPEOF(x)));
176188
}
177189
}
178190

191+
inline JSValue SEXP_to_JSValue_null(JSContext* ctx, bool auto_unbox) {
192+
if (auto_unbox) {
193+
return JS_NULL;
194+
} else {
195+
JSValue arr = JS_NewArray(ctx);
196+
JS_SetPropertyInt64(ctx, arr, 0, JS_NULL);
197+
return arr;
198+
}
199+
}
200+
179201
inline JSValue SEXP_to_JSValue(JSContext* ctx, const SEXP& x,
180202
bool auto_unbox_inp = false,
181203
bool auto_unbox = false) {
182204
bool auto_unbox_curr = static_cast<bool>(Rf_inherits(x, "AsIs")) ? false : auto_unbox_inp;
205+
if (Rf_isNull(x)) {
206+
return SEXP_to_JSValue_null(ctx, auto_unbox_curr);
207+
}
208+
183209
if (Rf_isDataFrame(x)) {
184210
return SEXP_to_JSValue_df(ctx, x, auto_unbox_inp, auto_unbox_curr);
185211
}

‎inst/tinytest/test_data_conversion.R‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
expect_equal(to_json(1), "[1]")
2-
expect_equal(to_json(1:3), "[1,2,3]")
3-
expect_equal(to_json(c(1.5, 2.5)), "[1.5,2.5]")
2+
expect_equal(to_json(c(NA, 1:3, NA)), "[null,1,2,3,null]")
3+
expect_equal(to_json(c(NA, 1.5, NA, 2.5)), "[null,1.5,null,2.5]")
44

55
expect_equal(to_json("a"), "[\"a\"]")
6-
expect_equal(to_json(c("a", "b", "c")), "[\"a\",\"b\",\"c\"]")
6+
expect_equal(to_json(c("a", "b", NA, "c")), "[\"a\",\"b\",null,\"c\"]")
77

88
expect_equal(to_json(TRUE), "[true]")
99
expect_equal(to_json(FALSE), "[false]")
10-
expect_equal(to_json(c(TRUE, FALSE)), "[true,false]")
10+
expect_equal(to_json(c(TRUE, NA, FALSE)), "[true,null,false]")
1111

12-
expect_equal(to_json(list(1, 2, 3)), "[[1],[2],[3]]")
13-
expect_equal(to_json(list(a = 1, b = 2, c = 3)),
14-
"{\"a\":[1],\"b\":[2],\"c\":[3]}")
12+
expect_equal(to_json(list(1, 2, 3, NA)), "[[1],[2],[3],[null]]")
13+
expect_equal(to_json(list(a = 1, b = 2, c = NA, d = 3)),
14+
"{\"a\":[1],\"b\":[2],\"c\":[null],\"d\":[3]}")
1515
expect_equal(to_json(list(a = "d", b = "e", c = "f")),
1616
"{\"a\":[\"d\"],\"b\":[\"e\"],\"c\":[\"f\"]}")
1717

@@ -59,3 +59,14 @@ expect_equal(list(list(a = 1, b = 2), list(c = 3, d = 4)), from_json("[{\"a\":[1
5959
expect_equal(list(c("e", "f"), c("g", "h")), from_json("[[\"e\",\"f\"],[\"g\",\"h\"]]"))
6060
expect_equal(list(list("e", "f"), list("g", "h")), from_json("[[[\"e\"],[\"f\"]],[[\"g\"],[\"h\"]]]"))
6161
expect_equal(list(list(a = "e", b = "f"), list(c = "g", d = "h")), from_json("[{\"a\":[\"e\"],\"b\":[\"f\"]},{\"c\":[\"g\"],\"d\":[\"h\"]}]"))
62+
63+
# NULL conversions
64+
expect_equal(NULL, from_json("null"))
65+
expect_equal(NA, from_json("[null]"))
66+
expect_equal(c(NA, 1), from_json("[null, 1]"))
67+
expect_equal(c(1, NA, 1), from_json("[1, null, 1]"))
68+
69+
expect_equal("[null]", to_json(NULL))
70+
expect_equal("null", to_json(NULL, auto_unbox = TRUE))
71+
expect_equal("[null]", to_json(NA))
72+
expect_equal("null", to_json(NA, auto_unbox = TRUE))

0 commit comments

Comments
 (0)