[5/15] Validate subscripts and make in-loop errors cancel OpenMP loops

[mns-nordicals]

Stacked PR 5 of 15 — branch conformability/05-subscript-validation,
cut from PR 4 (#140), branch conformability/04-ifelse-t-diag.
GitHub can only diff a fork branch against main, so until PR 4 merges
this page also shows PRs 1–4's commits; the 5 commits new to this
PR are Move emit_quickr_error_if() to error-handling.R … Extract check_subscript_exprs() shared by read and write subscripts.

Stack overview and suggested review order: #152.

Fixes #126.

What changed

Negative and zero subscripts are now compile-time errors. R's negative
subscript means exclusion and its zero subscript is silently dropped —
both produce result shapes that depend on the subscript's value, which
quickr's static-shape model cannot represent — and the generated Fortran
read out of bounds (x(-1), x(0)) and returned the wrong shape:

fn <- function(x) {
  declare(type(x = double(4)))
  x[-1L]
}
# before: silent out-of-bounds read, scalar result (R returns length 3)
# after:  Error: subscripts must be positive; R's negative (exclusion) and
#         zero subscripts are not supported: -1L

The syntactic form x[-i] is rejected too — unary minus on a subscript is
unambiguously exclusion in R, so the form is refused even when the value
is unknown (negative subscripts (exclusion) are not supported: -i). Same
for negative/zero elements of literal vectors (x[c(-1L, -2L)]) and
literal range bounds < 1 (x[0:2]).

Assignment subscripts get the same validation. x[-1L] <- 9 and
x[0L] <- 1 bypassed the read-side checks entirely and compiled into
silent out-of-bounds Fortran writes. compile_subset_designator() now
runs the same checks, covering [<-, [<<-, and closure host writes.

Literal subscripts are checked against statically-known extents.
x[4L] and x[2:4] on a declared double(3) compiled and read garbage
where R pads with NA (and grows the vector on writes) — neither
representable in quickr's static-shape model. When the base's extent
along an axis is a literal, out-of-range literal values, : endpoints,
and c() elements are now compile errors
(subscript exceeds its dimension's extent (3): 4L; R's out-of-range subscripts (NA padding, vector growing) are not supported). A single subscript
on a rank>1 base (R's linear indexing) checks against the product of the
dims. Symbolic subscripts and symbolic extents are untouched — symbolic
scalar subscripts stay trusted-in-bounds (see notes).

x[a:b] with runtime bounds gets a runtime guard. The emitted section
a:b:sign(1, b-a) and its claimed length abs(b-a)+1 are correct exactly
when both bounds are >= 1; x[1:n] with n = 0 used to read x(0) and
return 2 values where R returns 1. Now a single scalar check per statement
runs before any memory is touched:

if (n < 1_c_int) then
  call quickr_set_error_msg("index ranges in x[a:b] must have bounds >= 1")
  return
end if
out = x(1_c_int:n:sign(1, n-1_c_int))

Literal halves of the check are constant-folded away (x[2:4] emits no
guard), and descending ranges (x[n:1]) still work.

x[seq(a, b, by = step)]: a non-literal step is now a compile
error — the result length divides by the step and is evaluated in the
generated C bridge before any Fortran guard could run, so a zero step
was an unguardable division-by-zero crash. (It was also mis-sized:
seq_like_length_expr() silently dropped a non-literal by, so
x[seq(1L, 9L, by = k)] claimed length 9 for every k, returning garbage
past the real section; the same fix makes seq(1L, 9L, by = k) in value
position size correctly.) With a literal step and symbolic bounds, the
wrong-sign case gets a runtime guard raising "wrong sign in 'by'
argument", as R does.

x[seq_len(n)] / x[seq_along(y)] need no guard: their worst case is a
legal zero-length section, matching R's x[integer(0)] (covered by a new
test). For-loops are untouched — do i = 1, 0 already runs zero times.

Errors raised inside OpenMP loops now cancel the loop. Generated
error paths emit !$omp cancel do, but per the OpenMP spec cancel
constructs are no-ops unless OMP_CANCELLATION=true is set when the
OpenMP runtime first initializes — and nothing set it. The error
message was recorded correctly, but every remaining iteration still
ran, and statements after a failed in-loop check kept executing in that
iteration's thread. quickr now sets OMP_CANCELLATION=true in .onLoad
when unset (a pre-set value is respected). Caveats documented in
?declare: no effect if another package already initialized the OpenMP
runtime, so early exit stays best-effort — messages are always correct
either way.

How

• emit_quickr_error_if() moved from the BLAS translation file to
  R/error-handling.R (unchanged) so any handler can emit a statement-
  level runtime guard; this PR is the first non-BLAS consumer.
• R/r2f-subscript.R: new check_subscript_exprs() validates the raw R
  index expressions at the top of the [ handler, where literalness and
  unary minus are still visible (per index, check_subscript_expr()
  handles literal values, unary minus, : endpoints, and recurses into
  c()); subscript_axis_extents() supplies the per-axis literal
  extents, and compile_subset_designator() (R/r2f-closures.R) calls
  the same check_subscript_exprs() for assignments.
• R/r2f-iterables-helpers.R: new check_subscript_range_bounds()
  called from seq_like_r2f()'s subscript branch; plus the
  seq_like_length_expr() fix (the literal-bounds fast path now requires
  a literal by too).
• R/zzz.R, R/declare.R: the OMP_CANCELLATION set-if-unset and its
  documentation.

Tests

New test-subscript-validation.R: compile-time rejections for every
exclusion form; literal out-of-extent rejections for reads and writes
(scalar, range, c(), per-axis matrix, linear indexing, [<<-) with
in-range and symbolic cases still compiling; runtime guard firing for
x[1:n] (n = 0 and negative)
with the valid and descending cases still passing; no-guard translation
snapshot for literal bounds; the seq-step compile error and wrong-sign
runtime guard; zero-length x[seq_len(0)] matching R; symbolic-by
seq() sizing in value position. Translation snapshots pin the guard
text. New test-onload.R covers set-when-unset / respect-when-preset.

Snapshot churn: only example-roll_mean.md — its x[i:(i + n - 1)] now
carries the bounds guard inside the loop (plus the error-message plumbing
in the subroutine signature and C bridge).

Notes for review

• Behavior changes (candidates for NEWS): x[1:0]-style empty ranges now
  raise an R error instead of reading out of bounds (R drops the 0 and
  returns x[1] — a value-dependent shape quickr cannot produce);
  exclusion subscripts that previously "worked" by reading garbage now
  fail to compile; x[seq(a, b, by = k)] with non-literal k now fails
  to compile (it previously returned garbage past the real section);
  literal out-of-extent subscripts (x[4L] on double(3), reads and
  writes) now fail to compile instead of touching out-of-bounds memory.
• Scalar symbolic subscripts (x[k] where k might be <= 0 at run time)
  are deliberately not guarded per-element — they stay trusted to be
  positive and in bounds, as before this PR; a quickr.bounds_check
  option could add opt-in guards later.
• The guard inside roll_mean's loop is two scalar compares per
  iteration next to a windowed sum(); hoisting loop-invariant guards
  out of loops is a possible future optimization, not attempted here.

[codecov]

Codecov Report

❌ Patch coverage is 96.90265% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.37%. Comparing base (fafce1c) to head (37ea0a5).

Files with missing lines	Patch %	Lines
R/r2f-iterables-helpers.R	95.04%	5 Missing ⚠️
R/c-wrapper.R	96.66%	1 Missing ⚠️
R/r2f-subscript.R	98.70%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #141      +/-   ##
==========================================
+ Coverage   93.26%   93.37%   +0.11%     
==========================================
  Files          29       30       +1     
  Lines        6161     6370     +209     
==========================================
+ Hits         5746     5948     +202     
- Misses        415      422       +7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[t-kalinowski]

Thank you very much for this series of PRs! For the roll mean example, I wonder if we need the check every loop. I understand how it happens, but it seems like quite a performance tax to introduce for every loop iteration, especially since we have full information in advance.

[t-kalinowski]

Why do we need a bounds check here? Seems like the code is being overly defensive. For the generated code in quickr, I'd rather eer on the side of speed, even if it means users bear some responsibility to write correct (R) code.

[mns-nordicals]

I agree and the general intent with runtime guards is for them to be cheap one time checks. Later when it come to matrix operations I think the runtime guards will be more useful there.

Also, the implementation is half-baked, so I have removed it in the last commit.

[t-kalinowski]

The bigger risk would be n > size(x), no?

[mns-nordicals]

See reply to previous comment

[mns-nordicals]

The first fixes a pre-existing hoisting bug exposed by this PR’s new runtime guards. Unbraced for bodies inherited the enclosing statement’s hoist target, so loop-dependent guards or temporaries could be emitted before the loop and reference an uninitialized loop variable. The fix gives every loop body its own hoist target.

[mns-nordicals]

gfortran seem to have an compile option called -fcheck=bounds which probably does a better runtime check than what was tried here. So using that for extra safety is likely a better option.