-
Notifications
You must be signed in to change notification settings - Fork 532
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
prost-build: refactor code generator (ground work for builders) #1011
prost-build: refactor code generator (ground work for builders) #1011
Conversation
Collect message field data into structs that can be reused in multiple passes. This will be useful for the upcoming generation of builders. Add prost_path helper function to DRY repetitive expressions obtaining the path to the prost crate. Add boxed helper method to capture the logic of deciding whether a field is boxed.
In CodeGenerator::resolve_type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for splitting this of the builder PR.
Your commit description mentions three different improvements. Please split the separate improvements into separate commits.
} | ||
|
||
struct OneofField { | ||
name: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you use this variable you call it rust_name
. I suggest calling it that here as well
|
||
struct OneofField { | ||
name: String, | ||
proto: OneofDescriptorProto, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this should be called descriptor
} | ||
}); | ||
// Optional fields create a synthetic oneof that we want to skip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems new functionality. Why is this needed? Does this change the generated code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not change the generated code.
#901 adds a third pass over the oneof declarations to generate the setters for the builder, so I opted to collect the field data here in a pre-filtered and more usable form.
} | ||
}); | ||
// Optional fields create a synthetic oneof that we want to skip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not change the generated code.
#901 adds a third pass over the oneof declarations to generate the setters for the builder, so I opted to collect the field data here in a pre-filtered and more usable form.
for (idx, oneof) in message.oneof_decl.into_iter().enumerate() { | ||
let idx = idx as i32; | ||
// optional fields create a synthetic oneof that we want to skip | ||
let fields = match oneof_fields.remove(&idx) { | ||
Some(fields) => fields, | ||
None => continue, | ||
}; | ||
self.append_oneof(&fq_message_name, oneof, idx, fields); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was done with a moving iterator and moving the field data into append_oneof
, but there was no reason to move the data because code generation only needs references. And I need the same data for the builder pass which will be added later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, the code just moved. That de-duplucation is nice.
Superseded by #1016. |
Refactoring of existing code taken out of #901:
prost_path
helper function to DRY repetitive expressions constructing the path to the prost crate for generated code.boxed
helper method to capture the logic of deciding whether a field is boxed.