diff --git a/NEWS.md b/NEWS.md index 557ff88..e340a42 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # coro (development version) +* Generators and async functions no longer hang when their body contains an empty `{}` block, such as `generator(function() {})`. Empty blocks now behave like `NULL` (#75). + * Fixed performance issue related to JIT compilation of generator/async instances (#71). The JIT compiler is now able to properly cache the instances, making repeated instantiations fast, and preventing the JIT from kicking in on every call. * Async functions and generators can now be R6 methods (#63). diff --git a/R/parser.R b/R/parser.R index e47c704..1fb0d4d 100644 --- a/R/parser.R +++ b/R/parser.R @@ -314,6 +314,11 @@ block_states <- function(block, counter, continue, last, return, info) { saved_last <- last last <- FALSE + # An empty block has no states, which would loop forever (#75) + if (is_null(node_cdr(block))) { + block <- quote({ NULL }) + } + block <- duplicate(block, shallow = TRUE) refs <- block_refs(block) states <- NULL @@ -412,6 +417,12 @@ block_states <- function(block, counter, continue, last, return, info) { }, `{` = { nested_node <- node_cdr(expr) + # Splicing an empty block adds nothing and can drop the final + # state, so treat it as `NULL` (#75) + if (is_null(nested_node)) { + node_poke_car(node, NULL) + next + } nested_refs <- block_refs(expr) splice(nested_node, nested_refs) next diff --git a/tests/testthat/helper-flowery.R b/tests/testthat/helper-flowery.R index d6db17b..3995db4 100644 --- a/tests/testthat/helper-flowery.R +++ b/tests/testthat/helper-flowery.R @@ -10,3 +10,10 @@ expect_exhausted <- function(x) { local_methods <- function(..., .frame = caller_env()) { local_bindings(..., .env = global_env(), .frame = .frame) } + +# Turns an infinite loop into an error so a hang fails the test +with_time_limit <- function(expr, seconds = 5) { + setTimeLimit(elapsed = seconds, transient = TRUE) + on.exit(setTimeLimit(elapsed = Inf)) + expr +} diff --git a/tests/testthat/test-async.R b/tests/testthat/test-async.R index c938ea8..d9342c7 100644 --- a/tests/testthat/test-async.R +++ b/tests/testthat/test-async.R @@ -405,3 +405,10 @@ test_that("async methods in R6 classes", { expect_promise(out, status = "pending") }) }) + +test_that("empty `{}` blocks behave like `NULL` (#75)", { + with_time_limit({ + expect_null(wait_for(async(function() {})())) + expect_null(wait_for(async_generator(function() {})()())) + }) +}) diff --git a/tests/testthat/test-generator.R b/tests/testthat/test-generator.R index e711356..5e04da6 100644 --- a/tests/testthat/test-generator.R +++ b/tests/testthat/test-generator.R @@ -609,3 +609,12 @@ test_that("compiled instances do not pin their environment (#36)", { expect_null(wref_key(ref)) }) + +test_that("empty `{}` blocks behave like `NULL` (#75)", { + with_time_limit({ + expect_exhausted(generator(function() {})()()) + expect_equal(collect(generator(function() { yield(1); {} })()), list(1)) + expect_equal(collect(generator(function() { {}; yield(1) })()), list(1)) + expect_equal(collect(generator(function() { {}; yield(1); {} })()), list(1)) + }) +})