You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2020/09/03 09:05:04 UTC
[GitHub] [thrift] astronaut0131 opened a new pull request #2228: fix rust compiler typedef bug
astronaut0131 opened a new pull request #2228:
URL: https://github.com/apache/thrift/pull/2228
<!-- Explain the changes in the pull request below: -->
a trivial change,fix a bug in the rust compiler when using typedef with union
explanation:
```
typedef i32 int
union Number {
1: int a
}
```
this thrift file will generate a rs file that contains a compile error
```
error[E0308]: mismatched types
--> benchmark/src/main.rs:96:34
|
96 | o_prot.write_i32(f)?;
| ^
| |
| expected `i32`, found `&i32`
| help: consider dereferencing the borrow: `*f`
error: aborting due to previous error; 2 warnings emitted
```
it's because `ttype` is `t_typedef`
https://github.com/apache/thrift/blob/b0d14133d5071370905a1b54b37a1a7c86d50e6d/compiler/cpp/src/thrift/generate/t_rs_generator.cc#L1456
https://github.com/apache/thrift/blob/b0d14133d5071370905a1b54b37a1a7c86d50e6d/compiler/cpp/src/thrift/generate/t_rs_generator.cc#L1457
and `t_typedef`'s `is_base_type()` gives false, but what we need is the actual type of `typedef`
so we can fix this bug by getting the actual type of `typedef`
by 3 lines of code
```
if (ttype->is_typedef()) {
// get the actual type of typedef
ttype = ((t_typedef*)ttype)->get_type();
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2228: fix rust compiler typedef bug
Posted by GitBox <gi...@apache.org>.
Jens-G commented on pull request #2228:
URL: https://github.com/apache/thrift/pull/2228#issuecomment-702398662
I agree to what @ulidtko said about having a test, Can you do that?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G closed pull request #2228: Rust compiler generates invalid code when using typedef with union
Posted by GitBox <gi...@apache.org>.
Jens-G closed pull request #2228:
URL: https://github.com/apache/thrift/pull/2228
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ulidtko commented on pull request #2228: fix rust compiler typedef bug
Posted by GitBox <gi...@apache.org>.
ulidtko commented on pull request #2228:
URL: https://github.com/apache/thrift/pull/2228#issuecomment-686461912
Makes sense in general; but could you add a test demonstrating the bug? That'd also help to prevent future regressions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org