tafia / quick-protobuf

A rust implementation of protobuf parser
MIT License
452 stars 87 forks source link

Field presence #247

Closed snproj closed 1 year ago

snproj commented 1 year ago

Continuation of #243

Since I no longer have access to the ghpr-asia fork that I made that PR with.

Minor notes:

Future action

snproj commented 1 year ago

Made the changes as per above, also removed some extra parentheses in the generated get_size()es that previous versions of this PR added in.

Edit: cleaning up diff

snproj commented 1 year ago

I've spent a few hours trying to get the diff as small as possible for the get_size() and write_message() parts, but the way that the code generation has changed in this PR means that eliminating all the differences is hard without a bunch of error-prone rewriting (that will eventually be reverted anyway when I make the PR to clean it up, since I already have the code for that). I don't know that it's impossible, but I didn't want to delay too long haha

If you're curious, the previous code had control flow in various parts that determine when to add a dereference into the code, which had built up over time into generating a bunch of &&*&**&. This is the main source of diff, since I decided to rewrite the control flow to avoid these chains of references and dereferences; which makes it hard to artificially add them back in and shrink the diff.

Hopefully it's ok 🙏

thomaseizinger commented 1 year ago

I've spent a few hours trying to get the diff as small as possible for the get_size() and write_message() parts, but the way that the code generation has changed in this PR means that eliminating all the differences is hard without a bunch of error-prone rewriting (that will eventually be reverted anyway when I make the PR to clean it up, since I already have the code for that). I don't know that it's impossible, but I didn't want to delay too long haha

If you're curious, the previous code had control flow in various parts that determine when to add a dereference into the code, which had built up over time into generating a bunch of &&*&**&. This is the main source of diff, since I decided to rewrite the control flow to avoid these chains of references and dereferences; which makes it hard to artificially add them back in and shrink the diff.

Hopefully it's ok pray

Yeah that is fine. There is a means to an end when it comes to reducing diff :)

thomaseizinger commented 1 year ago

https://github.com/libp2p/rust-libp2p/pull/3455 uses this branch and it works well, green light from our end!

d-roak commented 1 year ago

Hi @snproj!

Can you do a version bump in the near future to include this change?

Thanks 🙏

mxinden commented 1 year ago

Friendly ping. Could you cut a new release with this patch?