[
https://issues.apache.org/jira/browse/THRIFT-2945?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15864893#comment-15864893
]
Allen George edited comment on THRIFT-2945 at 2/14/17 2:20 AM:
---------------------------------------------------------------
[~prz] - let me try to address these points individually.
# "Split the {{read_from_in_protocol}} and {{write_to_out_protocol}} as a
trait." This seems like a great idea. Perhaps it could be named {{TThriftType}}
to keep with the existing naming convention? I suggest submitting this as a PR
since it would be small and self-contained.
# "Auto-derive hash on generated types." Are you sure this works with types
that contain {{OrderedFloat}}? Thrift doubles (represented in Rust by floats,
not fixed-point integers) are the reason why I had to use {{BTreeMap}} and
{{BTreeSet}}, and was unable to auto-derive {{Hash}}. Perhaps I overlooked
something?
# Could you clarify why you need to change the enum and union code-generation?
From what I understand, your main requirement was getting a list of all defined
enum variants and associated values. Since we know the full list of variants
ahead from the thrift file we could simply add a {{values()}} method that
returns the information you need. Also, I specifically opted *not* to generate
macros since it adds another level of indirection that anybody debugging thrift
auto-gen code would have to reason about: they'd have to understand how the
thrift gets turned into a macro, the macro definition, and then the final rust
output.
# Finally, while I understand that you want to use the Thrift {{namespace}}
directive, could you give me a concrete example of what you expect the output
to be? Would it be output to different directories, or would you output it to
the same directory and then rely on a manual step afterwards to move it to the
appropriate Rust module. (PS. Your patch has a bug: you forgot to
{{rust_safe_case}} the module name, and it will break on thrift modules that
already have underscores in them)
Another points: I looked at your branch, and noticed that your formatting is
inconsistent with the existing code (the existing code uses 2-space tabs).
was (Author: allengeorge):
[~prz] - let me try to address these points individually.
# "Split the {{read_from_in_protocol}} and {{write_to_out_protocol}} as a
trait." This seems like a great idea. Perhaps it could be named {{TThriftType}}
to keep with the existing naming convention? I suggest submitting this as a PR
since it would be small and self-contained.
# "Auto-derive hash on generated types." Are you sure this works with types
that contain {{OrderedFloat}}? Thrift doubles (represented in Rust by floats,
not fixed-point integers) are the reason why I had to use {{BTreeMap}} and
{{BTreeSet}}, and was unable to auto-derive {{Hash}}. Perhaps I overlooked
something?
# Could you clarify why you need to change the enum and union code-generation?
From what I understand, your main requirement was getting a list of all defined
enum variants and associated values. Since we know the full list of variants
ahead from the thrift file we could simply add a {{values()}} method that
returns the information you need. Also, I specifically opted *not* to generate
macros since it adds another level of indirection that anybody debugging thrift
auto-gen code would have to reason about: they'd have to understand how the
thrift gets turned into a macro, the macro definition, and then the final rust
output.
# Finally, while I understand that you want to use the Thrift {{namespace}}
directive. Could you give me a concrete example of what you expect the output
to be? Would it be output to different directories, or would you output it to
the same directory and then rely on a manual step afterwards to move it to the
appropriate Rust module. (PS. Your patch has a bug: you forgot to
{{rust_safe_case}} the module name, and it will break on thrift modules that
already have underscores in them)
Another points: I looked at your branch, and noticed that your formatting is
inconsistent with the existing code (the existing code uses 2-space tabs).
> Implement support for Rust language
> -----------------------------------
>
> Key: THRIFT-2945
> URL: https://issues.apache.org/jira/browse/THRIFT-2945
> Project: Thrift
> Issue Type: New Feature
> Components: Rust - Compiler, Rust - Library
> Reporter: Maksim Golov
> Assignee: Allen George
> Fix For: 0.11.0
>
>
> Work on implementing support for Rust is in progress:
> https://github.com/maximg/thrift by Simon Génier and myself.
> It will probably take quite some time to complete. Please keep us updated if
> there are changes related to our work.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
