Skip to content

Commit 87b3092

Browse files
mns-nordicalst-kalinowski
authored andcommitted
Extract check_subscript_exprs() shared by read and write subscripts
The read-side `[` handler and the write-side compile_subset_designator() carried the same six-line validation loop; the invariant that both sides validate identically is now pinned by a single helper instead of a comment asking to keep two copies in sync. Review finding (fable-final-review.md #3); no behavior change.
1 parent f33189d commit 87b3092

2 files changed

Lines changed: 17 additions & 14 deletions

File tree

R/r2f-closures.R

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1392,12 +1392,7 @@ compile_subset_designator <- function(
13921392
# Same validation as the read-side `[` handler: assignment subscripts
13931393
# would otherwise lower R's exclusion/zero/out-of-range subscripts into
13941394
# silent out-of-bounds Fortran writes.
1395-
extents <- subscript_axis_extents(base_var, length(idx_args))
1396-
for (i in seq_along(idx_args)) {
1397-
if (!is_missing(idx_args[[i]])) {
1398-
check_subscript_expr(idx_args[[i]], extent = extents[[i]])
1399-
}
1400-
}
1395+
check_subscript_exprs(base_var, idx_args)
14011396

14021397
idxs <- whole_doubles_to_ints(idx_args)
14031398
idxs <- imap(idxs, function(idx, i) {

R/r2f-subscript.R

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,7 @@ r2f_handlers[["["]] <- function(
2323
drop <- idx_args$drop %||% TRUE
2424
idx_args$drop <- NULL
2525

26-
extents <- subscript_axis_extents(var@value, length(idx_args))
27-
for (i in seq_along(idx_args)) {
28-
if (!is_missing(idx_args[[i]])) {
29-
check_subscript_expr(idx_args[[i]], extent = extents[[i]])
30-
}
31-
}
26+
check_subscript_exprs(var@value, idx_args)
3227

3328
idxs <- whole_doubles_to_ints(idx_args)
3429
idxs <- imap(idxs, function(idx, i) {
@@ -306,11 +301,24 @@ check_subscript_expr <- function(e, extent = NULL) {
306301
invisible(NULL)
307302
}
308303

304+
# Validate every non-missing subscript in `idx_args` against `base_var`'s
305+
# statically known extents. The single entry point for both the read side
306+
# (the `[` handler) and the write side (compile_subset_designator() in
307+
# r2f-closures.R), so read and write subscripts validate identically.
308+
check_subscript_exprs <- function(base_var, idx_args) {
309+
extents <- subscript_axis_extents(base_var, length(idx_args))
310+
for (i in seq_along(idx_args)) {
311+
if (!is_missing(idx_args[[i]])) {
312+
check_subscript_expr(idx_args[[i]], extent = extents[[i]])
313+
}
314+
}
315+
invisible(NULL)
316+
}
317+
309318
# Statically-known extent per subscript axis; NULL where symbolic/unknown.
310319
# A single subscript on a rank>1 base is R's linear indexing -- its extent
311320
# is the product of the dims when all of them are known.
312-
# Used by: the `[` handler (above) and compile_subset_designator()
313-
# (r2f-closures.R), so read and write subscripts validate identically.
321+
# Used by: check_subscript_exprs()
314322
subscript_axis_extents <- function(var, n_idx) {
315323
dims <- var@dims
316324
if (n_idx == length(dims)) {

0 commit comments

Comments
 (0)