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

make-structs cmd line option #15

Merged
merged 5 commits into from
Feb 11, 2019
Merged

Conversation

deviator
Copy link
Contributor

@deviator deviator commented Feb 9, 2019

No description provided.

@deviator deviator changed the title make-structs cmd line option, partial resolve #14 make-structs cmd line option Feb 9, 2019
@deviator
Copy link
Contributor Author

deviator commented Feb 9, 2019

partial resolve #14

Copy link
Owner

@dcarp dcarp left a comment

Choose a reason for hiding this comment

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

This would be a good workaround till we find a solution to make struct/class setting persistent (per
message definition) in the .proto file.
Thank you for the PR!

@@ -9,7 +9,10 @@ import tutorial.addressbook;
/// This function fills in a Person message based on user input.
Person promptForAddress()
{
auto person = new Person;
static if (is(Person == class))
Copy link
Owner

Choose a reason for hiding this comment

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

Three times the same static if/else, maybe a small template make!T.
Ex.: auto person = make!Person

result ~= "\n";
result ~= strIndent;
result ~= indent > 0 ? "static " : "";
result ~= makeStructs ? "struct" : "class";
Copy link
Owner

Choose a reason for hiding this comment

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

Please check here if the protobuf message definition is recursive. struct cannot be used for recursive messages.
This should be check in the code generator, otherwise the user would need to deal with it at compile time, probably not knowing that the generated message definition is wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What you mean by recursive messages? A in B as field, and B in A as field, right?
Fields must be pointers in this case and there comes more difficult code for working with its (get address of stack variable is bad idea, need use GC for allocate memory, or any other variant). I think this case is example where structs are not suits.

protoc_gen_d/protoc-gen-d.d Outdated Show resolved Hide resolved
examples/dub.json Outdated Show resolved Hide resolved
protoc_gen_d/protoc-gen-d.d Outdated Show resolved Hide resolved
@dcarp dcarp merged commit 0559ecb into dcarp:master Feb 11, 2019
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

Successfully merging this pull request may close these issues.

2 participants