-Wenum-compare doesn't catch mismatched enum constant comparison in C mode

[llvmbot]

Bugzilla Link	28847
Version	trunk
OS	All
Reporter	LLVM Bugzilla Contributor
CC	@mmdriley

Extended Description

Consider this program (test.c)

enum foo_t {
    foo1, foo2, foo3
};

enum bar_t {
    bar1, bar2, bar3
};

int main(void) {
    enum foo_t foo = foo1;
    enum bar_t bar = bar1;

    int cmp1 = foo < bar; // variable - variable
    int cmp2 = foo < bar2; // variable - constant
    int cmp3 = foo1 < bar3; // constant - constant

    return cmp1 || cmp2 || cmp3;
}

If I compile this as C code, I only get a warning for the first comparison. If I compile in C++ mode, I get a warning for all three.

$ clang -Wall test.c
test.c:13:20: warning: comparison of two values with different enumeration types
      ('enum foo_t' and 'enum bar_t') [-Wenum-compare]
    int cmp1 = foo < bar;
               ~~~ ^ ~~~
1 warning generated.
$ clang -Wall -x c++ test.c
test.c:13:20: warning: comparison of two values with different enumeration types
      ('enum foo_t' and 'enum bar_t') [-Wenum-compare]
    int cmp1 = foo < bar;
               ~~~ ^ ~~~
test.c:14:20: warning: comparison of two values with different enumeration types
      ('enum foo_t' and 'bar_t') [-Wenum-compare]
    int cmp2 = foo < bar2;
               ~~~ ^ ~~~~
test.c:15:21: warning: comparison of two values with different enumeration types ('foo_t' and 'bar_t')
      [-Wenum-compare]
    int cmp3 = foo1 < bar3;
               ~~~~ ^ ~~~~
3 warnings generated.

I think the warnings should be emitted in C mode too. Tested with gcc 4.9.2 and 3 warnings are emitted for C code:

$ gcc -Wall test.c
test.c: In function 'main':
test.c:13:20: warning: comparison between 'enum foo_t' and 'enum bar_t' [-Wenum-compare]
     int cmp1 = foo < bar;
                    ^
test.c:14:20: warning: comparison between 'enum foo_t' and 'enum bar_t' [-Wenum-compare]
     int cmp2 = foo < bar2;
                    ^
test.c:15:21: warning: comparison between 'enum foo_t' and 'enum bar_t' [-Wenum-compare]
     int cmp3 = foo1 < bar3;
                     ^

[mmdriley]

This diagnostic is implemented by checkEnumComparison, which checks for comparisons between two different enum types: https://github.com/llvm-mirror/clang/blob/2bce94da8a363b4ffbd7ba2d6a1941942f9bac04/lib/Sema/SemaExpr.cpp#L9446-L9472

---

Here's what -Xclang -ast-dump gives us for the cmpN assignments in C++:

