rust-bindgen
rust-bindgen copied to clipboard
Remove `as` casts.
Use TryInto
instead of as
casts.
Can you elaborate on why is this useful?
This seems less legible and as long as the as behavior is well defined which afaiui it is, I'm not sure it's worth doing this.
as behavior is well defined which afaiui it is
I don't think casting between c_uint
/c_longlong
and usize
/u32
is well defined, which seems to be the bulk of these.
as behavior is well defined which afaiui it is
I don't think casting between
c_uint
/c_longlong
andusize
/u32
is well defined, which seems to be the bulk of these.
https://doc.rust-lang.org/reference/expressions/operator-expr.html#numeric-cast looks pretty well defined to me? So it saturates on truncation and zero/sign-extends properly.
I mean, yes, it is clearly defined that integers are truncated, but that's not necessarily what we want.
it saturates on truncation
Pretty sure integers don't saturate on truncation, otherwise something like this would be true:
assert_eq!(100000u32 as u16, u16::MAX);
I agree that as
can be confusing (specially if we're discussing if truncation or saturation is the right behavior). But having .try_into().unwrap()
is just causing bindgen to panic at random places without explanation which will be annoying. I'd rather handle each conversion properly or at least comment why unwrap
is fine in those cases.
Most of these are effectively impossible that they fail in any reasonable platform. I don't think c_uint::try_into<usize>
can ever panic for example
Most of these are effectively impossible that they fail in any reasonable platform.
Probably all of these are impossible on reasonable platforms. This just makes it obvious if that isn't actually the case anymore, e.g. due to refactoring/updating a dependency.
I don't think
c_uint::try_into<usize>
can ever panic for example
~~We can probably change these cases to just .into()
, i.e. just fail to compile on unreasonable platforms.~~
Oh right, From<u32>
is not implemented for usize
, so we can't just use .into()
.
Most of these are effectively impossible that they fail in any reasonable platform. I don't think
c_uint::try_into<usize>
can ever panic for example
This is not possible even on several un reasonable platforms. There is nothing in the standard explicitly forbidding uintptr_t
or size_t
from being smaller than unsigned int
, but it is effectively an inexorable conclusion from the design of every platform's data model.
:umbrella: The latest upstream changes (presumably 5ff913ab3349db1905cfbbf44a9121cbc223249d) made this pull request unmergeable. Please resolve the merge conflicts.
@pvdrz, @emilio, can you have another look at this?
@pvdrz, @emilio, please have another look here. Thanks.
:umbrella: The latest upstream changes (presumably 2013b8cb52381fce7b72d8ca7ae93d65f0cf7118) made this pull request unmergeable. Please resolve the merge conflicts.
@pvdrz, @emilio, please review this again.