Skip to content

Commit ee9ef1e

Browse files
committed
Take R's diag() identity form for any length-1 x
R's rule for diag(x) is length(x) == 1 with no nrow/ncol, not rank 0. quickr tested the rank, so a literal (diag(3L)) and a constant-folded local took the identity path while a declared integer(1) argument fell through to the vector constructor and produced a 1x1 matrix holding n instead of the n-by-n identity -- wrong in both shape and values. Both the handler and infer_dest_diag() now use passes_as_scalar(), so lowering and destination inference cannot disagree about which form a call takes. R derives the size with as.integer(x), which also makes diag(3.7) the 3x3 identity. Size expressions gain as.integer() so quickr can spell the same thing: INT() in Fortran (truncates toward zero, like R), a cast in the C bridge, and r2size() folds a literal instead of rejecting a non-whole double. The two renderers that spell a dim by deparsing -- bind_dim_string() and blas_int() -- route through one small rewriter so the R name cannot leak into emitted Fortran.
1 parent ac486d5 commit ee9ef1e

10 files changed

Lines changed: 210 additions & 6 deletions

File tree

NEWS.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,18 @@
2626
symbol in both cases. Locals declared with unknown (`NA`) dims still
2727
reallocate on assignment, like R.
2828

29+
- `diag(x)` with a length-1 `x` and no `nrow`/`ncol` now builds the
30+
identity matrix of size `x`, matching R. Previously only a *literal*
31+
size took that path, so `diag(n)` with `n` declared `integer(1)`
32+
returned a 1x1 matrix containing `n` instead of the n-by-n identity —
33+
a silent divergence in both shape and values. As in R the size is
34+
`as.integer(x)`, so a `double` or `logical` `x` works and truncates
35+
toward zero (`diag(3.7)` is the 3x3 identity, as in R). Use
36+
`diag(x, nrow)` for a 1x1 matrix holding `x`.
37+
38+
- `declare()` size expressions now accept `as.integer()`, which lowers to
39+
Fortran's `INT()` and to an integer cast in the generated C bridge.
40+
2941
- Elementwise operations (arithmetic, comparisons, `&`, `|`) now require
3042
operand lengths to match, unless one operand is a scalar or a vector is
3143
combined column-wise with a matrix whose rows it spans. R-style partial

R/c-wrapper.R

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,16 @@ dims2c_expr <- function(e, scope, c_hoist = NULL) {
590590
return(glue("(({e1}) < 0 ? -({e1}) : ({e1}))"))
591591
}
592592

