World age assertion with setindex!(::SubArray) in a generated function

[maleadt]

MWE reduced from Cassette (cc @jrevels):

bar(x::SubArray) = x[] = 42

@generated foo() = bar(whatever)

macro unused() end

foo()

The full thing used to work in the REPL too, but this MWE seems only to work when put in a file:

$ ./julia/usr/bin/julia src/main.jl
julia: /data/maleadt/Julia/creduce/julia/src/gf.c:1431: jl_method_instance_add_backedge: Assertion `callee->def.method->min_world <= caller->min_world && callee->max_world >= caller->max_world' failed.

release-1.0 built with only LLVM_ASSERTIONS=1 and FORCE_ASSERTIONS=1 set:

   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.0.0 (2018-08-08)
 _/ |\__'_|_|_|\__'_|  |  
|__/                   |

[andyferris]

While there may be a bug - I thought function generators were meant to be pure? Is whatever a global?

[maleadt]

I thought function generators were meant to be pure?

Sure, this is just after reducing the bug to the bare minimum. Originally, there wasn't any "undefined behavior" of the sort, and the MWE still works (even in the REPL) when fixing the generator usage:

   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: https://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.7.1-pre.0 (2018-08-09 00:19 UTC)
 _/ |\__'_|_|_|\__'_|  |  release-0.7/36cddc1006* (fork: 51 commits, 10 days)
|__/                   |  x86_64-pc-linux-gnu

julia> bar(x::SubArray) = x[] = 42
bar (generic function with 1 method)

julia> @generated foo(x) = bar(x)
foo (generic function with 1 method)

julia> macro unused() end
@unused (macro with 1 method)

julia> foo(nothing)
julia: /home/tbesard/Julia/julia-dev/src/gf.c:1431: jl_method_instance_add_backedge: Assertion `callee->def.method->min_world <= caller->min_world && callee->max_world >= caller->max_world' failed.

(while this specific version should have thrown a MethodError: no method matching bar(::Type{Nothing}))

[maleadt]

Just an update: I haven't had time to debug this, but it still reproduces on master as of today (8dd3326), and is still triggered by trivial Cassette usage (eg. JuliaLabs/Cassette.jl#73 (comment)).

[Keno]

I've debugged this as follows:
The callee in question is:

_setindex!(Base.IndexLinear, Base.SubArray{T, N, P, I, L} where L where I where P where N where T, Int64, Int64)

the caller is

setindex!(Base.SubArray{T, N, P, I, L} where L where I where P where N where T, Int64, Any...)

Because we don't know that the parent array of this subarray isn't another subarray, this ends up in an edgecycle (i.e. backedges are stored in the cycle_backedges list).
The world age of the setindex! gets expanded by jl_method_set_inferred to the valid age range we compute for the cycle. However, the same does not happen for the _setindex!, because we consider it already_inferred in cache_result, because we previously set inInference to false in finish, so it stays at its temporary inference bounds, causing the assertion failure later.

I'm not entirely sure what the right fix is, but that's how we get the assertion.

[Keno]

The following "fixes" it, but I assume that also defeats the purpose of the check:

diff --git a/base/compiler/typeinfer.jl b/base/compiler/typeinfer.jl
index ae842497ed..87b97c13f3 100644
--- a/base/compiler/typeinfer.jl
+++ b/base/compiler/typeinfer.jl
@@ -148,7 +148,6 @@ function finish(me::InferenceState)
         # a top parent will be cached still, but not this intermediate work
         # we can throw everything else away now
         me.cached = false
-        me.linfo.inInference = false
         me.src.inlineable = false
     else
         # annotate fulltree with type information

[Keno]

This also works:

diff --git a/base/compiler/typeinfer.jl b/base/compiler/typeinfer.jl
index ae842497ed..e22d9e04e2 100644
--- a/base/compiler/typeinfer.jl
+++ b/base/compiler/typeinfer.jl
@@ -148,8 +148,7 @@ function finish(me::InferenceState)
         # a top parent will be cached still, but not this intermediate work
         # we can throw everything else away now
         me.cached = false
-        me.linfo.inInference = false
-        me.src.inlineable = false
+        me.result.src = nothing
     else
         # annotate fulltree with type information
         type_annotate!(me)

However, @vtjnash will have to comment what the right answer is.

[vtjnash]

Oh, awesome work. Looks to me like the assert here is at fault. I think the only consistency check we can actually verify here is that callee->max_world == ~(size_t)0

[Keno]

Ok, but that's not true in this case. The callee max world is the same as the callee min world and equal to the world age of the generated function (since it never got expanded).

[vtjnash]

Oh, yeah that's never supposed to happen. We needed to allocate a dummy cache entry to have some way to convert the edge into a backedge (but then forgot to create the fake node and then tried to connect the back-edge instead to some unrelated target).

[vtjnash]

OK, design should be fixed now where this cannot happen (this assertion has been removed)