[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