fix tracking of serialization state for Function types and Expr

[amitmurthy]

While serialize_cycle was being called for all Function types, deserialize_cycle was not. This causes a mismatch in deserializing state.

Closes #12848

@JeffBezanson - I am not really familiar with the ser/deser codebase. Can you review this?

[JeffBezanson]

I think this needs to be after deserialize_cycle to handle possible cycles through f.env.

[JeffBezanson]

Thanks, great catch.

[tkelman]

This could really use a test if at all possible.

[tkelman]

whitespace

[amitmurthy]

Adding tests was a good suggestion. Threw up errors that @JeffBezanson / others are probably in a better position to fix.

[tkelman]

why ci skip, are they still broken?

[amitmurthy]

As the patch stands, deserialized A is

1 ====> 1
2 ====> 2
3 ====> Any[1,2,#= circular reference =#,abs,abs,(anonymous function),AST(:($(Expr(:lambda, Any[:(x::Any)], Any[Any[Any[:x,:Any,0]],Any[],0,Any[]], :(begin  # none, line 1:
        return x
    end))))),echo,:(begin  # none, line 1:
        return x
    end),CycleFoo.echo,:(return x),4,5]
4 ====> abs
5 ====> abs
6 ====> (anonymous function)
7 ====> AST(:($(Expr(:lambda, Any[:(x::Any)], Any[Any[Any[:x,:Any,0]],Any[],0,Any[]], :(begin  # none, line 1:
        return x
    end)))))
8 ====> echo
9 ====> begin  # none, line 1:
    return x
end
10 ====> CycleFoo.echo
11 ====> return x
12 ====> 4
13 ====> 5

indexexes 6,7 should be the same. As also index pairs (8,9) and (10,11)

[amitmurthy]

Yup. The fix may be a little more involved.

[amitmurthy]

Can anyone give an example of this type of function? Needs to be added to the tests.

[JeffBezanson]

I believe it is for inner generic functions. You can also generate a function that references itself using

function foo()
    f = x->f(x)
end

[amitmurthy]

A short term workaround could be to remove cyclic handling for Function types.

[JeffBezanson]

Ok I think I have this working now. Will push to this branch.

[tkelman]

backported in #13182