Skip to content

Commit d70138f

Browse files
committed
Fix array() dim=1 scalarization for non-scalar data
- Add regression for array(<non-scalar>, dim=1) which previously produced rank-mismatched RESHAPE() assignments. - In array() lowering, special-case scalar-like target dims (dim=1) to extract the first element via a hoisted temp array, avoiding RESHAPE() returning rank-1.
1 parent 1b63a44 commit d70138f

2 files changed

Lines changed: 74 additions & 44 deletions

File tree

R/r2f-constructors.R

Lines changed: 61 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ r2f_handlers[["matrix"]] <- function(args, scope = NULL, ...) {
7575
# TODO: reshape() if !passes_as_scalar(out)
7676
}
7777

78-
r2f_handlers[["array"]] <- function(args, scope = NULL, ...) {
78+
r2f_handlers[["array"]] <- function(args, scope = NULL, ..., hoist = NULL) {
7979
args$data %||% stop("array(data=) must be provided, cannot be NA")
8080
if (is.null(args$dim)) {
8181
stop("array(dim=) must be provided, cannot be NA")
@@ -155,53 +155,70 @@ r2f_handlers[["array"]] <- function(args, scope = NULL, ...) {
155155
# R semantics: `array()` flattens its input (dropping dim) then reshapes.
156156
# We implement this as `reshape()`; recycling is not supported here.
157157
dims_f <- dims2f(target_dims, scope)
158-
if (!nzchar(dims_f)) {
159-
dims_f <- "1"
160-
}
161-
if (grepl(":", dims_f, fixed = TRUE)) {
162-
stop("array(dim=) must be known", call. = FALSE)
163-
}
164-
shape <- glue("int([{dims_f}])")
165-
166-
data_r <- args$data
167-
is_fill_constructor <-
168-
is.call(data_r) &&
169-
is.symbol(data_r[[1L]]) &&
170-
as.character(data_r[[1L]]) %in%
171-
c(
172-
"logical",
173-
"integer",
174-
"double",
175-
"numeric"
176-
)
177-
178-
source <- if (is_fill_constructor) {
179-
axis_terms <- vapply(
180-
target_dims,
181-
function(d) {
182-
axis <- dims2f(list(d), scope)
183-
if (!nzchar(axis)) {
184-
"1"
185-
} else {
186-
axis
187-
}
188-
},
189-
character(1L)
158+
scalar_target <- !nzchar(dims_f) && length(target_dims) == 1L
159+
if (scalar_target) {
160+
# `dim = 1` is scalar-like in quickr (rank-1 length-1 is declared scalar).
161+
# Avoid `reshape(..., [1])` (rank-1) and instead return the first element.
162+
if (is.null(hoist)) {
163+
stop("internal error: array() requires hoist context", call. = FALSE)
164+
}
165+
target_dims <- list(1L)
166+
tmp <- hoist$declare_tmp(mode = out@value@mode, dims = out@value@dims)
167+
hoist$emit(glue("{tmp@name} = {out}"))
168+
idxs <- rep("1", out@value@rank)
169+
out <- Fortran(
170+
glue("{tmp@name}({str_flatten_commas(idxs)})"),
171+
Variable(mode = out@value@mode, dims = list(1L))
190172
)
191-
n_expr <- if (length(axis_terms) == 1L) {
192-
axis_terms[[1L]]
173+
} else {
174+
if (!nzchar(dims_f)) {
175+
dims_f <- "1"
176+
}
177+
if (grepl(":", dims_f, fixed = TRUE)) {
178+
stop("array(dim=) must be known", call. = FALSE)
179+
}
180+
shape <- glue("int([{dims_f}])")
181+
182+
data_r <- args$data
183+
is_fill_constructor <-
184+
is.call(data_r) &&
185+
is.symbol(data_r[[1L]]) &&
186+
as.character(data_r[[1L]]) %in%
187+
c(
188+
"logical",
189+
"integer",
190+
"double",
191+
"numeric"
192+
)
193+
194+
source <- if (is_fill_constructor) {
195+
axis_terms <- vapply(
196+
target_dims,
197+
function(d) {
198+
axis <- dims2f(list(d), scope)
199+
if (!nzchar(axis)) {
200+
"1"
201+
} else {
202+
axis
203+
}
204+
},
205+
character(1L)
206+
)
207+
n_expr <- if (length(axis_terms) == 1L) {
208+
axis_terms[[1L]]
209+
} else {
210+
paste0("(", paste0("(", axis_terms, ")", collapse = " * "), ")")
211+
}
212+
i <- scope@get_unique_var("integer")
213+
glue("[({out}, {i}=1, int({n_expr}))]")
193214
} else {
194-
paste0("(", paste0("(", axis_terms, ")", collapse = " * "), ")")
215+
# RESHAPE() requires `SOURCE` to be an array expression; array constructors
216+
# flatten array-valued expressions (which matches R's array() semantics).
217+
glue("[{out}]")
195218
}
196-
i <- scope@get_unique_var("integer")
197-
glue("[({out}, {i}=1, int({n_expr}))]")
198-
} else {
199-
# RESHAPE() requires `SOURCE` to be an array expression; array constructors
200-
# flatten array-valued expressions (which matches R's array() semantics).
201-
glue("[{out}]")
202-
}
203219

204-
out <- Fortran(glue("reshape({source}, {shape})"), out@value)
220+
out <- Fortran(glue("reshape({source}, {shape})"), out@value)
221+
}
205222
}
206223

207224
out@value <- Variable(

tests/testthat/test-array-reshape.R

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,3 +94,16 @@ test_that("array() fill reshape handles dim expressions that lower to comma-cont
9494
x <- y
9595
expect_quick_identical(fn, list(y, x))
9696
})
97+
98+
test_that("array() reshape supports dim = 1 for non-scalar data", {
99+
fn <- function(x) {
100+
declare(type(x = integer(2L, 3L, 4L)))
101+
# Rank-1 length-1 arrays are scalar-like in quickr; index the first element
102+
# to compare against base R without relying on `dim` attributes.
103+
array(as.double(x), dim = 1L)[1]
104+
}
105+
106+
set.seed(1)
107+
x <- array(sample(1:10, 24, replace = TRUE), dim = c(2L, 3L, 4L))
108+
expect_quick_identical(fn, list(x))
109+
})

0 commit comments

Comments
 (0)