`encode` fails for vectors with `eltype` of non-definitive size

[bachdavi]

I, experimentally ;), used your package for our JuliaSDK, https://github.com/RelationalAI/rai-sdk-julia/pull/60, and have encountered a problem when encoding a struct. We have a mutually recursive type RelType that is parameterized by two types:

struct RelType{T2<:Union{Nothing,var"##AbstractConstantType"},T1<:Union{Nothing,var"##AbstractValueType"}} <: var"##AbstractRelType"
    tag::Kind.T
    primitive_type::PrimitiveType.T
    value_type::T1
    constant_type::T2
end

In our tree structure, we have a type that contains a Vector of these RelTypes:

struct RelationId <: var"##AbstractRelationId"
    arguments::Vector{RelType}
end

When using encode to encode this type we get the following error:

julia> ProtocolBuffers.encode(e, m)                                                                                                                                                                        
ERROR: Argument is an incomplete RelType type and does not have a definite size.                                                                
Stacktrace:                                                                                                                                     
  [1] sizeof                                                                                                                                    
    @ ./essentials.jl:476 [inlined]                                                                                                             
  [2] encode(e::ProtocolBuffers.Codecs.ProtoEncoder{IOBuffer}, i::Int64, x::Vector{RelType{<:Union{Nothing, ConstantType}, <:Union{Nothing, Valu
eType}}})                                                                                                                                       
    @ ProtocolBuffers.Codecs ~/.julia/dev/ProtocolBuffers/src/codec/encode.jl:282                                                               
  [3] encode                                                                                                                                    
    @ ~/Projects/rai-sdk-julia/src/gen/relationalai/protocol/schema_pb.jl:392 [inlined]                                                         
  [4] _with_size                                                                                                                                
    @ ~/.julia/dev/ProtocolBuffers/src/codec/encode.jl:19 [inlined]                                                                             
  [5] encode(e::ProtocolBuffers.Codecs.ProtoEncoder{IOBuffer}, i::Int64, x::RelationId)                                                         
    @ ProtocolBuffers.Codecs ~/.julia/dev/ProtocolBuffers/src/codec/encode.jl:292                                                               
  [6] encode                                                                                                                                    
    @ ~/Projects/rai-sdk-julia/src/gen/relationalai/protocol/message_pb.jl:35 [inlined]                                                         
  [7] _with_size                                                                                                                                
    @ ~/.julia/dev/ProtocolBuffers/src/codec/encode.jl:19 [inlined]                                                                             
  [8] encode(e::ProtocolBuffers.Codecs.ProtoEncoder{IOBuffer}, i::Int64, x::Vector{RelationMetadata})                                           
    @ ProtocolBuffers.Codecs ~/.julia/dev/ProtocolBuffers/src/codec/encode.jl:285                                                               
  [9] encode(e::ProtocolBuffers.Codecs.ProtoEncoder{IOBuffer}, x::MetadataInfo)                                                                 
    @ RAI.Protocol_PB ~/Projects/rai-sdk-julia/src/gen/relationalai/protocol/message_pb.jl:67                                                   
 [10] top-level scope                                                                                                                           
    @ REPL[10]:1  

I tried to more specifically type the RelationId struct by adding type bounds to it, but either it's still not definitive, or we cannot construct the type. As a workaround, I added an overload for the encode for this specific type:

function ProtocolBuffers.encode(e::ProtocolBuffers.AbstractProtoEncoder, i::Int, x::Vector{Protocol_PB.RelType})
    # This is the max size of `RelType` when either the `ConstantType` or `ValueType` are
    # set!
    size = 24
    ProtocolBuffers.Codecs.maybe_ensure_room(e.io, length(x) * size)
    for el in x
        ProtocolBuffers.Codecs.encode_tag(e, i, ProtocolBuffers.Codecs.LENGTH_DELIMITED)
        ProtocolBuffers.Codecs._with_size(ProtocolBuffers.encode, e.io, e, el)
    end
    return nothing
end

[Drvi]

Hey @bachdavi, thanks for the report! All these maybe_ensure_room are highly approximate and strictly optional, sorry this crashed on you. I'll look into something more robust to use instead of sizeof for structs. Good job on the workaround, though!

I'll note that I wouldn't in general recommend using the package until https://github.com/Drvi/ProtocolBuffers.jl/pull/12 is merged (hopefully today/tomorrow), as some protos with relatively complex dependencies are not resolved correctly on master.

[bachdavi]

Thank you @Drvi and thank you for writing this package 🙂 It is a great improvement! Do you intend to register it when #12 is merged?

[Drvi]

@bachdavi I think once https://github.com/Drvi/ProtocolBuffers.jl/pull/12 is done, we'll be in pretty releasable state:)

I'm currently trying to get all of https://github.com/googleapis/googleapis to work (parsing, translating and importing is very close to working)... The repo contains cca 3700 proto files totalling~ 900k LOC. This exposes some design issues (Julia doesn't really like importing 900k LOC packages, for example), I'd like to get these foundations right before we go public.

[Drvi]

Hey, @bachdavi! I pushed a fix for this issue, could you verify this now works for you? Some more PRs have been merged since so you'll have to re-generate your code which will now produce code with different module names (apologies, but there is a reason for the change).

Sorry for the delay, the streak of me mispredicting time to finish a programming task keeps on going.