Better codegen for mutually recursive dependencies (#264)

* Better codegen for mutually recursive dependencies (#264)
* v1.1.0
Co-authored-by: Nick Robinson <npr251@gmail.com>

Author: Drvi

Project.toml

@@ -4,7 +4,7 @@ keywords = ["protobuf", "protoc"]
 license = "MIT"
 desc = "Julia protobuf implementation"
 authors = ["Tomáš Drvoštěp <tomas.drvostep@gmail.com>"]
-version = "1.0.16"
+version = "1.1.0"
 
 [deps]
 BufferedStreams = "e1450e63-4bb3-523b-b2a4-4ffa8c0fd77d"
@@ -17,7 +17,7 @@ Aqua = "0.8"
 BufferedStreams = "1.2"
 Dates = "1"
 EnumX = "1"
-JET = "0.4, 0.5, 0.6, 0.7, 0.8"
+JET = "0.4, 0.9"
 TOML = "1"
 Test = "1"
 julia = "1.6"

src/codegen/CodeGenerators.jl

@@ -38,8 +38,9 @@ struct Context
     proto_file::ProtoFile
     proto_file_path::String
     file_map::Dict{String,ResolvedProtoFile}
-    _curr_cyclic_defs::Set{String}
-    _toplevel_name::Ref{String}
+    _types_and_oneofs_requiring_type_params::Dict{String,Vector{Tuple{Bool,String}}}
+    _remaining_cyclic_defs::Set{String}
+    _toplevel_raw_name::Ref{String}
     transitive_imports::Set{String}
     options::Options
 end
@@ -179,10 +180,10 @@ function _protojl(
     # the files are read in order that respect these implicit dependencies.
     resolve_inter_package_references!(parsed_files, options)
     sorted_files, cyclical_imports = _topological_sort(parsed_files)
-    !isempty(cyclical_imports) && throw(error(string(
+    !isempty(cyclical_imports) && error(string(
         "Detected cyclical dependency among following imports: $cyclical_imports, ",
         "possibly, the individual files are resolvable, but their `package`s are not."
-    )))
+    ))
     sorted_files = [parsed_files[sorted_file] for sorted_file in sorted_files]
     n = Namespaces(sorted_files, output_directory, parsed_files)
     for m in n.non_namespaced_protos
@@ -198,4 +199,4 @@ end
 
 export protojl
 
-end # CodeGenerators
+end # CodeGenerators

src/codegen/decode_methods.jl

@@ -103,11 +103,13 @@ function generate_decode_method(io, t::MessageType, ctx::Context)
     println(io, "    " ^ (3 - !has_fields), "PB.skip(d, wire_type)")
     has_fields && println(io, "        end")
     println(io, "    end")
-    print(io, "    return ", jl_typename(t, ctx), "(")
+    print(io, "    return ")
+    _maybe_parametrize_constructor_to_handle_oneofs(io, t, ctx)
+    print(io, "(")
     for (i, field) in enumerate(t.fields)
         print(io, jl_fieldname_deref(field, ctx))
         i < n && (print(io, ", "))
     end
     println(io, ")")
     println(io, "end")
-end
+end

src/codegen/defaults.jl

@@ -15,8 +15,8 @@ function jl_default_value(@nospecialize(field::FieldType), ctx::Context)
 end
 
 function _is_optional_referenced_message(field::Union{FieldType{ReferencedType},GroupType}, ctx::Context)
-    struct_name = ctx._toplevel_name[]
-    (field.type.name == struct_name || field.type.name in ctx._curr_cyclic_defs) && return true
+    struct_name = ctx._toplevel_raw_name[]
+    (field.type.name == struct_name || field.type.name in ctx._remaining_cyclic_defs) && return true
     if field.label == Parsers.OPTIONAL || field.label == Parsers.DEFAULT
         return !_should_force_required(string(struct_name, ".", jl_fieldname(field)), ctx)
     end

src/codegen/metadata_methods.jl

@@ -102,10 +102,79 @@ function maybe_generate_kwarg_constructor_method(io, t::MessageType, ctx::Contex
         print(io, jl_fieldname(field), !isnothing(val) ? string(" = ", val) : "")
         i < n && print(io, ", ")
     end
-    print(io, ") = $(type_name)(")
+    print(io, ") = ")
+    _maybe_parametrize_constructor_to_handle_oneofs(io, t, ctx)
+
+    print(io, "(")
     for (i, field) in enumerate(t.fields)
         print(io, jl_fieldname(field))
         i < n && print(io, ", ")
     end
     println(io, ')')
-end
+end
+
+function maybe_generate_constructor_for_type_alias(io, t::MessageType, ctx::Context)
+    #=
+    Type aliases are supposed to point to _concretized_ stub definitions of the types which
+    we couldn't generate the normal way due to mutually recursive type dependencies. In case
+    the stub definition also contains oneof fields and we parametrize oneofs, the alias would
+    no longer be concretized and would would fail to type-match concrete OneOf instances
+    as valid inputs. Here we generate a constructor for the alias that that provides the
+    concrete types of all parameters to the stub definition. E.g:
+
+    ```proto
+    message A { B b = 1; }
+    message B { oneof x { int32 i = 1; A a = 2; } }
+    ```
+
+    would generate the following types, constructors and aliases:
+
+    ```julia
+    using ProtoBuf: OneOf
+    abstract type var"##Abstract#A" end
+    abstract type var"##Abstract#B" end
+
+    struct var"##Stub#A"{T1<:var"##Abstract#B"} <: var"##Abstract#A"
+        b::Union{Nothing,T1}
+    end
+    struct var"##Stub#B"{T1<:Union{Nothing,OneOf{<:Union{Int32,var"##Abstract#A"}}}} <: var"##Abstract#B"
+        x::T1
+    end
+
+    const A = var"##Stub#A"{var"##Stub#B"{<:Union{Nothing,<:OneOf}}}
+    const B = var"##Stub#B"{<:Union{Nothing,<:OneOf}}
+    B(x) = var"##Stub#B"{typeof(x)}(x) # <- This is what we generate in this method
+    ```
+    Without the last line, we wouldn't be able to call B on concrete OneOf instances:
+    ```julia
+    julia> B(OneOf(:i, Int32(1)))
+    ERROR: MethodError: no method matching (B)(::OneOf{Int32})
+    Stacktrace:
+    [1] top-level scope
+    @ REPL[9]:1
+
+    julia> B(x) = var"##Stub#B"{typeof(x)}(x)
+    B (alias for var"##Stub#B"{<:Union{Nothing, var"#s31"} where var"#s31"<:OneOf})
+
+    julia> B(OneOf(:i, Int32(1)))
+    B{OneOf{Int32}}(OneOf{Int32}(:i, 1))
+    ```
+    =#
+    !(ctx.options.parametrize_oneofs && t.has_oneof_field) && return
+    n = length(t.fields)
+    type_name = safename(t)
+    print(io, "$(type_name)(")
+    for (i, field) in enumerate(t.fields)
+        print(io, jl_fieldname(field))
+        i < n && print(io, ", ")
+    end
+    print(io, ") = ")
+    _maybe_parametrize_constructor_to_handle_oneofs(io, t, ctx)
+
+    print(io, "(")
+    for (i, field) in enumerate(t.fields)
+        print(io, jl_fieldname(field))
+        i < n && print(io, ", ")
+    end
+    println(io, ')')
+end

src/codegen/names.jl

@@ -1,5 +1,6 @@
 const JULIA_RESERVED_KEYWORDS = Set{String}([
     "abstract",
+    "AbstractProtoBufMessage",
     "Any",
     "Array",
     "baremodule",
@@ -19,6 +20,7 @@ const JULIA_RESERVED_KEYWORDS = Set{String}([
     "end",
     "Enum",
     "export",
+    "Expr",
     "false",
     "finally",
     "for",
@@ -64,9 +66,11 @@ function _safename(name::AbstractString)
     end
 end
 
-abstract_type_name(name::AbstractString) = string("var\"##Abstract", name, '"')
+abstract_type_name(name::AbstractString) = string("var\"##Abstract#", name, "\"")
 
 jl_fieldname(@nospecialize(f::AbstractProtoFieldType)) = _safename(f.name)
 jl_fieldname(f::GroupType) = _safename(f.field_name)
 
 _safe_namespace_string(ns::AbstractVector{<:AbstractString}) = string("var\"#$(first(ns))\"", '.', join(@view(ns[2:end]), '.'))
+
+stub_type_name(x) = string("var\"##Stub#", x, "\"")