Skip to content

Commit 42dd856

Browse files
committed
Route the scalar-matrix broadcast guards through guard_conformable_dims()
The broadcast fast path hand-rolled the check + guard emission because guard_dim_f() could only spell a guard side as a literal or as size(operand), and the fill has no array to size(). guard_dim_f() now accepts a caller-provided spelling, so the one conformability policy has no bypass, and the elementwise matrix message is a single shared constant (the runtime-guard text must match the compile-error text). Review finding (fable-final-review.md #4); emitted code is unchanged.
1 parent 2e17727 commit 42dd856

2 files changed

Lines changed: 37 additions & 26 deletions

File tree

R/r2f-operators-helpers.R

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -202,12 +202,22 @@ check_elementwise_lengths <- function(left, right) {
202202
list(ok = TRUE, unknown = TRUE)
203203
}
204204

205-
# Render one side of a dim-comparison guard: a literal dim as the literal,
206-
# anything else as the operand's actual extent (whole size when `axis` is
207-
# NULL). size() is an inquiry, so applying it to operand expression text
208-
# does not evaluate the operand.
205+
# The message shared by every enforcement point of the elementwise matrix
206+
# shape contract: the runtime-guard text must match the compile-error text.
207+
elementwise_matrix_msg <-
208+
"elementwise matrix operations require matching dimensions"
209+
210+
# Render one side of a dim-comparison guard: a caller-provided spelling
211+
# (`f`, for operands with no array to size(), e.g. a scalar fill's claimed
212+
# dims) wins; then a literal dim as the literal; anything else as the
213+
# operand's actual extent (whole size when `axis` is NULL). size() is an
214+
# inquiry, so applying it to operand expression text does not evaluate the
215+
# operand.
209216
# Used by: guard_conformable_dims()
210-
guard_dim_f <- function(dim, operand, axis = NULL) {
217+
guard_dim_f <- function(dim, operand, axis = NULL, f = NULL) {
218+
if (!is.null(f)) {
219+
return(f)
220+
}
211221
if (is_wholenumber(dim)) {
212222
return(as.character(as.integer(dim)))
213223
}
@@ -224,7 +234,10 @@ guard_dim_f <- function(dim, operand, axis = NULL) {
224234
# runtime guard emitted before the consuming statement; provably equal
225235
# dims need nothing. Never warn-and-proceed. `axis` NULL compares the
226236
# operand's whole size (rank-1 operands).
227-
# Used by: resolve_elementwise(), r2f-conditionals.R, r2f-matrix*.R
237+
# Used by: resolve_elementwise(), compile_binop_operands(),
238+
# r2f-conditionals.R, r2f-matrix*.R. `left_f`/`right_f` override that
239+
# side's guard spelling (see guard_dim_f()); its `left`/`right` operand is
240+
# then unused and may be NULL.
228241
guard_conformable_dims <- function(
229242
left_dim,
230243
right_dim,
@@ -234,7 +247,9 @@ guard_conformable_dims <- function(
234247
left,
235248
right,
236249
left_axis = NULL,
237-
right_axis = NULL
250+
right_axis = NULL,
251+
left_f = NULL,
252+
right_f = NULL
238253
) {
239254
stopifnot(is_string(message))
240255
conform <- check_elementwise_lengths(left_dim, right_dim)
@@ -251,7 +266,7 @@ guard_conformable_dims <- function(
251266
}
252267
emit_quickr_error_if(
253268
glue(
254-
"{guard_dim_f(left_dim, left, left_axis)} /= {guard_dim_f(right_dim, right, right_axis)}"
269+
"{guard_dim_f(left_dim, left, left_axis, left_f)} /= {guard_dim_f(right_dim, right, right_axis, right_f)}"
255270
),
256271
message,
257272
hoist,
@@ -386,14 +401,13 @@ resolve_elementwise <- function(
386401
}
387402

388403
if (left_rank == 2L && right_rank == 2L) {
389-
matrix_msg <- "elementwise matrix operations require matching dimensions"
390404
left_dims <- matrix_dims(left)
391405
right_dims <- matrix_dims(right)
392406
for (axis in 1:2) {
393407
guard_conformable_dims(
394408
if (axis == 1L) left_dims$rows else left_dims$cols,
395409
if (axis == 1L) right_dims$rows else right_dims$cols,
396-
matrix_msg,
410+
elementwise_matrix_msg,
397411
hoist,
398412
scope,
399413
left = left,

R/r2f-operators.R

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -177,22 +177,19 @@ compile_binop_operands <- function(args, scope, ..., hoist = NULL) {
177177
if (broadcastable) {
178178
other_dims <- matrix_dims(other)
179179
for (axis in 1:2) {
180-
other_dim <- if (axis == 1L) other_dims$rows else other_dims$cols
181-
verdict <- check_elementwise_lengths(fill_dims[[axis]], other_dim)
182-
if (!verdict$ok) {
183-
stop(
184-
"elementwise matrix operations require matching dimensions",
185-
call. = FALSE
186-
)
187-
}
188-
if (verdict$unknown) {
189-
emit_quickr_error_if(
190-
glue("({fill_dims_f[[axis]]}) /= size({other}, {axis})"),
191-
"elementwise matrix operations require matching dimensions",
192-
hoist,
193-
scope
194-
)
195-
}
180+
# The fill has no array to size(), so its side of a runtime guard
181+
# is spelled from the claimed dim expression via `left_f`.
182+
guard_conformable_dims(
183+
fill_dims[[axis]],
184+
if (axis == 1L) other_dims$rows else other_dims$cols,
185+
elementwise_matrix_msg,
186+
hoist,
187+
scope,
188+
left = NULL,
189+
right = other,
190+
right_axis = axis,
191+
left_f = glue("({fill_dims_f[[axis]]})")
192+
)
196193
}
197194
fill <- r2f(fills[[j]]$data, scope, ..., hoist = hoist)
198195
out <- list(fill, other)

0 commit comments

Comments
 (0)