-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document endian conversion functions for ints #12944
Conversation
@@ -99,32 +99,127 @@ pub unsafe fn move_val_init<T>(dst: &mut T, src: T) { | |||
intrinsics::move_val_init(dst, src) | |||
} | |||
|
|||
/// Convert an i16 to little endian. | |||
/// | |||
/// Since the target is little endian, this is a no-op |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing .
s?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, these docs aren't necessarily built on the target machine; so I would prefer the doc string to say something more like
On little-endian targets this is a no-op, on big-endian targets the bytes are swapped.
I guess you could also mention the target on which the docs are built, but that's optional.
Updated diff with @huonw feedback comments |
#[cfg(target_endian = "big")] #[inline] pub fn to_be64(x: i64) -> i64 { x } | ||
|
||
|
||
/// Convert an i16 from little endian. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a little ambiguous about what it's converting to, perhaps something like "Convert a little endian i16 to the host's endianness"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm concerned "host" is unclear in the context of a crosscompiler -- I had originally used that wording.
Maybe "to the target's endianness"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good to me.
Updated diff with feedback from @alexcrichton |
Fix and rename `overflow_check_conditional` fixes rust-lang#2457 Other changes: * Limit the lint to unsigned types. * Actually check if the operands are the same rather than using only the first part of the path. * Allow the repeated expression to be anything as long as there are no side effects. changelog: Rename `overflow_check_conditional` to `panicking_overflow_check` and move to `correctness`
No description provided.