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.

Sign up for GitHub

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

Jump to bottom

Rewrite the Rust generator #96

Merged
merged 2 commits into from
May 13, 2024
Merged

Rewrite the Rust generator #96

merged 2 commits into from
May 13, 2024

Conversation

hchataing
Copy link
Collaborator

  • WIP new rust generator
  • WIP split generator for root and derived packets
  • WIP rewrite serializer
  • WIP update generated files

mgeisler
mgeisler reviewed Apr 23, 2024
View reviewed changes
Copy link
Collaborator

@mgeisler mgeisler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks much better than the old code, well done!

pdl-compiler/src/backends/rust/parser.rs Outdated Show resolved Hide resolved
pdl-compiler/src/backends/rust/parser.rs Outdated Show resolved Hide resolved
pdl-compiler/src/backends/rust/serializer.rs Outdated Show resolved Hide resolved
@hchataing hchataing force-pushed the rust-gen-refactor branch 3 times, most recently from f61960a to 27c07d5 Compare April 24, 2024 00:45
mgeisler
mgeisler approved these changes Apr 24, 2024
View reviewed changes
Copy link
Collaborator

@mgeisler mgeisler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks very nice and I imagine the smaller generated files will result in an improvement in code size of the final binaries?

@mgeisler mgeisler changed the title Rewrite the rust generator Rewrite the Rust generator Apr 24, 2024
@hchataing
Copy link
Collaborator Author

I think this looks very nice and I imagine the smaller generated files will result in an improvement in code size of the final binaries?

That might be the case, I haven't checked though.
Do you think we should benchmark the parser / serializer ?

@mgeisler
Copy link
Collaborator

Do you think we should benchmark the parser / serializer ?

No, I wouldn't bother unless that seems to be a bottleneck for someone?

Of course, setting up a benchmark wouldn't be super hard — perhaps you can try Divan, which I've been meaning to try out myself 😄

@hchataing
Refactor the rust generator
5a64e66 
This commit performs an overhaul of the rust generator
which aims at improving the API of the generated code,
to make it more readable and usable.

The major changes are:

- packet and struct have the same generated code.
  This also means that packet derivation is also
  supported for structs now

- builder declarations are removed.
  The generator produces a unique struct for each
  packet that contains the immediate data fields,
  and fields copied from parent declarations

- constrained fields do not appear in the generated
  struct, but an accessor is provided that returns
  the constant value for the field

- the trait pdl_runtime::Packet is extended with the
  function decode() which parses the packet from an
  input slice and returns the remainder data

- TryFrom trait implementations are provided for
  all child and parent packets; these exist mostly
  for convenience and do not aim to be performant
@hchataing hchataing force-pushed the rust-gen-refactor branch from 9731af6 to 2b354fc Compare May 1, 2024 16:22
@hchataing hchataing merged commit 9508a36 into main May 13, 2024
8 checks passed
@hchataing hchataing deleted the rust-gen-refactor branch May 13, 2024 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgeisler mgeisler mgeisler approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hchataing @mgeisler