Improve documentation for bitops #16961
Conversation
Use func Use let in runnableExamples
|
let's wait for #16622 to get merged 1st as that PR has been in the queue for a while, to avoid conflicts there |
|
|
@konsumlamm ok #16622 got merged, please rebase :) |
| importc: "__popcnt", header: "<intrin.h>", noSideEffect.} | ||
| proc builtin_popcnt64(a2: uint64): uint64 {. | ||
| importc: "__popcnt64", header: "<intrin.h>", noSideEffect.} | ||
| func builtin_popcnt16(a2: uint16): uint16 {. |
There was a problem hiding this comment.
optional: turn into more readable 1-liners?
func builtin_popcnt16(a2: uint16): uint16 {.importc: "__popcnt16", header: "<intrin.h>".}(even if exceeds that (IMO silly) 80 col limit a bit)
There was a problem hiding this comment.
let's defer this to an eventual followup PR instead
| proc parityBits*(x: SomeInteger): int {.inline, noSideEffect.} = | ||
| ## Calculate the bit parity in integer. If number of 1-bit | ||
| ## is odd parity is 1, otherwise 0. | ||
| func parityBits*(x: SomeInteger): int {.inline.} = |
There was a problem hiding this comment.
pre-existing, but why isn't this returning bool? nim isn't C...; casting should have no cost, and resulting api would be easier; maybe we need another proc that returns bool (maybe bitParity ?)
| ## otherwise result is undefined. | ||
| func firstSetBit*(x: SomeInteger): int {.inline.} = | ||
| ## Returns the 1-based index of the least significant set bit of `x`. | ||
| ## If `x` is zero, when `noUndefinedBitOpts` is set, the result is 0, |
There was a problem hiding this comment.
pre-existing but IMO future PR should add -d:nimNoUndefinedBitOpts of which noUndefinedBitOpts would become an undocumented alias
There was a problem hiding this comment.
LGTM, I've checked everything including fact that large main2 block was entirely covered by other tests block even though that's not obvious at 1st sight: some static blocks in main2 are covered by fact we have static: main
the duplication was introduced in #13803 (note that before #13803 bitops were also tested with
-d:noIntrinsicsBitOpts
-d:noUndefinedBitOps
(see https://github.com/nim-lang/Nim/pull/13803/files#diff-cc11adc098ea503d26c0788e763173c275570c6074503ebea374ed348344562c)
in future PR we should add:
discard """
matrix: "; -d:noIntrinsicsBitOpts -d:noUndefinedBitOps"
"""to test this as well
* Improve documentation for bitops Use func Use let in runnableExamples * Remove unnecessary tests Fix nim-lang#7587
func(instead ofprocwith{.noSideEffect.})letinrunnableExampleswhere possibleEDIT: Closes #7587.