Skip to content
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

Merged
merged 1 commit into from
Mar 17, 2014
Merged

Conversation

mcpherrinm
Copy link
Contributor

No description provided.

@@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing .s?

Copy link
Member

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.

@mcpherrinm
Copy link
Contributor Author

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.
Copy link
Member

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"

Copy link
Contributor Author

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"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good to me.

@mcpherrinm
Copy link
Contributor Author

Updated diff with feedback from @alexcrichton

@bors bors closed this Mar 17, 2014
@bors bors merged commit 5026d11 into rust-lang:master Mar 17, 2014
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 11, 2024
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`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants