Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Proto3 optional fields? #38

Open
m-o-e opened this issue Jul 14, 2021 · 4 comments
Open

Add support for Proto3 optional fields? #38

m-o-e opened this issue Jul 14, 2021 · 4 comments

Comments

@m-o-e
Copy link
Contributor

m-o-e commented Jul 14, 2021

Protoc says:

foobar.proto: is a proto3 file that contains optional fields, but code generator protoc-gen-crystal hasn't been updated to support optional fields in proto3. Please ask the owner of this code generator to support proto3 optional.

optional was re-introduced to proto3 here:
protocolbuffers/protobuf#1606 (comment)

Would be great to have it supported in Crystal. :)

@jgaskins
Copy link
Collaborator

@m-o-e Hmm, I'll have to look into this. Do you have a .proto file we can use to test this?

@m-o-e
Copy link
Contributor Author

m-o-e commented Jul 16, 2021

syntax = "proto3";

message Proto3MsgWithOptional {
  optional string hello = 1;
}

When you run this through protoc you should get:

foo.proto: is a proto3 file that contains optional fields, but code generator protoc-gen-crystal hasn't been updated to support optional fields in proto3. Please ask the owner of this code generator to support proto3 optional.--crystal_out:

(my protoc --version is libprotoc 3.17.3)

According to the implementation guide apparently supporting this requires to
generate "synthetic oneof's" but I haven't fully wrapped my head around it, yet.

While looking into it I tried to improve the existing oneof support a little
because that's something I actually need right now. I have no immediate need
for proto3 optional atm, though, so probably won't have time to tackle that
one soon (just figured I'd open this ticket for awareness).

@jgaskins
Copy link
Collaborator

Huh. I thought all fields were optional in proto3. There was a whole uproar about it. That's why I asked if you could provide an example — I was sure we had to be talking about two different things. 😂 Apparently not. Sure enough, with the latest version of protoc, I do indeed get that error. I'll have a look.

@m-o-e
Copy link
Contributor Author

m-o-e commented Jul 17, 2021

Huh. I thought all fields were optional in proto3

Yup, they were until recently.
And well, still are. But by providing optional you can now distinguish between
"default value" and "unset" on the receiving side. It's really quite confusing,
the thread might shed some light:

protocolbuffers/protobuf#1606 (comment)
(and the impl guide linked above)

Essentially Google admitted their mistake and fixed it in a way
that should make life easier for protobuf-users again, but looks like
a bit of a headache for the implementors. 😞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants