mransan / ocaml-protoc

A Protobuf Compiler for OCaml
https://mransan.github.io/ocaml-protoc/
MIT License
179 stars 33 forks source link

Allow field label to be optional as per proto3 syntax #92

Closed nathanielc closed 7 years ago

nathanielc commented 8 years ago

The protobuf version 3 syntax allows the message fields to not have a label, in which case they are considered optional fields. Example

syntax = "proto3";

message Person {
  string name = 1;
  int32 id = 2;
  string email = 3;
  repeated string phone = 4;
}

This change updates the parser to allow such messages.

I couldn't figure out how to do this conditionally based on the syntax directive at the beginning of the proto file. Any pointers in the right direction would be nice.

mransan commented 8 years ago

Sorry for the delay, I've been travelling recently and had little time. Will look at it very this week.

mransan commented 8 years ago

Hi @nathanielc, in PR #96 I continue your work and propagated the parser change to the compiler other stages.

As explained here, the new proto3 fields generated an ocaml record field which equivalent to what was generated for required label in proto2. The parsing behavior of a message is consistent with the proto3 specs with well-defined, non-overridable values.

In short proto3 semantics says that all fields are optional, but the generated code should not leak the fact that a field is not set; instead default values are used. In fact even if a field is explicitly set to the default value, the field value is not required to be encoded since the decoding semantic will not distinguish the 2 cases. This has been a controversial change of proto3 but one that I would rather follow and be consistent with.

It would be awesome if you could test the latest change by doing:

opam pin add ocaml-protoc ./ 
opam remove ocaml-protoc
opam install ocaml-protoc

Any feedback is appreciated! In about 2 weeks time, after some minor cleanup, I'll publish the new opam version.

nathanielc commented 8 years ago

@mransan Awesome! I'll take a look sometime this week.

mransan commented 7 years ago

ocaml-protoc now supports the proto3 syntax and behavior. Next step would be JSON support