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

Documentation for checked shifts is misleading #57

Closed
mateon1 opened this issue Apr 5, 2018 · 1 comment
Closed

Documentation for checked shifts is misleading #57

mateon1 opened this issue Apr 5, 2018 · 1 comment

Comments

@mateon1
Copy link

mateon1 commented Apr 5, 2018

Excerpt from the CheckedShl trait:

///Performs a left shift that returns `None` on overflow.
pub trait CheckedShl: Sized + Shl<u32, Output=Self> {
    /// Shifts a number to the left, checking for overflow. If overflow happens,
    /// `None` is returned.
...
    fn checked_shl(&self, rhs: u32) -> Option<Self>;

The current wording seems to imply that if bits are shifted past the integer's range, then None is returned. This is not the case, the primitive checked_shl explicitly documents this:

        /// Checked shift left. Computes `self << rhs`, returning `None`
        /// if `rhs` is larger than or equal to the number of bits in `self`.
...
        #[stable(feature = "wrapping", since = "1.7.0")]
        #[inline]
        pub fn checked_shl(self, rhs: u32) -> Option<Self> {
            let (a, b) = self.overflowing_shl(rhs);
            if b {None} else {Some(a)}
        }

None is only returned if rhs is larger than the number of bits in the numeric type.
E.g.

0x88u8.checked_shl(4) == Some(0x80)
0x88u8.cheched_shl(8) == None
@cuviper
Copy link
Member

cuviper commented Apr 5, 2018

Right, it's an overflow in the number of bits being shifted, regardless of the value involved. Would you like to send a PR to clarify this?

bors bot added a commit that referenced this issue Oct 9, 2018
90: Fix CheckedShl/CheckedShr documentation r=cuviper a=samueltardieu

Fix #57 and more:

- CheckedShl was hinting that None was returned on overflow rather than
  on too large a rhs.
- Ditto for CheckedShr.
- CheckedShr documentation erroneously indicated that a left shift was
  going to be performed instead of a right shift.

Co-authored-by: Samuel Tardieu <sam@rfc1149.net>
@bors bors bot closed this as completed in #90 Oct 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants