From b19c2ee39ea49db534918e621859bb45b24e0f01 Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Wed, 15 Dec 2021 00:57:07 +0000 Subject: [PATCH 1/5] fixed bug in replace_returns --- src/compiler.jl | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/compiler.jl b/src/compiler.jl index c7b310f46..b9c7eead9 100644 --- a/src/compiler.jl +++ b/src/compiler.jl @@ -518,16 +518,17 @@ function replace_returns(e::Expr) end if Meta.isexpr(e, :return) - # NOTE: `return` always has an argument. In the case of - # an empty `return`, the lowered expression will be `return nothing`. - # Hence we don't need any special handling for empty returns. - retval_expr = if length(e.args) > 1 - Expr(:tuple, e.args...) - else - e.args[1] + # We capture the original return-value in `retval` and return + # a `Tuple{typeof(retval),typeof(__varinfo__)}`. + # If we don't capture the return-value separately, cases such as + # `return x = 1` will result in `(x = 1, __varinfo__)` which will + # mistakenly attempt to construct a `NamedTuple` (which fails on Julia 1.3 + # and is not our intent). + @gensym retval + return quote + $retval = $(e.args...) + return $retval, __varinfo__ end - - return :(return ($retval_expr, __varinfo__)) end return Expr(e.head, map(replace_returns, e.args)...) From cf2cf656e50cc201975472590ab690825ecd3a28 Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Wed, 15 Dec 2021 00:57:34 +0000 Subject: [PATCH 2/5] added test for empty_model --- test/compiler.jl | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/compiler.jl b/test/compiler.jl index 0544bb5f6..a3ef5f6bd 100644 --- a/test/compiler.jl +++ b/test/compiler.jl @@ -546,6 +546,13 @@ end end @testset "return value" begin + # Make sure that a return-value of `x = 1` isn't combined into + # an attempt at a `NamedTuple` of the form `(x = 1, __varinfo__)`. + @model empty_model() = begin x = 1; end + empty_vi = VarInfo() + retval_and_vi = DynamicPPL.evaluate!!(empty_model(), empty_vi, SamplingContext()) + @test retval_and_vi isa Tuple{Int,typeof(empty_vi)} + # Even if the return-value is `AbstractVarInfo`, we should return # a `Tuple` with `AbstractVarInfo` in the second component too. @model demo() = return __varinfo__ From 2a4cc569e4b7b3932f2670bf158e877a5b35f711 Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Wed, 15 Dec 2021 01:04:15 +0000 Subject: [PATCH 3/5] bumped patch version --- Project.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index 844289af5..5257e90f8 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "DynamicPPL" uuid = "366bfd00-2699-11ea-058f-f148b4cae6d8" -version = "0.17.0" +version = "0.17.1" [deps] AbstractMCMC = "80f14c24-f653-4e6a-9b94-39d6b0f70001" From bdc3e650b195076b8b23689f8c79f032f5509266 Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Wed, 15 Dec 2021 01:08:06 +0000 Subject: [PATCH 4/5] styling for test --- test/compiler.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/compiler.jl b/test/compiler.jl index a3ef5f6bd..d242d52eb 100644 --- a/test/compiler.jl +++ b/test/compiler.jl @@ -548,7 +548,7 @@ end @testset "return value" begin # Make sure that a return-value of `x = 1` isn't combined into # an attempt at a `NamedTuple` of the form `(x = 1, __varinfo__)`. - @model empty_model() = begin x = 1; end + @model empty_model() = return x = 1; empty_vi = VarInfo() retval_and_vi = DynamicPPL.evaluate!!(empty_model(), empty_vi, SamplingContext()) @test retval_and_vi isa Tuple{Int,typeof(empty_vi)} From 34f22e8a0dbf723766f719b18037e88c33d015dc Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Wed, 15 Dec 2021 01:10:31 +0000 Subject: [PATCH 5/5] Update test/compiler.jl Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- test/compiler.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/compiler.jl b/test/compiler.jl index d242d52eb..9dc81ff16 100644 --- a/test/compiler.jl +++ b/test/compiler.jl @@ -548,7 +548,7 @@ end @testset "return value" begin # Make sure that a return-value of `x = 1` isn't combined into # an attempt at a `NamedTuple` of the form `(x = 1, __varinfo__)`. - @model empty_model() = return x = 1; + @model empty_model() = return x = 1 empty_vi = VarInfo() retval_and_vi = DynamicPPL.evaluate!!(empty_model(), empty_vi, SamplingContext()) @test retval_and_vi isa Tuple{Int,typeof(empty_vi)}