Help with circular type/inner constructor

randyzwitch · October 8, 2018, 2:18pm

(continued from Slack)

As part of creating types that are originally created in a C++ project, I get the following:

mutable struct TDatumVal <: Thrift.TMsg
  int_val::Int64
  real_val::Float64
  str_val::String
  arr_val::Vector{TDatum}
  TDatumVal() = (o=new(); fillunset(o); o)
end # mutable struct TDatumVal

mutable struct TDatum <: Thrift.TMsg
  val::TDatumVal
  is_null::Bool
  TDatum() = (o=new(); fillunset(o); o)
end # mutable struct TDatum

The above code is auto-generated from Thrift.jl (issue). I tried to just make arr_val in TDatumVal an AbstractVector, but this doesn’t work.

Can someone tell me how I can remedy this? I’ve looked at the inner constructor section of the manual, but I’m still not really clear about how I can fix this (and especially, since the type code was generated in the first place)

mohamed82008 · October 8, 2018, 2:21pm

https://github.com/JuliaLang/julia/issues/269

Workaround:

https://github.com/JuliaLang/julia/issues/269#issuecomment-68421745

randyzwitch · October 8, 2018, 2:29pm

mutable struct TDatumVal{T} <: Thrift.TMsg
  int_val::Int64
  real_val::Float64
  str_val::String
  arr_val::Vector{T}
  TDatumVal() = (o=new(); fillunset(o); o)
end # mutable struct TDatumVal

mutable struct TDatum{T} <: Thrift.TMsg
  val::T
  is_null::Bool
  TDatum() = (o=new(); fillunset(o); o)
end # mutable struct TDatum

