next_power_of_two should panic on overflow

[Gankra]

If you feed in a number such that the next_power_of_two would overflow, it spits out 0. I'm fine with that honestly, since it's easy to detect, but maybe that isn't what we want here. Maybe it should return a Result/Option/panic/saturate?

Should be specified so that the behaviour can be relied upon, or explicitly documented as unspecified.

[darnuria]

@gankro Actually there is a std::num::UnsignedInt::checked_next_power_of_two a good contribution would be to add a documentation precision into std::num::UnsignedInt::next_power_of_two on how is defined behavior on overflow?

It could be tagged as a documentation bug, and an easy one? :)

[Gankra]

Yes, this can be considered a documentation bug, assuming we sit down and settle what we want the semantics to be. I've got no strong opinion, so I'll defer to @bjz and @aturon (unless you have a thought on what it should be?).

[aturon]

cc @nikomatsakis

[theemathas]

cc #22020

[steveklabnik]

Triage: docs wise, this says " Unspecified behavior on overflow.". @theemathas has linked to the overflow RFC, and this discussion was well before that, so there's no reason for overflow behavior to be particularly exceptional here, right? It follows the same rules as everything else, unless @rust-lang/libs wants to make it special in some way.

[brson]

Here's the implementation:

        pub fn next_power_of_two(self) -> Self {
            let bits = size_of::<Self>() * 8;
            let one: Self = Self::one();
            one << ((bits - self.wrapping_sub(one).leading_zeros() as usize) % bits)
        }

I think @gankro's original comments were from before the overflow RFC so this no longer results in 0 on overflow but behaves per the RFC. And as stated there is now a checked_ variant of this function. I think we should just update the documentation to state that it's overflow behavior corresponds to whatever setting controls overflow and move on. Is there existing precedent for methods in std allowing overflow internally?

[irfanhudda]

I'll take a shot at this.

[Mark-Simulacrum]

So, I was going to close this, but actually, it looks like the behavior the documentation currently specifies isn't the behavior the code produces. Namely, based on #40706 (comment) I would expect https://is.gd/wRatpD to panic in debug mode, but it doesn't. Nominating for libs team discussion: Can we confirm that we want to panic (I think we should) in debug?

[alexcrichton]

Yep this is just a bug, the intention is that it panics in debug mode on overflow.

[scottmcm]

Confirmed in today's nightly: https://play.rust-lang.org/?gist=3bd6bdb2ff2c5ea633f9ae3091faf5d7&version=nightly&mode=debug