Skip to content

Commit 2337a7c

Browse files
committed
Update to make returning list almost feel like R
1 parent 29a6b94 commit 2337a7c

2 files changed

Lines changed: 84 additions & 41 deletions

File tree

R/c-wrapper.R

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -73,19 +73,32 @@ make_c_bridge <- function(fsub, strict = TRUE, headers = TRUE) {
7373
}
7474
append(c_body) <- glue("return {return_var_names};")
7575
} else {
76-
return_var_names_un <- unname(return_var_names)
76+
return_var_values <- unname(return_var_names)
77+
provided_names <- names(return_var_names)
78+
if (is.null(provided_names)) provided_names <- rep("", length(return_var_values))
79+
has_any_names <- any(nzchar(provided_names))
80+
7781
append(c_body) <- c(
78-
glue("SEXP _ans = PROTECT(Rf_allocVector(VECSXP, {length(return_var_names_un)}));"),
79-
imap(return_var_names_un, function(nm, i) {
82+
glue("SEXP _ans = PROTECT(Rf_allocVector(VECSXP, {length(return_var_values)}));"),
83+
imap(return_var_values, function(nm, i) {
8084
glue("SET_VECTOR_ELT(_ans, {i-1}, {nm});")
81-
}),
82-
glue("SEXP _names = PROTECT(Rf_allocVector(STRSXP, {length(return_var_names_un)}));"),
83-
imap(return_var_names_un, function(nm, i) {
84-
glue("SET_STRING_ELT(_names, {i-1}, Rf_mkChar(\"{nm}\"));")
85-
}),
86-
"Rf_setAttrib(_ans, R_NamesSymbol, _names);"
85+
})
8786
)
88-
append(c_body) <- glue("UNPROTECT({n_protected + 2});")
87+
88+
if (has_any_names) {
89+
names_to_use <- provided_names
90+
append(c_body) <- c(
91+
glue("SEXP _names = PROTECT(Rf_allocVector(STRSXP, {length(return_var_values)}));"),
92+
imap(names_to_use, function(nm, i) {
93+
glue('SET_STRING_ELT(_names, {i-1}, Rf_mkChar("{nm}"));')
94+
}),
95+
"Rf_setAttrib(_ans, R_NamesSymbol, _names);"
96+
)
97+
append(c_body) <- glue("UNPROTECT({n_protected + 2});")
98+
} else {
99+
append(c_body) <- glue("UNPROTECT({n_protected + 1});")
100+
}
101+
89102
append(c_body) <- "return _ans;"
90103
}
91104

@@ -439,14 +452,21 @@ as_friendly_size_expression <- function(d) {
439452
closure_return_var_names <- function(closure) {
440453
return_var_expr <- last(body(closure))
441454
if (is.symbol(return_var_expr)) {
442-
return(as.character(return_var_expr))
455+
val <- as.character(return_var_expr)
456+
# Return named to keep interface consistent
457+
return(setNames(val, val))
443458
}
444459
if (is_call(return_var_expr, quote(list))) {
445460
args <- as.list(return_var_expr)[-1L]
446-
return(map_chr(args, as.character))
461+
vals <- map_chr(args, as.character)
462+
nms <- names(args)
463+
if (is.null(nms)) nms <- rep("", length(vals))
464+
# Use provided names when present; fallback to symbol names
465+
# nms <- ifelse(nzchar(nms), nms, vals)
466+
return(setNames(vals, nms))
447467
}
448-
## is it redundent ? new_fortran_subroutine also errors ?
449-
stop("return value must be a symbol or list of symbols")
468+
## is it redundent ? new_fortran_subroutine also errors ?
469+
stop("return value must be a symbol or list of symbols")
450470
}
451471

452472

Lines changed: 50 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,81 @@
11
test_that("multiple return values", {
22
fn <- function(x) {
3-
declare(
4-
type(x = double(n)),
5-
type(y = double(n)),
6-
type(z = double(n))
7-
)
3+
declare(type(x = integer(n)))
84
y <- x + 1
95
z <- x + 2
106
list(y = y, z = z)
117
}
128
qfn <- quick(fn)
13-
x <- as.double(1:3)
9+
x <- 1:3
1410
expect_equal(qfn(x), fn(x))
1511
})
1612

1713
test_that("multiple return values via assignment", {
1814
fn <- function(x) {
19-
declare(
20-
type(x = double(n)),
21-
type(y = double(n)),
22-
type(z = double(n))
23-
)
15+
declare(type(x = integer(n)))
2416
y <- x + 1
2517
z <- x + 2
2618
out <- list(y = y, z = z)
2719
out
2820
}
2921
qfn <- quick(fn)
30-
x <- as.double(1:3)
22+
x <- 1:3
3123
expect_equal(qfn(x), fn(x))
3224
})
3325

3426
test_that("single return variable still works", {
3527
fn <- function(x) {
36-
declare(
37-
type(x = double(n)),
38-
type(y = double(n))
39-
)
28+
declare(type(x = integer(n)))
4029
y <- x + 1
4130
y
4231
}
4332
qfn <- quick(fn)
44-
x <- as.double(1:3)
33+
x <- 1:3
4534
expect_equal(qfn(x), fn(x))
4635
})
4736

48-
test_that("Errors if list is used outside return pattern [r2f error]", {
37+
test_that("custom names for multiple return values are preserved", {
38+
fn <- function(x) {
39+
declare(type(x = integer(n)))
40+
y <- x + 1L
41+
z <- x + 2L
42+
list(abc = y, def = z)
43+
}
44+
qfn <- quick(fn)
45+
x <- 1:3
46+
expect_equal(qfn(x), fn(x))
47+
})
48+
49+
test_that("no names add to unnamed elements", {
50+
fn <- function(x) {
51+
declare(type(x = integer(n)))
52+
y <- x + 1L
53+
z <- x + 2L
54+
list(y, z)
55+
}
56+
qfn <- quick(fn)
57+
x <- 1:3
58+
expect_equal(qfn(x), fn(x))
59+
})
60+
test_that("mixed named and symbols list work", {
61+
fn <- function(x) {
62+
declare(type(x = integer(n)))
63+
y <- x + 1L
64+
z <- x + 2L
65+
list(abc = y, z)
66+
}
67+
qfn <- quick(fn)
68+
x <- 1:3
69+
expect_equal(qfn(x), fn(x))
70+
})
71+
72+
73+
74+
75+
test_that("errors if list is used outside return pattern [r2f error]", {
4976
## list not last or second to last
5077
fn <- function(x) {
51-
declare(
52-
type(x = double(n)),
53-
type(y = double(n)),
54-
type(z = double(n))
55-
)
78+
declare(type(x = integer(n)))
5679
y <- x + 1
5780
z <- x + 2
5881
out <- list(y = y, z = z)
@@ -63,9 +86,9 @@ test_that("Errors if list is used outside return pattern [r2f error]", {
6386
## List is being accessed
6487
fn <- function(x) {
6588
declare(
66-
type(x = double(n)),
67-
type(y = double(n)),
68-
type(z = double(n))
89+
type(x = integer(n)),
90+
type(y = integer(n)),
91+
type(z = integer(n))
6992
)
7093
y <- x + 1
7194
z <- x + 2
@@ -74,4 +97,4 @@ test_that("Errors if list is used outside return pattern [r2f error]", {
7497
}
7598
expect_error(quick(fn), "Unsupported function: list")
7699

77-
})
100+
})

0 commit comments

Comments
 (0)