11

In some legacy code I have a lot of enums, and a huge switch cases. I would like to test that the switches have pure enum types. Nonsense example:

typedef enum EN
{
    EN_0,
    EN_1
} EN_T;

typedef enum DK
{
    DK_0,
    DK_1
} DK_T;

EN_T bar = ...
switch( bar )
{
    case EN_0:
    ...
    break;
    case DK_1: //<-- mixed type
    ...
    break;
}

I tried compiling this with gcc with -Wall -Wextra -pedantic, and get no warnings. Any ideas of how to test for this? Either as compiler warnings or dedicated test code. As both the switches and enums have 100+ members it has to be generic to some level.

Edit: Please notice I am not concerned about if this is legal c, according to the C standard.

It is bad practice, and compiler can warn about bad practice or potential errors that do not break the standard, like if( a = 1)... would always be true, perfectly legal but likely to be a mistake.

I can make the compiler warn if a switch on an enum does not contain all values of that enum a.s.o.

It is preferred if the compiler can to the work, but if a tool like lint or similar can do this I would be happy too.

Otzen
  • 540
  • 4
  • 22
  • Trying to do this in-place in the existing program is not a good idea. It is fairly simple to copy/paste the switch statement in a text file, write a program/script that knows the given enum declaration, then search for each `case` and see if the following constant is one of the listed. – Lundin Jan 16 '18 at 10:35
  • @Lundin Well easy if it is only a single switch, but I have several places with switch inside switch. So ofcourse it can be done, but it would need some parsing capability. – Otzen Jan 16 '18 at 12:48
  • You need one file that contains the enum declaration and one file that contains the switch. Hardly rocket science from there, just do simple text search for `case`. To invent something "smarter" will take longer time and this is something you'll only do once for that particular project. – Lundin Jan 16 '18 at 12:52
  • @Lundin Sorry for not making the context clear. I have many files with more big nested switches, and no I would not only do this once. I need to continuously check that someone did not break this rule. As many developers (of different skill levels) is working on the codebase . – Otzen Jan 16 '18 at 13:15
  • @Otzen: It might be an absurd idea but could you not code some of your software stack in C++, where it is possible to induce more compile-time failures? – Bathsheba Jan 16 '18 at 13:20

5 Answers5

7

No, you can't restrict switch case labels to the explicit values of a particular enum. (You can in C++ out of interest from C++11).

If you are able to change the enum values so they don't intersect, that might help you a little, but only at runtime.

Bathsheba
  • 231,907
  • 34
  • 361
  • 483
  • 1
    And now I again have to visit the standard and figure out how it's done. This is why I like your answers, and allow myself to taunt the sock bots. – StoryTeller - Unslander Monica Jan 16 '18 at 08:50
  • @StoryTeller.: I guess 6.4.2 2nd point is what we loooking for *where the constant-expression shall be a converted constant expression (5.19) of the promoted type of the switch condition. No two of the case constants in the same switch shall have the same value after conversion to the **promoted type** of the switch condition.* This is where it differs from C standard – user2736738 Jan 16 '18 at 08:52
  • *out of interest from C++11*. What is then the alternative? – JFMR Jan 16 '18 at 08:54
  • @coderredoc - The idea is to prevent promotions and implicit conversions as a whole. And it's enough to use an expression of a scoped enumeration type in the condition of the switch. So only the scoped enumerators can easily appear as case labels. I'm kinda kicking myself for needing to check :) – StoryTeller - Unslander Monica Jan 16 '18 at 08:55
  • @水飲み鳥: See this: https://stackoverflow.com/questions/18335861/why-is-enum-class-preferred-over-plain-enum – Bathsheba Jan 16 '18 at 08:56
  • @Bathsheba.: Yes now I see. Then again it is not plain enum. – user2736738 Jan 16 '18 at 08:57
  • @StoryTeller.: Ah yes..scoped enumeration is good enough. The standard part is the desired one though? – user2736738 Jan 16 '18 at 08:59
  • @coderredoc - On the contrary. It's an even plainer enum. The presence of implicit conversions and promotions is less straightforward, IMO. And yes, that's the relevant place (the conversion to the type of the controlling expression). – StoryTeller - Unslander Monica Jan 16 '18 at 09:01
  • @StoryTeller.: No promotion to other types like `int` - yes logical I would say. – user2736738 Jan 16 '18 at 09:02
  • @Bathsheba Sorry, I'm already aware of *scoped enums*. I thought you meant that there was another (superior) alternative to scoped enums (which are in turn available since C++11) for that `switch`-`case` issue as of C++11. – JFMR Jan 16 '18 at 09:08
  • @水飲み鳥: In C++, you can wrap an `enum` in a `namespace`, which can help clarity in switch labels. Although naturally this is not enforceable at compile time. – Bathsheba Jan 16 '18 at 09:09
  • @Bathsheba Is that the pre-C++11 way you are referring to? – JFMR Jan 16 '18 at 09:10
  • I am not so concerned abut the standard, if it violated the standard, the compiler would most certainly complain. I mean, AFAIK the standard allows 'if( a = true )...' But the compiler will still warn as it is bad practice. – Otzen Jan 16 '18 at 12:59
  • 2
    @Otzen: I don't know who told you that an assignment within a conditional expression is bad practice, but that's pure unbridled dogmatism. Used appropriately it is a powerful construct. Works particularly well with the I/O library. – Bathsheba Jan 16 '18 at 13:02
  • @Bathsheba Well newer the less most compiler can warn about this. – Otzen Jan 16 '18 at 13:08
