Skip to content

Commit ef58aaf

Browse files
committed
Promote ifelse() branches and shape the result like test
Fortran's merge() requires same-typed branches, but the ifelse() handler took its result mode from `yes` alone and never cast `no`: mixed-mode branches emitted an invalid mixed-type merge() (ifelse(c, 1L, a) with double `a` failed at the gfortran stage). The result shape also came from the first non-scalar of (test, yes, no), where R documents ifelse() as returning a result shaped like `test`. Promote both branches to their common lattice mode with promote_operands(), and take the result's mode and shape from the promoted branches and the (booleanized) test respectively. A scalar test with array-valued branches is now a compile-time error -- merge() cannot represent R's length-1 result -- instead of silently emitting branch-shaped code.
1 parent d052a82 commit ef58aaf

3 files changed

Lines changed: 128 additions & 4 deletions

File tree

R/r2f-conditionals.R

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,27 @@
66
r2f_handlers[["ifelse"]] <- function(args, scope, ...) {
77
.[mask, tsource, fsource] <- lapply(args, r2f, scope, ...)
88
mask <- booleanize_logical_as_int(mask)
9-
# (tsource, fsource, mask)
10-
mode <- tsource@value@mode
11-
dims <- conform(mask@value, tsource@value, fsource@value)@dims
12-
Fortran(glue("merge({tsource}, {fsource}, {mask})"), Variable(mode, dims))
9+
10+
# merge() requires same-typed branches; promote both to their common mode.
11+
promoted <- promote_operands(list(tsource, fsource), context = "ifelse()")
12+
.[tsource, fsource] <- promoted$args
13+
mode <- promoted$mode
14+
15+
# R: the result is shaped like `test` (branches only contribute values).
16+
# A scalar test with array branches is not representable with merge().
17+
if (
18+
passes_as_scalar(mask@value) &&
19+
!(passes_as_scalar(tsource@value) && passes_as_scalar(fsource@value))
20+
) {
21+
stop(
22+
"ifelse() result takes the shape of `test`; ",
23+
"array-valued yes/no with scalar test is not supported",
24+
call. = FALSE
25+
)
26+
}
27+
28+
Fortran(
29+
glue("merge({tsource}, {fsource}, {mask})"),
30+
Variable(mode, mask@value@dims)
31+
)
1332
}

tests/testthat/_snaps/ifelse.md

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# ifelse promotes branches and shapes like test
2+
3+
Code
4+
fn
5+
Output
6+
function(c, a) {
7+
declare(type(c = logical(n)), type(a = double(n)))
8+
ifelse(c, 1L, a)
9+
}
10+
<environment: 0x0>
11+
Code
12+
cat(fsub)
13+
Output
14+
subroutine fn(c, a, out_, c__len_) bind(c)
15+
use iso_c_binding, only: c_double, c_int, c_ptrdiff_t
16+
implicit none
17+
18+
! manifest start
19+
! sizes
20+
integer(c_ptrdiff_t), intent(in), value :: c__len_
21+
22+
! args
23+
integer(c_int), intent(in) :: c(c__len_) ! logical
24+
real(c_double), intent(in) :: a(c__len_)
25+
real(c_double), intent(out) :: out_(c__len_)
26+
! manifest end
27+
28+
29+
out_ = merge(real(1_c_int, kind=c_double), a, (c/=0))
30+
end subroutine
31+
Code
32+
cat(cwrapper)
33+
Output
34+
#define R_NO_REMAP
35+
#include <R.h>
36+
#include <Rinternals.h>
37+
38+
39+
extern void fn(
40+
const int* const c__,
41+
const double* const a__,
42+
double* const out___,
43+
const R_xlen_t c__len_);
44+
45+
SEXP fn_(SEXP _args) {
46+
// c
47+
_args = CDR(_args);
48+
SEXP c = CAR(_args);
49+
if (TYPEOF(c) != LGLSXP) {
50+
Rf_error("typeof(c) must be 'logical', not '%s'", Rf_type2char(TYPEOF(c)));
51+
}
52+
const int* const c__ = LOGICAL(c);
53+
const R_xlen_t c__len_ = Rf_xlength(c);
54+
55+
// a
56+
_args = CDR(_args);
57+
SEXP a = CAR(_args);
58+
if (TYPEOF(a) != REALSXP) {
59+
Rf_error("typeof(a) must be 'double', not '%s'", Rf_type2char(TYPEOF(a)));
60+
}
61+
const double* const a__ = REAL(a);
62+
const R_xlen_t a__len_ = Rf_xlength(a);
63+
64+
if (c__len_ != a__len_)
65+
Rf_error("length(a) must equal length(c),"
66+
" but are %0.f and %0.f",
67+
(double)a__len_, (double)c__len_);
68+
const R_xlen_t out___len_ = c__len_;
69+
SEXP out_ = PROTECT(Rf_allocVector(REALSXP, out___len_));
70+
double* out___ = REAL(out_);
71+
72+
fn(
73+
c__,
74+
a__,
75+
out___,
76+
c__len_);
77+
78+
UNPROTECT(1);
79+
return out_;
80+
}
81+

tests/testthat/test-ifelse.R

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,27 @@ test_that("ifelse", {
2626
}
2727
expect_quick_equal(fn, list(seq(-5, 5, length.out = 20), double(20)))
2828
})
29+
30+
test_that("ifelse promotes branches and shapes like test", {
31+
fn <- function(c, a) {
32+
declare(type(c = logical(n)), type(a = double(n)))
33+
ifelse(c, 1L, a)
34+
}
35+
expect_translation_snapshots(fn)
36+
expect_quick_equal(fn, list(c(TRUE, FALSE, TRUE), c(2, 4, 6)))
37+
38+
# logical branches join as logical
39+
fn2 <- function(c, a) {
40+
declare(type(c = logical(n)), type(a = logical(n)))
41+
ifelse(c, FALSE, a)
42+
}
43+
expect_quick_equal(fn2, list(c(TRUE, FALSE, TRUE), c(TRUE, TRUE, FALSE)))
44+
})
45+
46+
test_that("ifelse with scalar test and array branch errors cleanly", {
47+
fn <- function(c, a) {
48+
declare(type(c = logical(1)), type(a = double(n)))
49+
ifelse(c, a, 0)
50+
}
51+
expect_error(quick(fn), "shape of `test`")
52+
})

0 commit comments

Comments
 (0)