Re: [boost] [atomic] (op)_and_test naming

Subject: Re: [boost] [atomic] (op)_and_test naming
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2018-01-25 12:45:53

On 01/25/18 02:32, Gavin Lambert via Boost wrote:
> On 25/01/2018 00:35, Andrey Semashev wrote:
>> I would like to ask for the community opinion on the naming of the 
>> (op)_and_test functions that appeared in Boost.Atomic 1.66.
> 
> For clarity, I was the one who filed the issue mentioned in the OP.
> 
> My argument is the following:
> 
>    * "if (x)" is true when x is an integer type that is nonzero.  (And 
> this convention is often extended to non-integer types as well, for 
> suitable definitions of "zero".)
>  Â  * "atomic_flag::test_and_set" is true when the flag was previously set.
>  Â  * "atomic<T>::bit_test_and_set" is true when the bit was previously 
> 1/set.
>  Â  * "atomic<T>::fetch_add" returns the value prior to the add, which is 
> true if nonzero due to the first rule.
> 
> It thus seems peculiar to have "atomic<>::add_and_test" return true when 
> the result is zero.
> 
> 
> I can understand why this was done, as it's a natural consequence of the 
> assembly implementation that tends to operate around a "zero flag" 
> rather than a "nonzero flag", but it seems strongly counter-intuitive as 
> an interface in a higher level language.

Originally, when I picked the name, CPU flags were not my motivation. 
There are different instructions that test the flag for being set as 
well as being not set, so there's no difference from this perspective.

I guess, to me "test" may mean checking for something being zero or not 
zero depending on the context, and I found it acceptable to use this 
word alone for the sake of shorter function names. Remembering that the 
functions check the result for being zero did not seem that much of a 
problem and the intended use case seemed to favor that interpretation 
and not the opposite.

> To me at least, "test" itself implies "return true if non-zero", partly 
> as a consequence of these other things.
> 
> So "bit_test_and_set" would fundamentally mean "test if the bit is 
> non-zero, then set it and return the result of the prior test"... which 
> is indeed what it does.
> 
> And "add_and_test" would fundamentally mean "add this and return true if 
> the result is non-zero"... which is *not* what the current 
> implementation does.

So would you prefer to keep the name and change the result?