zig icon indicating copy to clipboard operation
zig copied to clipboard

std.math.isPowerOfTwo: return false for 0

Open wooster0 opened this issue 1 year ago • 1 comments

I would argue that the input of 0 can happen pretty often and mathematically it makes more sense to actually return false rather than assert that the input can't be 0. We're in std.math, not std.fast.

When trying to compile

export fn x() u0 {
    return 0;
}

a debug crash happens only with a debug build of the compiler:

src/Sema.zig:23338:43: 0x9395c1 in explainWhyTypeIsNotExtern (zig)
        .Int => if (!std.math.isPowerOfTwo(ty.intInfo(sema.mod.getTarget()).bits)) {
                                          ^

In the release build of the compiler it does compile-error as expected but takes the wrong compile error branch and thus still exhibits incorrect behavior because the assert(v != 0) is not there and so isPowerOfTwo returns true for 0. Cases like this exist elsewhere in the compiler too. Of course this is something that can be fixed in the compiler itself but clearly it would be convenient for isPowerOfTwo to just correctly handle this very common input.

The user can always add an assert(x != 0) in their own code as an optimization or they can just use their own isPowerOfTwo function.

wooster0 avatar May 29 '23 00:05 wooster0

Good to have a second opinion on that. Wasn't sure about it either.

Negative integers always return false. What if we make it assert(value > 0)? Otherwise, this is good to go I think.

wooster0 avatar Jun 13 '23 21:06 wooster0

I agree that the assertion should apply to negative integers as well. Here's some prior art:

andy@ark ~> python3
Python 3.10.11 (main, Apr  4 2023, 22:10:32) [GCC 11.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import math
>>> math.log2(0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: math domain error
>>> math.log2(-1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: math domain error

andrewrk avatar Jun 17 '23 21:06 andrewrk