ppx_deriving_protobuf icon indicating copy to clipboard operation
ppx_deriving_protobuf copied to clipboard

Does not support passing builtin types as type parameters

Open struktured opened this issue 9 years ago • 3 comments

type 'a range = {min:'a [@key 1]; max:'a [@key 2]} [@@deriving Protobuf];;

_type 'a range = { min : 'a; max : 'a; } val range_from_protobuf : (Protobuf.Decoder.t -> 'a) -> Protobuf.Decoder.t -> 'a range = val range_to_protobuf : ('a -> Protobuf.Encoder.t -> unit) -> 'a range -> Protobuf.Encoder.t -> unit = *

type string_range = string range [@@deriving Protobuf];;

*Error: Unbound value string_from_protobuf_

For completeness, note that following works fine:

type foo = string [@@deriving Protobuf];;

type foo = bytes val foo_from_protobuf : Protobuf.Decoder.t -> bytes = val foo_to_protobuf : bytes -> Protobuf.Encoder.t -> unit =

struktured avatar Oct 11 '14 16:10 struktured

Yes. This is a known problem, in fact.

The core of the issue is that you need the parameter of a parametric type to be a message. I decided to not just synthesize them when you pass one of the builtin types as a parameter. Perhaps I should; but I'm not very convinced yet, as it makes the eventual protobuf structures unclear.

whitequark avatar Oct 11 '14 16:10 whitequark

I see, ok, thanks for the clarification. I suppose I was hoping that in the case I posted, that

type string_range = string range [@@deriving Protobuf]

would be exactly the same as if you typed the following into ppx_deriving_protobuf (from (de)serialization perspective, at least):

type string_range_explicit = { min:string [@key 1] : max : string [@key 2] } [@@deriving Protobuf]

I suppose you were more initially concerned with

type data_x = < a complex ocaml type annotated with deriving protobuf > type data_y = < another complex ocaml type annotated with deriving protobuf >

type payload = {data: 'a [@key 1]; length:int [@key 2]} [@@deriving Protobuf]

And thus, the only way to handle both data_x and data_y is to make an inner message (eg. how else can the length field be assuredly associated with key 2 otherwise)?

However, when it's a primitive (not a builtin type necessarily, but a true protobuf primitive), then you don't want to synthesize a message at all as there is no need to (it can use data's key 1 rather than wrapping itself in a message). Besides the point, when the 'a type is made concrete you don't need the polymorphism overhead either.

In theory, you could even "embed" or "inline" any specific instance of 'a, but it would obfuscate the protobuf message definitions and make it harder to evolve existing protocols.

type data_x = {zed:string [@key 1]} [@@deriving Protobuf] type data_y = {foo:string [@key 1]; bar:int [@key 2]} [@@deriving Protobuf]

type payload = {data: 'a [@embedded 1]; length:int [@offset 2]} [@@deriving Protobuf]

Then if you wrote

type data_x_payload = data_x payload [@@deriving Protobuf] type data_y_payload = data_y payload [@@deriving Protobuf]

It would implicitly create the following messages:

type data_x_payload = {zed: string [@key 1]; length: int [@key 2]} [@@deriving Protobuf]

type data_y_payload = {foo:bar [@key 1];bar: string [@key 2]; length: int [@key 3]} [@@deriving Protobuf]

I'm not saying that feature is worth implementing at all, but I do think if you auto inlined in the case of protobuf primitives (and only when 'a type is made concrete with a type alias) that would be a reasonable approach, as its very obvious to the programmer what the resulting message would be.

My only concern is if a typed alias version gets passed to a protocol which always expect 'a to be wrapped, it would probably generate errors at runtime unless there's a way the ocaml compiler can verify beforehand. Maybe we are forced to wrap everything for that very reason.

I can certainly work around this, so no rush resolving it one or another.

struktured avatar Oct 11 '14 17:10 struktured

What you're saying is entirely correct. I don't want to introduce runtime type errors in the library.

whitequark avatar Oct 11 '14 17:10 whitequark