Merge pull request #116 from mrc-ide/mrc-6009

Error if arrays accessed without index

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: odin2
 Title: Next generation odin
-Version: 0.2.10
+Version: 0.2.11
 Authors@R: c(person("Rich", "FitzJohn", role = c("aut", "cre"),
                     email = "[email protected]"),
              person("Wes", "Hinsley", role = "aut"),

R/parse.R

@@ -184,6 +184,19 @@ parse_check_consistent_dimensions_rhs <- function(eq, dat, call, src = eq$src) {
       "E2019", src, call)
   }
 
+  throw_array_as_scalar <- function(var, rank) {
+    what <- rank_description(rank)
+    if (rank == 1) {
+      dummy_index <- "..."
+    } else {
+      dummy_index <- paste(rep(".", rank), collapse = ", ")
+    }
+    odin_parse_error(
+      c("Trying to use {what} '{var}' without index",
+        i = sprintf("Did you mean '{var}[%s]'", dummy_index)),
+      "E2022", src, call)
+  }
+
   fn_use_whole_array <- c("length", "nrow", "ncol", "OdinReduce",
                           "OdinInterpolate")
 
@@ -233,6 +246,11 @@ parse_check_consistent_dimensions_rhs <- function(eq, dat, call, src = eq$src) {
       } else {
         lapply(expr[-1], check)
       }
+    } else if (is.symbol(expr)) {
+      nm <- as.character(expr)
+      if (nm %in% dat$storage$arrays$name) {
+        throw_array_as_scalar(nm, dim_ranks[[nm]])
+      }
     }
   }
   check(eq$rhs$expr)

tests/testthat/test-generate.R

@@ -2358,7 +2358,7 @@ test_that("cast integers to sizes", {
 test_that("can generate system with output", {
   dat <- odin_parse({
     initial(x[]) <- 0
-    deriv(x[]) <- x * r[i]
+    deriv(x[]) <- x[i] * r[i]
     r <- parameter()
     n <- 3
     dim(x) <- n
@@ -2384,7 +2384,7 @@ test_that("can generate system with output", {
     c(method_args$rhs,
       "  const auto * x = state + 0;",
       "  for (size_t i = 1; i <= shared.dim.x.size; ++i) {",
-      "    state_deriv[i - 1 + 0] = x * shared.r[i - 1];",
+      "    state_deriv[i - 1 + 0] = x[i - 1] * shared.r[i - 1];",
       "  }",
       "}"))
 

tests/testthat/test-parse.R

@@ -888,7 +888,7 @@ test_that("LHS of assignment with [] on sum is accepted", {
   expect_error(
     odin_parse({
       initial(x) <- 0
-      update(x) <- a_tot
+      update(x) <- sum(a_tot)
       dim(a) <- c(4, 4)
       dim(a_tot) <- 4
       a[, ] <- 3
@@ -1054,9 +1054,36 @@ test_that("don't duplicate offsets when boundary condition used in initial", {
   dat <- odin_parse({
     initial(x[]) <- 0
     initial(x[1]) <- 1
-    update(x[]) <- x + 1
+    update(x[]) <- x[i] + 1
     dim(x) <- 4
   })
   expect_equal(dat$variables, "x")
   expect_equal(nrow(dat$storage$packing$state), 1)
 })
+
+
+test_that("prevent use of arrays without braces", {
+  err <- expect_error(
+    odin_parse({
+      initial(x[]) <- 0
+      update(x[]) <- b
+      b[] <- x[i]
+      dim(x) <- 5
+      dim(b) <- 5
+    }),
+    "Trying to use vector 'b' without index")
+  expect_match(conditionMessage(err),
+               "Did you mean 'b[...]'", fixed = TRUE)
+
+  err <- expect_error(
+    odin_parse({
+      initial(x[]) <- 0
+      update(x[]) <- b
+      b[, ] <- 1
+      dim(x) <- 5
+      dim(b) <- c(5, 5)
+    }),
+    "Trying to use matrix 'b' without index")
+  expect_match(conditionMessage(err),
+               "Did you mean 'b[., .]'", fixed = TRUE)
+})

vignettes/errors.Rmd

@@ -1097,6 +1097,17 @@ You have tried to set the dimensions of the same variable more than once. For ex
 dim(a, b) <- 2
 dim(b, c) <- 3
 
+# `E2022`
+
+Trying to access an array without using square bracket indexes.  For example if you write:
+
+```r
+dim(b) <- 10
+a[] <- b
+```
+
+you would see this error, as you should have used `b[i]` on the right hand side (*probably!*; you might have meant `a[] <- b[1]`, too).
+
 # `E3001`
 
 An array access would be out of bounds.  This error is thrown where your code would result in you reading or writing out of bounds on an array (or a dimension of an array).  Currently, we throw this error if we are certain that your access is invalid. For example, we would error if your code contains: