Skip to content

Commit 0f86280

Browse files
committed
Extract parent_call_name() and materialize_via_hoist() helpers
fill_constructor_value() and the matrix() scalar case duplicated both the parent-call sniff (positional indexing into the calls stack) and the declare_tmp/emit/Fortran materialize triplet. One copy of each now. Review finding (fable-final-review.md #5); no behavior change.
1 parent d55d41f commit 0f86280

1 file changed

Lines changed: 24 additions & 17 deletions

File tree

R/r2f-constructors.R

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,26 @@ is_fill_constructor_call <- function(e) {
1414
as.character(e[[1L]]) %in% c("logical", "integer", "double", "numeric")
1515
}
1616

17+
# Name of the call one frame above the current handler ("" at top level).
18+
# The materialization decisions below branch on it: a fill constructor or
19+
# matrix(scalar, ...) may stay a scalar only where the parent broadcasts,
20+
# spreads, or pads it.
21+
parent_call_name <- function(calls) {
22+
if (length(calls) >= 2L) calls[[length(calls) - 1L]] else ""
23+
}
24+
25+
# Materialize `code` into a hoisted temporary and return the temporary.
26+
# `what` names the construct for the internal-error message when no hoist
27+
# context is available.
28+
materialize_via_hoist <- function(code, mode, dims, hoist, what) {
29+
if (is.null(hoist)) {
30+
stop("internal error: ", what, " requires hoist context", call. = FALSE)
31+
}
32+
tmp <- hoist$declare_tmp(mode = mode, dims = dims)
33+
hoist$emit(glue("{tmp@name} = {code}"))
34+
Fortran(tmp@name, tmp)
35+
}
36+
1737
# --- Handlers ---
1838

1939
r2f_handlers[["c"]] <- function(args, scope = NULL, ...) {
@@ -156,17 +176,11 @@ fill_constructor_value <- function(literal, mode, args, scope, ..., hoist) {
156176
if (passes_as_scalar(var)) {
157177
return(out)
158178
}
159-
calls <- list(...)$calls
160-
parent_call <- if (length(calls) >= 2L) calls[[length(calls) - 1L]] else ""
179+
parent_call <- parent_call_name(list(...)$calls)
161180
if (parent_call %in% c("<-", "=", "<<-", "c", "array", "matrix")) {
162181
return(out)
163182
}
164-
if (is.null(hoist)) {
165-
stop("internal error: fill constructor requires hoist context", call. = FALSE)
166-
}
167-
tmp <- hoist$declare_tmp(mode = mode, dims = var@dims)
168-
hoist$emit(glue("{tmp@name} = {literal}"))
169-
Fortran(tmp@name, tmp)
183+
materialize_via_hoist(literal, mode, var@dims, hoist, "fill constructor")
170184
}
171185

172186
register_r2f_handler(
@@ -228,18 +242,11 @@ r2f_handlers[["matrix"]] <- function(args, scope = NULL, ..., hoist = NULL) {
228242
# it as-is there; in any other context (sum(...), %*%, ...) the expression
229243
# must be a real rank-2 array, so materialize it into a hoisted temporary.
230244
if (passes_as_scalar(src@value)) {
231-
calls <- list(...)$calls
232-
parent_call <- if (length(calls) >= 2L) calls[[length(calls) - 1L]] else ""
233-
if (parent_call %in% c("<-", "=", "<<-")) {
245+
if (parent_call_name(list(...)$calls) %in% c("<-", "=", "<<-")) {
234246
src@value <- out_val
235247
return(src)
236248
}
237-
if (is.null(hoist)) {
238-
stop("internal error: matrix() requires hoist context", call. = FALSE)
239-
}
240-
tmp <- hoist$declare_tmp(mode = src@value@mode, dims = dims)
241-
hoist$emit(glue("{tmp@name} = {src}"))
242-
return(Fortran(tmp@name, tmp))
249+
return(materialize_via_hoist(src, src@value@mode, dims, hoist, "matrix()"))
243250
}
244251

245252
rows <- dims[[1L]]

0 commit comments

Comments
 (0)