Skip to content

Commit 1313b25

Browse files
committed
Materialize fill constructors before matrix(): reshape() needs an array SOURCE
matrix() was on the fill-constructor pass-through whitelist, so matrix(numeric(6), 3, 2) reached the reshape() lowering as the scalar literal 0.0_c_double claiming array dims. That only compiled because hoist_unless_name() used to materialize every non-name; when it learned to skip literals (earlier on this branch), the emitted reshape(0.0_c_double, ...) became a gfortran error -- and matrix(logical(k), ...) kept working only because the literal regex does not match .false.. Drop matrix from the whitelist so fills materialize into a hoisted array temporary like any other array consumer, restoring the pre-cleanup code shape deliberately instead of by regex accident. Found by codex review (fable-final round); regression relative to main.
1 parent 091dc56 commit 1313b25

2 files changed

Lines changed: 37 additions & 6 deletions

File tree

R/r2f-constructors.R

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,19 +165,20 @@ r2f_handlers[["rep.int"]] <- function(args, scope, ..., hoist = NULL) {
165165

166166
# Compile a zero-fill constructor call: a single scalar literal carrying
167167
# array dims. Whole-array assignment broadcasts that correctly, and
168-
# c()/array()/matrix() spread or pad it explicitly, so those contexts keep
169-
# the scalar form. Any other consumer (elementwise ops, reductions, ...)
170-
# needs a real array expression -- an expression like `numeric(2) + 1`
171-
# would otherwise contribute one element where its dims claim two -- so
172-
# materialize the fill into a hoisted temporary there.
168+
# c()/array() spread it as an implied-do, so those contexts keep the
169+
# scalar form. Any other consumer (elementwise ops, reductions,
170+
# matrix() -- whose reshape() lowering needs an array SOURCE, not a
171+
# scalar literal) needs a real array expression -- an expression like
172+
# `numeric(2) + 1` would otherwise contribute one element where its dims
173+
# claim two -- so materialize the fill into a hoisted temporary there.
173174
fill_constructor_value <- function(literal, mode, args, scope, ..., hoist) {
174175
var <- Variable(mode = mode, dims = r2dims(args, scope))
175176
out <- Fortran(literal, var)
176177
if (passes_as_scalar(var)) {
177178
return(out)
178179
}
179180
parent_call <- parent_call_name(list(...)$calls)
180-
if (parent_call %in% c("<-", "=", "<<-", "c", "array", "matrix")) {
181+
if (parent_call %in% c("<-", "=", "<<-", "c", "array")) {
181182
return(out)
182183
}
183184
materialize_via_hoist(literal, mode, var@dims, hoist, "fill constructor")

tests/testthat/test-recycling.R

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,36 @@ test_that("fill constructors materialize where an array is required", {
236236
expect_quick_identical(symbolic, list(c(5, 6), 3L))
237237
})
238238

239+
test_that("fill constructors materialize inside matrix()", {
240+
# matrix() lowers non-scalar data through reshape(), whose SOURCE must
241+
# be an array. Fills used to pass through as scalar literals with
242+
# claimed dims and relied on hoist_unless_name() to materialize them;
243+
# once that helper learned to skip literals, the generated
244+
# reshape(0.0_c_double, ...) failed to compile (and logical(k) only
245+
# kept working because the literal regex missed `.false.`). Fills now
246+
# materialize before matrix() like any other array consumer.
247+
numeric_fill <- function() {
248+
matrix(numeric(6), 3, 2)
249+
}
250+
expect_quick_identical(numeric_fill, list())
251+
252+
integer_fill <- function() {
253+
matrix(integer(6), 3, 2)
254+
}
255+
expect_quick_identical(integer_fill, list())
256+
257+
logical_fill <- function() {
258+
matrix(logical(6), 3, 2)
259+
}
260+
expect_quick_identical(logical_fill, list())
261+
262+
assigned <- function() {
263+
x <- matrix(numeric(6), 3, 2)
264+
x
265+
}
266+
expect_quick_identical(assigned, list())
267+
})
268+
239269
test_that("matrix(scalar, m, n) materializes where an array is required", {
240270
reduced <- function() {
241271
sum(matrix(2, 2, 3))

0 commit comments

Comments
 (0)