593+
if (identical(op, "as.integer")) {
594+
if (length(args) != 1L) {
595+
stop("as.integer() expects one argument")
596+
}
597+
# a C cast to an integer type truncates toward zero, as R's
598+
# as.integer() does
599+
e1 <- dims2c_expr(args[[1L]], scope, c_hoist = c_hoist)
600+
return(glue("((R_xlen_t)({e1}))"))
601+
}
602+
593603
if (op %in% c("+", "-", "*", "/", "%/%", "%%", "^")) {
594604
if (length(args) == 1L && op %in% c("+", "-")) {
595605
e1 <- dims2c_expr(args[[1L]], scope, c_hoist = c_hoist)

R/manifest.R

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,8 @@ dims2f_eval_base_env[["%%"]] <- function(e1, e2) {
511511
}
512512
dims2f_eval_base_env[["^"]] <- function(e1, e2) glue("({e1})**({e2})")
513513
dims2f_eval_base_env[["abs"]] <- function(x) glue("abs({x})")
514+
# Fortran INT() truncates toward zero, like as.integer() in R.
515+
dims2f_eval_base_env[["as.integer"]] <- function(x) glue("int({x})")
514516
dims2f_eval_base_env[["length"]] <- function(x) {
515517
if (is.symbol(x)) {
516518
glue("size({as.character(x)})")

R/r2f-matrix-blas.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ ensure_blas_operand_name <- function(x, hoist) {
338338
# Wrap an expression as a BLAS int literal.
339339
blas_int <- function(x) {
340340
x_str <- if (is.language(x)) {
341-
gsub("([0-9]+)L\\b", "\\1", deparse1(x))
341+
gsub("([0-9]+)L\\b", "\\1", deparse1(fortranize_size_calls(x)))
342342
} else if (is_wholenumber(x)) {
343343
as.character(as.integer(x))
344344
} else {

R/r2f-matrix-infer.R

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -321,8 +321,11 @@ infer_dest_diag <- function(args, scope) {
321321
}
322322
}
323323

324-
# Case: x is a scalar symbol without nrow/ncol (identity matrix)
325-
if (!is.null(x) && x@rank == 0L && !has_nrow && !has_ncol) {
324+
# Case: x is a length-1 symbol without nrow/ncol (identity matrix). Must
325+
# match the handler's predicate exactly, or the inferred destination
326+
# would be the 1x1 constructor result instead of the identity's (x, x).
327+
# The size depends on x's value, so leave the destination uninferred.
328+
if (!is.null(x) && passes_as_scalar(x) && !has_nrow && !has_ncol) {
326329
return(NULL)
327330
}
328331

R/r2f-matrix.R

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ bind_dim_string <- function(dim) {
312312
} else if (is.numeric(dim)) {
313313
as.character(dim)
314314
} else {
315-
gsub("([0-9]+)L\\b", "\\1", deparse1(dim))
315+
gsub("([0-9]+)L\\b", "\\1", deparse1(fortranize_size_calls(dim)))
316316
}
317317
}
318318

@@ -710,8 +710,28 @@ register_r2f_handler(
710710
"diag() only supports scalar, vector, or matrix inputs"
711711
)
712712

713-
if (!has_nrow && !has_ncol && x_rank == 0L) {
714-
nrow <- r2size(x_arg, scope)
713+
# R's identity form is `length(x) == 1L` with no nrow/ncol -- it does
714+
# not require a rank-0 value, so a declared `integer(1)` argument or a
715+
# length-1 vector takes it too (R: diag(c(3)) is the 3x3 identity).
716+
# The size comes from x's *value*, and the result is always double.
717+
if (!has_nrow && !has_ncol && passes_as_scalar(x@value)) {
718+
# R sizes the identity with as.integer(x), so a double or logical `x`
719+
# is fine and truncates toward zero. Coerce in the size expression
720+
# rather than requiring an integer, so diag(n) works whatever the
721+
# caller declared. An integer `x` needs no wrapper.
722+
size_arg <- if (identical(x@value@mode, "integer")) {
723+
x_arg
724+
} else if (x@value@mode %in% c("double", "logical")) {
725+
call("as.integer", x_arg)
726+
} else {
727+
stop(
728+
"diag(x) with a length-1 `x` builds an identity matrix of size ",
729+
"`x`, which requires a numeric `x`; got ",
730+
x@value@mode,
731+
call. = FALSE
732+
)
733+
}
734+
nrow <- r2size(size_arg, scope)
715735
ncol <- nrow
716736
x_val <- Fortran("1.0_c_double", Variable("double"))
717737
return(diag_matrix(

R/r2f-operators-helpers.R

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,28 @@ reshape_vector_for_matrix <- function(vec, rows, cols) {
391391
# -1 where truncation differs from floor (negative non-integers). `x` is
392392
# spliced three times, so callers hoist non-trivial expressions first.
393393
# Used by: r2f-math.R (floor), r2f-arithmetic.R (double %/%)
394+
# Size expressions may carry an as.integer() coercion (diag()'s identity
395+
# form sizes the result with as.integer(x), as R does). The two renderers
396+
# that spell a dim by deparsing -- bind_dim_string() and blas_int() -- would
397+
# emit the R name verbatim, so map it to Fortran's INT(), which truncates
398+
# toward zero the same way. dims2f()/dims2c() translate the call properly
399+
# and do not need this.
400+
# Used by: bind_dim_string() (r2f-matrix.R), blas_int() (r2f-matrix-blas.R)
401+
fortranize_size_calls <- function(e) {
402+
if (!is.call(e)) {
403+
return(e)
404+
}
405+
if (identical(e[[1L]], quote(as.integer))) {
406+
e[[1L]] <- quote(int)
407+
}
408+
for (i in seq_along(e)[-1L]) {
409+
if (!is_missing(e[[i]])) {
410+
e[[i]] <- fortranize_size_calls(e[[i]])
411+
}
412+
}
413+
e
414+
}
415+
394416
real_floor_expr <- function(x) {
395417
aint <- glue("aint({x})")
396418
glue("({aint} - merge(1.0_c_double, 0.0_c_double, ({x} < {aint})))")

R/sizes.R

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,41 @@ r2size <- function(r, scope) {
228228

229229
switch(
230230
op,
231+
as.integer = {
232+
if (length(r) != 2L) {
233+
stop("as.integer() in a size expression expects one argument")
234+
}
235+
# A numeric literal is coerced here rather than recursed into:
236+
# r2size() rejects a non-whole double, which is exactly the
237+
# case as.integer() exists to handle.
238+
if (is.numeric(r[[2L]]) && length(r[[2L]]) == 1L) {
239+
return(as.integer(r[[2L]]))
240+
}
241+
# An explicit coercion is exactly what the "not an integer"
242+
# warning asks for, so don't also warn about the operand.
243+
inner <- withCallingHandlers(
244+
r2size(r[[2L]], scope),
245+
warning = function(w) {
246+
if (
247+
grepl(
248+
"size is not an integer",
249+
conditionMessage(w),
250+
fixed = TRUE
251+
)
252+
) {
253+
invokeRestart("muffleWarning")
254+
}
255+
}
256+
)
257+
if (is.atomic(inner) && length(inner) == 1L) {
258+
if (is.na(inner)) {
259+
return(NA_integer_)
260+
}
261+
# truncates toward zero, as as.integer() does in R
262+
return(as.integer(inner))
263+
}
264+
call("as.integer", inner)
265+
},
231266
length = {
232267
var <- get0(as.character(r[[2L]]), scope)
233268
if (!inherits(var, Variable)) {

tests/testthat/test-matrix.R

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,88 @@ test_that("t() and diag() preserve integer mode", {
348348
expect_quick_equal(dident, list())
349349
})
350350

351+
test_that("diag() takes R's identity form for any length-1 x", {
352+
# R's rule is length(x) == 1 with no nrow/ncol, not rank 0: a declared
353+
# integer(1) argument is the n x n identity, not a 1x1 matrix holding n
354+
dsym <- function(n) {
355+
declare(type(n = integer(1)))
356+
diag(n)
357+
}
358+
expect_quick_equal(dsym, list(3L), list(1L))
359+
360+
# same through the inferred-destination path
361+
ddest <- function(n) {
362+
declare(type(n = integer(1)))
363+
out <- diag(n)
364+
out
365+
}
366+
expect_quick_equal(ddest, list(3L))
367+
368+
# a size expression works too
369+
dexpr <- function(n) {
370+
declare(type(n = integer(1)))
371+
diag(n + 1L)
372+
}
373+
expect_quick_equal(dexpr, list(2L))
374+
375+
# a length-1 vector is still length 1
376+
dvec1 <- function(v) {
377+
declare(type(v = integer(1)))
378+
diag(v)
379+
}
380+
expect_quick_equal(dvec1, list(3L))
381+
382+
# nrow/ncol switch off the identity form, as R's nargs() rule does
383+
d1x1 <- function(v) {
384+
declare(type(v = double(1)))
385+
diag(v, 1L)
386+
}
387+
expect_quick_equal(d1x1, list(3))
388+
389+
# longer vectors keep building a diagonal matrix
390+
dlong <- function(v) {
391+
declare(type(v = double(3)))
392+
diag(v)
393+
}
394+
expect_quick_equal(dlong, list(c(1, 2, 3)))
395+
396+
# R sizes the identity with as.integer(x), so a double or logical x works
397+
# and truncates toward zero
398+
ddbl <- function(x) {
399+
declare(type(x = double(1)))
400+
diag(x)
401+
}
402+
expect_quick_equal(ddbl, list(3), list(3.7), list(1))
403+
404+
ddbl_dest <- function(x) {
405+
declare(type(x = double(1)))
406+
out <- diag(x)
407+
out
408+
}
409+
expect_quick_equal(ddbl_dest, list(3))
410+
411+
dlgl <- function(b) {
412+
declare(type(b = logical(1)))
413+
diag(b)
414+
}
415+
expect_quick_equal(dlgl, list(TRUE))
416+
417+
# a non-whole literal truncates too, rather than tripping the
418+
# "size must be an integer" check
419+
dfrac <- function() {
420+
out <- diag(3.7)
421+
out
422+
}
423+
expect_quick_equal(dfrac, list())
424+
425+
# an explicit coercion at the call site is the same program
426+
dcoerce <- function(x) {
427+
declare(type(x = double(1)))
428+
diag(as.integer(x))
429+
}
430+
expect_quick_equal(dcoerce, list(3))
431+
})
432+
351433
test_that("t() and diag() preserve logical mode", {
352434
m <- matrix(c(TRUE, FALSE, TRUE, TRUE), 2, 2)
353435

tests/testthat/test-size-expr-abs.R

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,21 @@ test_that("declare() size expressions validate abs() arity", {
3535

3636
expect_error(quick(bad), "unused argument", fixed = TRUE)
3737
})
38+
39+
test_that("declare() size expressions support as.integer()", {
40+
# as.integer() reaches size expressions through diag()'s identity form,
41+
# but it is spellable on its own: INT() in Fortran, a cast in the bridge
42+
fn <- function(x, n) {
43+
declare(type(x = double(1)), type(n = integer(1)))
44+
out <- double(as.integer(x) + n)
45+
out
46+
}
47+
expect_quick_equal(fn, list(2, 3L), list(2.9, 1L))
48+
49+
bad <- function(x) {
50+
declare(type(x = double(1)))
51+
out <- double(as.integer(x, 2L))
52+
out
53+
}
54+
expect_error(quick(bad), "expects one argument", fixed = TRUE)
55+
})

0 commit comments

Comments
 (0)