Support for casts in macros with smaller integers#15
Conversation
|
Thanks for your contribution! I won't have time to look at it for at least a week, but maybe @emilio can |
|
It looks reasonable, but I don't have enough nom expertise to properly review. Thanks a lot for working on this! |
crab2313
force-pushed
the
fix-casts-in-macros
branch
from
7e60551 to
2f087fd
Compare
|
@jethrogb Is there any chance to get this pull request merged? I am trying to upload my first crate to crates.io and this feature is the only blocker. |
jethrogb
left a comment
jethrogb
left a comment
There was a problem hiding this comment.
Thanks for the PR! I like the general direction.
| #define Int_n128 (int8_t)128 | ||
| #define Int_n121 (int8_t)1234567 | ||
| #define Int_111 (unsigned int)111.111 | ||
| #define Int_4294967295 (unsigned int)-1 No newline at end of file |
| impl<'a> PRef<'a> { | ||
| method!(cast<PRef<'a>,&[Token],TypeCast,::Error>, mut self, | ||
| delimited!(p!("("), alt!( | ||
| do_parse!(k!("unsigned") >> k!("int") >> (TypeCast::UnsignedInt(size_of::<c_uint>()))) | |
There was a problem hiding this comment.
This will choose the size based on the platform that bindgen runs on (generally the compilation host), that seems incorrect.
There was a problem hiding this comment.
Should we handle cross compilation? unsigned int may be different size on different architectures.
There was a problem hiding this comment.
Yes we should, but I don't have a good idea for how.
| do_parse!(i!("uint32_t") >> (TypeCast::UnsignedInt(4))) | | ||
| do_parse!(i!("int8_t") >> (TypeCast::SignedInt(1))) | | ||
| do_parse!(i!("int16_t") >> (TypeCast::SignedInt(2))) | | ||
| do_parse!(i!("int32_t") >> (TypeCast::SignedInt(4))) |
There was a problem hiding this comment.
What about long, long long, and 64-bit types? In general, it may be useful to handle unknown types here in some graceful manner.
There was a problem hiding this comment.
Any integer type longer than 32 bits(size_t, uint64_t, long) is not supported since EvalResult::Int can not hold it. We need change the API to support 64-bit types.
| impl EvalResult { | ||
| fn cast(self, ctype: TypeCast) -> Self { | ||
| use self::TypeCast::*; | ||
| let round = |bits| 2i64.pow((bits) as u32); |
There was a problem hiding this comment.
Unnecessary parentheses around bits
| impl EvalResult { | ||
| fn cast(self, ctype: TypeCast) -> Self { | ||
| use self::TypeCast::*; | ||
| let round = |bits| 2i64.pow((bits) as u32); |
There was a problem hiding this comment.
I think this doesn't implement rounding, maybe give it another name?
|
@crab2313, any update on this? |
4 participants
Partially fix #4 in the most common cases.
Any integer type longer than 32 bits(size_t, uint64_t, long) is not supported since
EvalResult::Intcan not hold it. For example:Please give me some ideas to simplify the implementation. I know it is ugly.