Change memcmp and bcmp return type to c_int #980
Conversation
compiler-builtins/src/mem/impls.rs
Outdated
|
|
||
| #[inline(always)] | ||
| pub unsafe fn compare_bytes(s1: *const u8, s2: *const u8, n: usize) -> i32 { | ||
| pub unsafe fn compare_bytes(s1: *const u8, s2: *const u8, n: usize) -> crate::mem::c_int { |
There was a problem hiding this comment.
Could you import core::ffi::c_int and use c_int here instead?
I didn't even realize we had crate::mem::c_int, I'll probably remove that to just use core::ffi instead.
There was a problem hiding this comment.
The changes in compiler-builtins/src/mem/mod.rs still need a full path core::ffi::c_int though, unfortunately the macro makes imports a bit weird.
There was a problem hiding this comment.
I didn't even realize we had
crate::mem::c_int, I'll probably remove that to just usecore::ffiinstead.
Doing this here #981
compiler-builtins/src/mem/impls.rs
Outdated
| let b = *s2.wrapping_add(i); | ||
| if a != b { | ||
| return a as i32 - b as i32; | ||
| return a as crate::mem::c_int - b as crate::mem::c_int; |
There was a problem hiding this comment.
| return a as crate::mem::c_int - b as crate::mem::c_int; | |
| return crate::mem::c_int::from(a) - crate::mem::c_int::from(b); |
To make it clearer the conversions are lossless.
|
Ouuups pushed too early, I will do the other changes. |
|
Should I revert my change for the |
tgross35
force-pushed
the
memcmp-use-c_int
branch
from
67a6e90 to
c6af92d
Compare
|
No need, it figured things out with a rebase. One nit though, in compiler-builtins/src/mem/impls.rs could you import Sorry, should have noticed before doing the rebase |
tgross35
left a comment
tgross35
left a comment
There was a problem hiding this comment.
Great, thanks for the fix!
3 participants
This PR fix the return type of
memcmpandbcmpbuiltin function on target with apointer_widthequals to 16.Linked issue: rust-lang/rust#144076