[m-rev.] for review: add functions for setting and testing individual bits of uint32s

Zoltan Somogyi zoltan.somogyi at runbox.com
Fri Apr 9 22:08:03 AEST 2021


2021-04-09 21:53 GMT+10:00 "Julien Fischer" <jfischer at opturion.com>:
> +    % set_bit(U, I) = N:
> +    % N is the value obtained by setting the I'th bit of U to one.

"the i'th bit" is ambiguous, since it can be counted from
either end of the word. I would add "(the bit worth 2^i)"
after it, here and everywhere else.

> +    % unchecked_set_bit(U, I) = N:
> +    % As above, but the behaviour is undefined if I is not in [0, 31].

I would say "is not in the range [0,31]", here and elsewhere.

> +unchecked_clear_bit(U, I) =
> +    U /\ (\ (1u32 `unchecked_left_shift` cast_to_int(I))).
> +
> +
> +flip_bit(U, I) =

Delete the extra blank line.

> +set_bit(00000000000000000000000000000000, 0) = 00000000000000000000000000000001
> +set_bit(00000000000000000000000000000000, 1) = 00000000000000000000000000000010
> +set_bit(00000000000000000000000000000000, 2) = 00000000000000000000000000000100
> +set_bit(00000000000000000000000000000000, 7) = 00000000000000000000000010000000
> +set_bit(00000000000000000000000000000000, 15) = 00000000000000001000000000000000
> +set_bit(00000000000000000000000000000000, 31) = 10000000000000000000000000000000

I would print the bit index using %-2u, to make the results line up, in all the tests.

> +%---------------------------------------------------------------------------%
> +
> +:- pred run_modify_test((func(uint32, uint) = uint32)::in, string::in,
> +    io::di, io::uo) is cc_multi.
> +
> +run_modify_test(Func, Desc, !IO) :-
> +    io.format("*** Test operation '%s' ***\n\n", [s(Desc)], !IO),
> +    list.foldl(run_modify_test_2(Func, Desc), modify_numbers, !IO).
> +
> +:- pred run_modify_test_2((func(uint32, uint) = uint32)::in, string::in,
> +    uint32::in, io::di, io::uo) is cc_multi.
> +
> +run_modify_test_2(Func, Desc, U, !IO) :-
> +    list.foldl(run_modify_test_3(Func, Desc, U), bit_indexes, !IO).
> +
> +:- pred run_modify_test_3((func(uint32, uint) = uint32)::in, string::in,
> +    uint32::in, uint::in, io::di, io::uo) is cc_multi.

I would replace the _2 and _3 suffixes with meaningful suffixes,
such as _on_input and _on_input_at_index.

Otherwise, the diff is fine.

Zoltan.


More information about the reviews mailing list