3

From standard there is only one constraint so far case labeled statement

The expression of each case label shall be an integer constant expression and no two of the case constant expressions in the same switch statement shall have the same value after conversion.

As long as it is an integer constant expression it doesn't matter whether they belong to different enums or not. So yes you can't do what you want in C.

user2736738
  • 30,591
  • 5
  • 42
  • 56
0

case xxx is simple keyword with not-that-hard typical syntax. When in pinch, it should be possible to catch most occurrences it by regular expressions. First candidate for expression that comes to me is something like

(^|\s)case\s+[^:]+:
             \---/anything terminated by colon
\----/drop things like 'uppercase'

This would catch most, if not all, typical occurrences of a case keyword though the file. Then, detect switch keywords:

)\s*{\s*case\s

That should do it. Although it wouldn't look for switch keyword, it detects first closing-parenthesis that is before first case. IMHO, close enough and should work in most cases.

Being able to detect case and switch and their location, you can group cases by their preceding switch, and perform validation of case values.

That of course means you would have to write a small utility that would do that, but for me it sounds like 50-100 lines of non-minimified code.

That way of course will not handle things like:

// macros:
#define SAFE_SWITCH(x) switch(assert_non_negative(x)){
#define SWITCH_END     }

SAFE_SWITCH(..) case BAR: .... SWITCH_END

// clever hacks from bored programmers:
switch(parser.nodetype)
{
    default: throw ..;

    #include "opcodes.inc"
    #include "operands.inc"
    #include "keywords.inc"
}

etc. so that's not a perfect solution, but if your switch/case are 'clean' (no such macros, and so on) then it's worth considering.

quetzalcoatl
  • 32,194
  • 8
  • 68
  • 107
0

From the documentation on -Wswitch-enum (assuming you are using GCC): "case labels outside the enumeration range also provoke warnings when this option is used." AFAICK, this switch is not enabled by -Wall or -Wextra.

pmf
  • 7,619
  • 4
  • 47
  • 77
  • 1
    I tried that option too, it only warn if the "foreign" Enum has integer value outside the switched enum range. – Otzen Jan 17 '18 at 07:37
0

Well I will answer it myself. After some more research i conclude that at least gcc will not complain about this, I need to use an extra program like pc-lint.

I made a slight rewrite to emphasis the problem.

#include <stdio.h>

typedef enum EN
{
    ZERO,
    ONE
} EN_T;

typedef enum DK
{
    EN,  /* Danish word for One */
    TO  /* Danish word for Two */
} DK_T;

char* E2str(  EN_T en )
{
    char* ret;
    switch( en )
    {
        case ZERO:
            ret = "0";
        break;
        case TO:
            ret = "2";
        break;
    }
    return ret;
}
int main( void )
{
    printf( "0 = %s\n", E2str( ZERO ) );
    printf( "1 = %s\n", E2str( ONE ) );
    return 0;
}

This will compile fine, with no warnings even with:

gcc -o t.exe t.c -Wall -Wextra -pedantic

The output will be:

0 = 0
1 = 2

It is clear that this output was probably not the intention of the writer. And yes in this small example it is clear and obvious when just looking at the code. But imagine this being a switch with 200+ cases, and the switch contains other switches, and the naming of the enum is not as clear as in my example in the original question. It becomes close to impossible to spot errors like the one in this example.

Also note that by using -Wextra I enable a check in gcc that will warn if I have a switch on an enum, and the cases does not contain all the values in that enum. But because the TO enum has the sane numeric value as ONE, gcc doesn't even complain about missing Enums in switch, apparently it does only look at the numeric value, and not the provided enum for this check.

My test with pc-lint, spotted both

    --- Module:   t.c (C)
                   _
            case TO:
    t.c  23  Warning 408: Type mismatch with switch expression
        _
        }
    t.c  26  Info 787: enum constant 'EN::ONE' not used within switch

Unfortunately this was not the answer I was hoping for, it would be so much nicer to have this done by the compiler, and not by yet another tool.

Still open to give someone else the credit for an better answer.

Otzen
  • 540
  • 4
  • 22