You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Allen George (Jira)" <ji...@apache.org> on 2021/03/06 14:37:00 UTC
[jira] [Commented] (THRIFT-5364) Remove clippy::box_vec lint
override
[ https://issues.apache.org/jira/browse/THRIFT-5364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17296550#comment-17296550 ]
Allen George commented on THRIFT-5364:
--------------------------------------
I _think_ the best option here is to know what context the code is being generated in, i.e. whether the type/read is for a top-level container element or not. If it's a top-level container element we should **never** use boxing, regardless of whether the type is a typedef or not. But if it's not a top-level container element then we should use the standard rules.
In other words, add an {{is_top_level_container_element}} as a flag that can override the default code-generation behavior.
> Remove clippy::box_vec lint override
> ------------------------------------
>
> Key: THRIFT-5364
> URL: https://issues.apache.org/jira/browse/THRIFT-5364
> Project: Thrift
> Issue Type: Improvement
> Components: Rust - Compiler
> Reporter: Allen George
> Priority: Major
>
> Currently the rust code generator wraps types in a `Box` in two functions:
> # {{to_rust_type}}
> # {{render_type_sync_read}}
> We do this only when a type is a {{typedef}} and {{ttypedef->is_forward_typedef() == true}}. This can result in situations where the following thrift ({{test/Recursive.thrift}}):
> {noformat}
> struct RecTree {
> 1: list<RecTree> children
> 2: i16 item
> }
> {noformat}
> generates the following rust:
> {noformat}
> #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
> pub struct RecTree {
> pub children: Option<Vec<Box<RecTree>>>,
> pub item: Option<i16>,
> }
> {noformat}
> Clippy trips this code up with the following error:
> {noformat}
> /root/.cargo/bin/cargo fmt --all -- --check
> /root/.cargo/bin/cargo clippy --all -- -D warnings
> Checking kitchen-sink v0.1.0 (/thrift/src/lib/rs/test)
> error: `Vec<T>` is already on the heap, the boxing is unnecessary.
> --> src/recursive.rs:35:24
> |
> 35 | pub children: Option<Vec<Box<RecTree>>>,
> | ^^^^^^^^^^^^^^^^^ help: try: `Vec<recursive::RecTree>`
> |
> = note: `-D clippy::vec-box` implied by `-D warnings`
> = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#vec_box
> error: aborting due to previous error
> error: could not compile `kitchen-sink`.
> {noformat}
> This happens because all container elements ({{Vec}},{{BTreeSet}},{{BTreeMap}}) are automatically placed on the heap, so a {{Box}} is an additional level of indirection.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)