Re: [PATCH 3/8] CLI: replace some constructs with more uncrustify friendly ones

[David Bremner <david@tethera.net>]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> On Thu 2019-06-13 08:08:32 -0300, David Bremner wrote:
>>    - add parens in some ternery operators
>
> itym "ternary"

yep.

>
>> @@ -120,13 +120,13 @@ _process_string_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *
>>  static int _opt_set_count (const notmuch_opt_desc_t *opt_desc)
>>  {
>>      return
>> -	!!opt_desc->opt_inherit +
>> -	!!opt_desc->opt_bool +
>> -	!!opt_desc->opt_int +
>> -	!!opt_desc->opt_keyword +
>> -	!!opt_desc->opt_flags +
>> -	!!opt_desc->opt_string +
>> -	!!opt_desc->opt_position;
>> +	(bool) opt_desc->opt_inherit +
>> +	(bool) opt_desc->opt_bool +
>> +	(bool) opt_desc->opt_int +
>> +	(bool) opt_desc->opt_keyword +
>> +	(bool) opt_desc->opt_flags +
>> +	(bool) opt_desc->opt_string +
>> +	(bool) opt_desc->opt_position;
>>  }
>
> i find this is deeply weird.  It looks like it is coercing various types
> into bools, and then summing a list of bools.
>
> While the spec might well say that the sum of two bools should be an int
> (i haven't checked), it's not at all obvious to me that the infix +
> operator should assume that type.  (float + float is a float, not an
> int, for example)

Yes, the C11 standard seems pretty clear here, 6.3.1.{1,2}

>
> in some sense, the !! operator works better here because i know that its
> output is likely to be an int, so summing makes sense.
>

For whatever reason I never used this idiom much. I _think_ it should be
replaced in new code with (bool), but I don't feel strongly about it.
There's an argument to be made that we should really just use ternary
operators there. I would accept a patch to do that if you feel strongly
enough.

d