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

Julien Fischer jfischer at opturion.com
Fri Apr 9 22:36:20 AEST 2021


On Fri, 9 Apr 2021, Zoltan Somogyi wrote:

> 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.

Done.

>> +    % 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.

Done; both for these additions and in a few other spots.

>> +unchecked_clear_bit(U, I) =
>> +    U /\ (\ (1u32 `unchecked_left_shift` cast_to_int(I))).
>> +
>> +
>> +flip_bit(U, I) =
>
> Delete the extra blank line.

Done.

>> +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.

Done.

>> +%---------------------------------------------------------------------------%
>> +
>> +:- 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.

Done.

> Otherwise, the diff is fine.

Thanks for that.

Julien.


More information about the reviews mailing list