bug-indent
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Regression in 2.2.11


From: indent
Subject: Re: Regression in 2.2.11
Date: Mon, 05 Apr 2010 21:41:43 +0200
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-GB; rv:1.9.1.8) Gecko/20100228 SUSE/3.0.3-1.1.1 Thunderbird/3.0.3

Indeed your described behaviour is a defined part of C, however I'd say
that such a usage falls under the category of 'cleverness' - where most
programmers would need to reach for the manual to verify what the result
would be and what the intention is. See for example what "Writing Solid
Code" - Steve Maguire has to say on the subject (basically "avoid
cleverness"). Since your expression is two applications of an operator
(and not a multi-character operator) I would say that in this regard
version 2.2.11 doesn't regress, but (unintentionally) fixes a
long-standing defect. The space makes the fact that it is two separate
applications of an operator clear. Your code would become much more
readable if you had

#define TO_BOOLEAN(x)   (! !(x))

...

int
foo(int a)
{
  return TO_BOOLEAN(a);
}


My opinions aside,  why should I accept an (undocumented) patch to
indent that singles out your special case for special treatment? The
submitter of the '- --' update didn't know of your special case and a
reader of the code containing your update will also wonder why the test
for '!' is there. You say "simple", but that only applies to the
implementation, not the subsequent examination and maintenance of the
code. Why, for example aren't other unary operators addressed?

BTW I do recognise your particular problem, I just don't agree with your
workaround (whereas the "- --" fix /had/ to be done to avoid a situation
which could result in indent generating  buggy code).

David.

On 05/04/10 10:37, Edward Hervey wrote:
> Hi,
>
>   This construct is used quite a bit in GStreamer (where we are using
> indent as a git pre-commit script to make sure all code is indented in a
> unified fashion). If that construct wasn't allowed, considering the
> number of esoteric platforms it compiles on... we'd know by now if it
> wasn't valid C :)
>
>   The result of '!!' ensures the result can only be 1 or 0.
>
>    Edward
>
> On Mon, 2010-04-05 at 10:24 +0200, indent wrote:
>   
>> I am very baffled as to why anyone would want to write "return !!a". Are
>> you sure you're writing C and not some other language?
>>
>> On 03/04/10 21:00, Edward Hervey wrote:
>>     
>>> Hi,
>>>
>>>   I found a regression in indent 2.2.11.
>>>
>>>   The following code
>>> a = !!b;
>>>   Now becomes
>>> a = ! !b;
>>>
>>>   This used to work perfectly fine with most versions of indent prior to
>>> 2.2.11.
>>>
>>>   The regression was introduced with the 'fix' for avoiding "- --a"
>>> becomeing "---a".
>>>
>>>   A simple fix for this would be to check if the token in question is
>>> not the '!' operator:
>>>
>>> Line 616:
>>> -------------
>>>             if ((parser_state_tos->last_token == unary_op) &&
>>> -               (e_code > s_code) &&
>>> +               (e_code > s_code) && (*res != '!') &&
>>>                 (*(e_code - 1) == *res))
>>>             {
>>>                 *(e_code++) = ' ';
>>>             }
>>> --------------
>>>
>>> Sample test for regression:
>>> ------------------
>>> int
>>> foo(int a)
>>> {
>>>   return !!a;
>>> }
>>> ------------------
>>>
>>>    Edward
>>>
>>>
>>>
>>> _______________________________________________
>>> bug-indent mailing list
>>> address@hidden
>>> http://lists.gnu.org/mailman/listinfo/bug-indent
>>>
>>>   
>>>       
>>     
>
>
>   



reply via email to

[Prev in Thread] Current Thread [Next in Thread]