[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