|-DeclStmt 0x55de80d391f0 <line:13:5, col:25>
| -VarDecl 0x55de80d38cf8 <col:5, col:22> col:9 used cmp1 'int' cinit | -ImplicitCastExpr 0x55de80d391d8 <col:16, col:22> 'int'
| -BinaryOperator 0x55de80d391b0 <col:16, col:22> 'bool' '<' | |-ImplicitCastExpr 0x55de80d38dd8 <col:16> 'int' <IntegralCast> | | -ImplicitCastExpr 0x55de80d38da8 col:16 'enum foo_t':'foo_t'
| | -DeclRefExpr 0x55de80d38d58 <col:16> 'enum foo_t':'foo_t' lvalue Var 0x55de80d38b50 'foo' 'enum foo_t':'foo_t' | -ImplicitCastExpr 0x55de80d38df0 col:22 'int'
| -ImplicitCastExpr 0x55de80d38dc0 <col:22> 'enum bar_t':'bar_t' <LValueToRValue> | -DeclRefExpr 0x55de80d38d80 col:22 'enum bar_t':'bar_t' lvalue Var 0x55de80d38c40 'bar' 'enum bar_t':'bar_t'
|-DeclStmt 0x55de80d39358 <line:14:5, col:26>
| -VarDecl 0x55de80d39220 <col:5, col:22> col:9 used cmp2 'int' cinit | -ImplicitCastExpr 0x55de80d39340 <col:16, col:22> 'int'
| -BinaryOperator 0x55de80d39318 <col:16, col:22> 'bool' '<' | |-ImplicitCastExpr 0x55de80d392e8 <col:16> 'int' <IntegralCast> | | -ImplicitCastExpr 0x55de80d392d0 col:16 'enum foo_t':'foo_t'
| | -DeclRefExpr 0x55de80d39280 <col:16> 'enum foo_t':'foo_t' lvalue Var 0x55de80d38b50 'foo' 'enum foo_t':'foo_t' | -ImplicitCastExpr 0x55de80d39300 col:22 'int'
| -DeclRefExpr 0x55de80d392a8 <col:22> 'bar_t' EnumConstant 0x55de80d38898 'bar2' 'bar_t' |-DeclStmt 0x55de80d394a8 <line:15:5, col:27> | -VarDecl 0x55de80d39388 <col:5, col:23> col:9 used cmp3 'int' cinit
| -ImplicitCastExpr 0x55de80d39490 <col:16, col:23> 'int' <IntegralCast> | -BinaryOperator 0x55de80d39468 <col:16, col:23> 'bool' '<'
| |-ImplicitCastExpr 0x55de80d39438 col:16 'int'
| | -DeclRefExpr 0x55de80d393e8 <col:16> 'foo_t' EnumConstant 0x55de80d386c0 'foo1' 'foo_t' | -ImplicitCastExpr 0x55de80d39450 col:23 'int'
| `-DeclRefExpr 0x55de80d39410 col:23 'bar_t' EnumConstant 0x55de80d388e0 'bar3' 'bar_t'

vs. in C:

|-DeclStmt 0x55db7c58fa58 <line:13:5, col:25>
| -VarDecl 0x55db7c58f580 <col:5, col:22> col:9 used cmp1 'int' cinit | -BinaryOperator 0x55db7c58fa30 <col:16, col:22> 'int' '<'
| |-ImplicitCastExpr 0x55db7c58f660 col:16 'unsigned int'
| | -ImplicitCastExpr 0x55db7c58f630 <col:16> 'enum foo_t':'enum foo_t' <LValueToRValue> | | -DeclRefExpr 0x55db7c58f5e0 col:16 'enum foo_t':'enum foo_t' lvalue Var 0x55db7c58f3a0 'foo' 'enum foo_t':'enum foo_t'
| -ImplicitCastExpr 0x55db7c58f678 <col:22> 'unsigned int' <IntegralCast> | -ImplicitCastExpr 0x55db7c58f648 col:22 'enum bar_t':'enum bar_t'
| -DeclRefExpr 0x55db7c58f608 <col:22> 'enum bar_t':'enum bar_t' lvalue Var 0x55db7c58f4b0 'bar' 'enum bar_t':'enum bar_t' |-DeclStmt 0x55db7c58fba8 <line:14:5, col:26> | -VarDecl 0x55db7c58fa88 <col:5, col:22> col:9 used cmp2 'int' cinit
| -BinaryOperator 0x55db7c58fb80 <col:16, col:22> 'int' '<' | |-ImplicitCastExpr 0x55db7c58fb50 <col:16> 'unsigned int' <IntegralCast> | | -ImplicitCastExpr 0x55db7c58fb38 col:16 'enum foo_t':'enum foo_t'
| | -DeclRefExpr 0x55db7c58fae8 <col:16> 'enum foo_t':'enum foo_t' lvalue Var 0x55db7c58f3a0 'foo' 'enum foo_t':'enum foo_t' | -ImplicitCastExpr 0x55db7c58fb68 col:22 'unsigned int'
| -DeclRefExpr 0x55db7c58fb10 <col:22> 'int' EnumConstant 0x55db7c58f0e8 'bar2' 'int' |-DeclStmt 0x55db7c58fcb0 <line:15:5, col:27> | -VarDecl 0x55db7c58fbd8 <col:5, col:23> col:9 used cmp3 'int' cinit
| -BinaryOperator 0x55db7c58fc88 <col:16, col:23> 'int' '<' | |-DeclRefExpr 0x55db7c58fc38 <col:16> 'int' EnumConstant 0x55db7c58ef10 'foo1' 'int' | -DeclRefExpr 0x55db7c58fc60 col:23 'int' EnumConstant 0x55db7c58f130 'bar3' 'int'

---

In C++, the enumeration constants take the type of their enumeration. That's http://eel.is/c++draft/dcl.enum#5 and is implemented here: https://github.com/llvm-mirror/clang/blob/2bce94da/lib/Sema/SemaDecl.cpp#L16675-L16679

In C, though, enumeration constants have type int (C18 6.4.4.3). We see that in the ASTs above, and it explains why checkEnumComparison doesn't see anything wrong with comparisons that include an enumeration constant.

The Clang diagnostic for implicit conversion between Enum types has explicit logic (added in llvm-mirror/clang@5a5b38f4a) to see "through" EnumConstantDecls to their enumeration type in C.

The solution here is probably to factor that logic out of CheckImplicitConversion and use it in checkEnumComparison too.

[llvmbot]

@llvm/issue-subscribers-c

Author: None (llvmbot)

| | | | --- | --- | | Bugzilla Link | [28847](https://llvm.org/bz28847) | | Version | trunk | | OS | All | | Reporter | LLVM Bugzilla Contributor | | CC | @mmdriley |

Extended Description

Consider this program (test.c)

enum foo_t {
foo1, foo2, foo3
};

enum bar_t {
bar1, bar2, bar3
};

int main(void) {
enum foo_t foo = foo1;
enum bar_t bar = bar1;

int cmp1 = foo < bar; // variable - variable
int cmp2 = foo < bar2; // variable - constant
int cmp3 = foo1 < bar3; // constant - constant

return cmp1 || cmp2 || cmp3;

}

If I compile this as C code, I only get a warning for the first comparison. If I compile in C++ mode, I get a warning for all three.

$ clang -Wall test.c
test.c:13:20: warning: comparison of two values with different enumeration types
('enum foo_t' and 'enum bar_t') [-Wenum-compare]
int cmp1 = foo < bar;
~~~ ^ ~~~
1 warning generated.
$ clang -Wall -x c++ test.c
test.c:13:20: warning: comparison of two values with different enumeration types
('enum foo_t' and 'enum bar_t') [-Wenum-compare]
int cmp1 = foo < bar;
~~~ ^ ~~~
test.c:14:20: warning: comparison of two values with different enumeration types
('enum foo_t' and 'bar_t') [-Wenum-compare]
int cmp2 = foo < bar2;
~~~ ^ ~~~~
test.c:15:21: warning: comparison of two values with different enumeration types ('foo_t' and 'bar_t')
[-Wenum-compare]
int cmp3 = foo1 < bar3;
~~~~ ^ ~~~~
3 warnings generated.

I think the warnings should be emitted in C mode too. Tested with gcc 4.9.2 and 3 warnings are emitted for C code:

$ gcc -Wall test.c
test.c: In function 'main':
test.c:13:20: warning: comparison between 'enum foo_t' and 'enum bar_t' [-Wenum-compare]
int cmp1 = foo < bar;
^
test.c:14:20: warning: comparison between 'enum foo_t' and 'enum bar_t' [-Wenum-compare]
int cmp2 = foo < bar2;
^
test.c:15:21: warning: comparison between 'enum foo_t' and 'enum bar_t' [-Wenum-compare]
int cmp3 = foo1 < bar3;
^