Implement -Wduplicated-branches (PR c/64279)

Martin Sebor msebor@gmail.com
Mon Oct 24 14:59:00 GMT 2016


On 10/24/2016 08:44 AM, Marek Polacek wrote:
> On Mon, Oct 24, 2016 at 04:39:20PM +0200, Jakub Jelinek wrote:
>> On Mon, Oct 24, 2016 at 04:34:40PM +0200, Marek Polacek wrote:
>>> On Mon, Oct 24, 2016 at 08:18:25AM -0600, Martin Sebor wrote:
>>>>> The case above is just a case where with -O GCC can figure out what the value
>>>>> of a const-qualified variable is, see decl_constant_value_for_optimization.
>>>>> Since the warning is implemented even before gimplifying, optimizations like
>>>>> CP don't come into play yet.
>>>>
>>>> Ah, okay.  That should limit the number of these false positives.
>>>> (I saw -O2 in dg-options and assumed it was important.  It sounds
>>>> like the test case should pass even with -O1).
>>>
>>> Yep--even -O is enough.
>>>
>>>> But even without constant propagation there will be similar cases
>>>> (though probably less pervasive).  For instance, if j were defined
>>>> to something like this:
>>>>
>>>>   const int j = 4 == sizeof (size_t);
>>>
>>> Well, I think the warning would still be desirable:
>>>
>>>   const int j = 4 == sizeof (long);
>>>   if (j == 0)
>>>     {
>>>       if (i > 10) /* { dg-warning "this condition has identical branches" } */
>>>         *p = j * 2 + 1;
>>>       else
>>>         *p = 1;
>>>     }
>>>
>>> Given the j == 0 check, the branches really are duplicated.  This is actually
>>> a distilled version of what I found in gcov-io.c.
>>
>> Are you hashing before folding or after folding?
>
> After, e.g. I think build_modify_expr c_fully_fold operands.
>
>> If before folding, you wouldn't complain about the gcov-io.c test.
>> With folding, one can imagine something like:
>>   const int a = sizeof (long);
>>   const int b = sizeof (long long);
>>   int c;
>>   if (cond)
>>     c = a;
>>   else
>>     c = b;
>> or similar, where a and b can be the same on some targets and different on
>> others.  I believe the warning is still useful, but it will probably be
>> never false positive free.
>
> Unfortunately :(.  Maybe tolerable for -Wextra, though.

That's why I wondered about data.  I've seen examples like the one
above but it's hard to judge how common they are and what their
overall proportion is among all code like that where the warning
would be justified.  But to be clear, I don't raise this to suggest
the warning shouldn't be added or is never useful but rather in
hopes that the data might lead to insights into how to reduce the
false positive rate (if it's even a problem).  For instance, I
imagine it should be fairly easy to avoid warning on the simple
example above.

Martin



More information about the Gcc-patches mailing list