[cfe-commits] False positive for -Wunreachable-code

[David Blaikie]

On Tue, Oct 30, 2012 at 9:25 AM, Jordan Rose <jordan_rose at apple.com> wrote:
>
> On Oct 30, 2012, at 2:34 , Abramo Bagnara <abramo.bagnara at bugseng.com> wrote:
>
>> Il 28/10/2012 08:12, Abramo Bagnara ha scritto:
>>> $ cat p.c
>>> #include <stdio.h>
>>>
>>> enum e { a, b = 4 } x = 3;
>>>
>>> void g(int v) {
>>>  printf("%d\n", v);
>>> }
>>>
>>> int main(int argc, char **argv) {
>>>  switch (x) {
>>>  case a:
>>>    g(0);
>>>    break;
>>>  case b:
>>>    g(1);
>>>    break;
>>>  default:
>>>    g(2);
>>>    break;
>>>  }
>>> }
>>> $ _clang -Wunreachable-code -Wcovered-switch-default -O2 p.c
>>> p.c:17:3: warning: default label in switch which covers all enumeration
>>> values
>>>      [-Wcovered-switch-default]
>>>  default:
>>>  ^
>>> p.c:18:7: warning: will never be executed [-Wunreachable-code]
>>>    g(2);
>>>      ^
>>> $ ./a.out
>>> 2
>>>
>>> Of course -Wcovered-switch-default warning is a perfectly true positive.
>>>
>>> My reading of the standard is that nothing prevent an enum to have a
>>> value different from listed enum constants if this value is compatible
>>> with enum range (and code generation seems to agree on that).
>>
>> I've attached the patch for review.
>>
>> The fixed testcase shows well why to hide warnings about undefined
>> behaviour in code actually generated is a bad thing.
>
> If we do this, we're going to want this under a CFG option at the very least. The static analyzer should continue assuming that an enum input to a switch is always a valid enum constant, in order to keep our false positive rate down.

Yeah, I doubt this'll be any better in the compiler proper, really.
The heuristic exists to, as you rightly point out, reduce false
positives & that rationale exists in the compiler as well.

While, yes, it means we lose some true positives as well, that
probably isn't worth the increase in false positives.