Skip to content

std.math: add comptime_int handling for power-of-two functions #24059

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Fri3dNstuff
Copy link
Contributor

This PR implements handling for comptime_int inputs, and iN inputs, for the functions:

  • std.math.floorPowerOfTwo
  • std.math.ceilPowerOfTwo
  • std.math.ceilPowerOfTwoPromote
  • std.math.ceilPowerOfTwoAssert

Previously, all functions did not handle comptime_ints, and additionally the ceil- functions did not handle signed numbers, and asserted the input be positive - this PR changes the assertion to instead be a return of 1.

Generally, the current behaviour of the functions in the face of edge cases seems odd: why shouldn't the ceiling functions cope with non-positive inputs? And why does std.math.floorPowerOfTwo return 0 for a non-positive input: shouldn't that return an error? The existing behaviour of the floor function was not changed, due to it perhaps being dependent on, but it might be worth while rethinking.

@squeek502
Copy link
Collaborator

squeek502 commented Jun 2, 2025

If you're going to change the implementation of ceilPowerOfTwo and friends, please provide benchmarks.

You can see how the current implementation was benchmarked here: #2426 (and some more context about 0 being disallowed as an input here: #2617)

(personally, I'd prefer this PR to do one thing at a time: make comptime_int work, and then make a separate PR for the behavior changes if those are wanted)

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.

2 participants