julia> using OmniSci
[ Info: Precompiling OmniSci [f1ee3d5f-0699-5209-8f21-83915c40971f]
ERROR: LoadError: LoadError: syntax: too few type parameters specified in "new{...}"
Stacktrace:
 [1] include at ./boot.jl:317 [inlined]
 [2] include_relative(::Module, ::String) at ./loading.jl:1041
 [3] include at ./sysimg.jl:29 [inlined]
 [4] include(::String) at /home/randyzwitch/.julia/dev/OmniSci/src/OmniSci.jl:1
 [5] top-level scope at none:0
 [6] include at ./boot.jl:317 [inlined]
 [7] include_relative(::Module, ::String) at ./loading.jl:1041
 [8] include(::Module, ::String) at ./sysimg.jl:29
 [9] top-level scope at none:2
 [10] eval at ./boot.jl:319 [inlined]
 [11] eval(::Expr) at ./client.jl:389
 [12] top-level scope at ./none:3
in expression starting at /home/randyzwitch/.julia/dev/OmniSci/src/mapd_types.jl:4
in expression starting at /home/randyzwitch/.julia/dev/OmniSci/src/OmniSci.jl:48
ERROR: Failed to precompile OmniSci [f1ee3d5f-0699-5209-8f21-83915c40971f] to /home/randyzwitch/.julia/compiled/v1.0/OmniSci/AxjCm.ji.
Stacktrace:
 [1] macro expansion at ./logging.jl:313 [inlined]
 [2] compilecache(::Base.PkgId, ::String) at ./loading.jl:1187
 [3] macro expansion at ./logging.jl:311 [inlined]
 [4] _require(::Base.PkgId) at ./loading.jl:944
 [5] require(::Base.PkgId) at ./loading.jl:855
 [6] macro expansion at ./logging.jl:311 [inlined]
 [7] require(::Module, ::Symbol) at ./loading.jl:837

ssfrr · October 8, 2018, 2:39pm

You only need the type parameter in the first type, which can’t “see” the 2nd type.

Here’s a simpler example with a circular reference:

mutable struct Foo{T}
    x::Union{Nothing, T}
end

mutable struct Bar
    x::Foo{Bar}
end

f = Foo{Bar}(nothing)
f.x = Bar(f)

randyzwitch · October 8, 2018, 2:42pm

Still the same issue

mutable struct TDatumVal{T} <: Thrift.TMsg
  int_val::Int64
  real_val::Float64
  str_val::String
  arr_val::Vector{T}
  TDatumVal() = (o=new(); fillunset(o); o)
end # mutable struct TDatumVal

mutable struct TDatum <: Thrift.TMsg
  val::TDatumVal
  is_null::Bool
  TDatum() = (o=new(); fillunset(o); o)
end # mutable struct TDatum

julia> using OmniSci
[ Info: Recompiling stale cache file /home/randyzwitch/.julia/compiled/v1.0/OmniSci/AxjCm.ji for OmniSci [f1ee3d5f-0699-5209-8f21-83915c40971f]
ERROR: LoadError: LoadError: syntax: too few type parameters specified in "new{...}"
Stacktrace:
 [1] include at ./boot.jl:317 [inlined]
 [2] include_relative(::Module, ::String) at ./loading.jl:1041
 [3] include at ./sysimg.jl:29 [inlined]
 [4] include(::String) at /home/randyzwitch/.julia/dev/OmniSci/src/OmniSci.jl:1
 [5] top-level scope at none:0
 [6] include at ./boot.jl:317 [inlined]
 [7] include_relative(::Module, ::String) at ./loading.jl:1041
 [8] include(::Module, ::String) at ./sysimg.jl:29
 [9] top-level scope at none:2
 [10] eval at ./boot.jl:319 [inlined]
 [11] eval(::Expr) at ./client.jl:389
 [12] top-level scope at ./none:3
in expression starting at /home/randyzwitch/.julia/dev/OmniSci/src/mapd_types.jl:4
in expression starting at /home/randyzwitch/.julia/dev/OmniSci/src/OmniSci.jl:48

mohamed82008 · October 8, 2018, 2:44pm

That seems to work, not sure of performance.

julia> mutable struct Type1{T}
           a::T
           Type1{T}() where {T} = new{T}()
       end

julia> mutable struct Type2{T}
           a::T
           Type2{T}() where {T} = new{T}()
       end

julia> t = Type1{Type2}()
Type1{Type2}(#undef)

julia> t.a = Type2{Type1}()
Type2{Type1}(#undef)

julia> t.a.a = t
Type1{Type2}(Type2{Type1}(Type1{Type2}(#= circular reference @-2 =#)))

randyzwitch · October 8, 2018, 2:52pm

Thanks. While this allows for the package to precompile, it still causes the error “UnionAll has no field types”.

mutable struct TDatumVal{T} <: Thrift.TMsg
  int_val::Int64
  real_val::Float64
  str_val::String
  arr_val::Vector{T}
  TDatumVal{T}() where T = (o=new(); fillunset(o); o)
end # mutable struct TDatumVal

mutable struct TDatum{T} <: Thrift.TMsg
  val::T
  is_null::Bool
  TDatum{T}() where T = (o=new(); fillunset(o); o)
end # mutable struct TDatum

julia> se = sql_execute(conn, "select count(*) as records from mapd_counties", false, 100, 100)
ERROR: type UnionAll has no field types
Stacktrace:
 [1] getproperty at ./sysimg.jl:15 [inlined]
 [2] meta(::Type, ::Array{Symbol,1}, ::Array{Int64,1}, ::Dict{Symbol,Any}, ::Bool) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:510
 [3] meta(::Type, ::Array{Symbol,1}, ::Array{Int64,1}, ::Dict{Symbol,Any}) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:503
 [4] meta(::Type) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:494
 [5] meta(::Type, ::Array{Symbol,1}, ::Array{Int64,1}, ::Dict{Symbol,Any}, ::Bool) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:522
 [6] meta(::Type, ::Array{Symbol,1}, ::Array{Int64,1}, ::Dict{Symbol,Any}) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:503
 [7] meta(::Type) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:494
 [8] meta(::Type, ::Array{Symbol,1}, ::Array{Int64,1}, ::Dict{Symbol,Any}, ::Bool) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:522
 [9] meta(::Type, ::Array{Symbol,1}, ::Array{Int64,1}, ::Dict{Symbol,Any}) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:503
 [10] meta(::Type) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:494
 [11] meta(::Type, ::Array{Symbol,1}, ::Array{Int64,1}, ::Dict{Symbol,Any}, ::Bool) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:520
 [12] meta(::Type, ::Array{Symbol,1}, ::Array{Int64,1}, ::Dict{Symbol,Any}) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:503
 [13] meta(::Type) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:494
 [14] meta(::Type, ::Array{Symbol,1}, ::Array{Int64,1}, ::Dict{Symbol,Any}, ::Bool) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:520
 [15] meta at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:503 [inlined]
 [16] meta(::Type{OmniSci.sql_execute_result}) at /home/randyzwitch/.julia/dev/OmniSci/src/mapd_types.jl:828
 [17] read_container(::Thrift.TBinaryProtocol, ::OmniSci.sql_execute_result) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:174
 [18] read(::Thrift.TBinaryProtocol, ::OmniSci.sql_execute_result) at /home/randyzwitch/.julia/packages/Thrift/BxShm/src/base.jl:169
 [19] sql_execute(::MapDClient, ::String, ::String, ::Bool, ::String, ::Int32, ::Int32) at /home/randyzwitch/.julia/dev/OmniSci/src/mapd_client.jl:531
 [20] sql_execute(::OmniSciConnection, ::String, ::Bool, ::Int64, ::Int64) at /home/randyzwitch/.julia/dev/OmniSci/src/misc.jl:93
 [21] top-level scope at none:0

ssfrr · October 8, 2018, 2:59pm

Ah, sorry, your constructor for the parametric type needs to be TDatumVal{T}() where T = ....

Here’s a more similar example:

mutable struct Foo{T}
    x::Union{Nothing, T}
    Foo{T}() where T = new(nothing)
end

Foo() = Foo{Bar}()

mutable struct Bar
    x::Foo{Bar}
end

f = Foo()
f.x = Bar(f)

I’m not sure how you’re constructing your type, but you’ll have to make sure you either explicitly create it with TDatumVal{TDatum}() or create a new method (like in my example above) that fills in the type parameter for you.

ssfrr · October 8, 2018, 3:02pm

Though this error might be a bug similar to MethodInstance display errors with UnionAll has no field parameters · Issue #20332 · JuliaLang/julia · GitHub? (I’m not sure). Can you reproduce with a MWE?

randyzwitch · October 8, 2018, 3:09pm

Unfortunately, creating a MWE is difficult as the package I’m trying to create is for accessing a database. And whatever Thrift.jl is trying to do after it gets the message but before creating the type is pretty difficult to understand as well.

StefanKarpinski · October 8, 2018, 3:38pm

We really need to implement something for mutually recursive types at some point soon. It’s always been a low priority since you can work around it, but it’s just one of those features you occasionally need and it’s irritating that it’s not more straightforward. Should really be done in an early 1.x version.

randyzwitch · October 8, 2018, 3:51pm

Do you have a suggestion how to proceed @StefanKarpinski? It seems like no matter how I proceed, I run into an issue:

If I use AbstractVector like I have now (link), I get a ArgumentError: type does not have a definite number of fields
If I do the Parametric type definition, I get a UnionAll error

StefanKarpinski · October 8, 2018, 4:18pm

I would file an issue; I’m not sure about a workaround, I’m afraid.

mohamed82008 · October 9, 2018, 12:06am

A complete working example will help to see where the problem is. I don’t know why your example is failing but if mine is working, it seems like yours should. Not an expert though, so I may be wrong.

randyzwitch · October 9, 2018, 12:20am

I’m not saying your example doesn’t work, but it also causes a problem downstream with UnionAll.

randyzwitch · October 9, 2018, 1:03pm

Thanks to Tanmay, I now have an answer. It’s Thrift.jl specific instead of fixing the circular issue, but that’s okay since that’s where the errors started.

Thanks everyone who tried to help, I really appreciate it!