add warnings enum-to-int and int-to-enum

Commit Message

Kamil Dudka Sept. 1, 2009, 9:59 p.m. UTC

On Monday 31 of August 2009 22:53:54 Josh Triplett wrote:
> That seems quite sensible, and it even seems like something sparse
> should warn about by default.
>
> The reverse, warning about the assignment of an enum value to int, seems
> useful as well, but sparse shouldn't issue that warning by default
> because a lot of code just uses enum to define constants.
>
> Distinguishing between anonymous and named enums seems useful as well.
> An anonymous enum just creates some named constants but doesn't create a
> type to use them with, so assigning those constants to int shouldn't
> generate a warning.  (Corner case: "enum { ... } foo;".)

Here is my first take at casting from/to enum along with a simple test case. 
Any feedback welcome...

Kamil

Comments

Josh Triplett Sept. 1, 2009, 11:24 p.m. UTC | #1

On Tue, Sep 01, 2009 at 11:59:09PM +0200, Kamil Dudka wrote:
> On Monday 31 of August 2009 22:53:54 Josh Triplett wrote:
> > That seems quite sensible, and it even seems like something sparse
> > should warn about by default.
> >
> > The reverse, warning about the assignment of an enum value to int, seems
> > useful as well, but sparse shouldn't issue that warning by default
> > because a lot of code just uses enum to define constants.
> >
> > Distinguishing between anonymous and named enums seems useful as well.
> > An anonymous enum just creates some named constants but doesn't create a
> > type to use them with, so assigning those constants to int shouldn't
> > generate a warning.  (Corner case: "enum { ... } foo;".)
> 
> Here is my first take at casting from/to enum along with a simple test case. 
> Any feedback welcome...

Thanks for writing the patch.  This looks really good so far.  I have a
couple of comments on the cases you warn about and how you classify the
warnings:

> static void foo(void) {
>     enum ENUM_TYPE_A { VALUE_A } var_a;
>     enum ENUM_TYPE_B { VALUE_B } var_b;
>     enum /* anon. */ { VALUE_C } anon_enum_var;
>     int i;
> 
>     // always OK
>     var_a = VALUE_A;
>     var_a = (enum ENUM_TYPE_A) VALUE_B;
>     var_b = (enum ENUM_TYPE_B) i;
>     i = (int) VALUE_A;

Fair enough; a cast should indeed make the warning go away.  Good catch
to include the case of casting an enum value to a different enum type.
I'd also suggest including some casts of values like 0 in the "always
OK" section:

    var_a = (enum ENUM_TYPE_B) 0;

>     i = VALUE_B;

Why does this not generate a warning with Wenum-to-int?  You should have
to cast VALUE_B to int.

>     anon_enum_var = VALUE_C;
>     i = VALUE_C;
>     i = anon_enum_var;

Excellent, this represents exactly the behavior I had in mind when I
suggested the anonymous enum issue.

>     // already caught by -Wenum-mismatch (default)
>     var_a = var_b;
>     var_b = anon_enum_var;
>     anon_enum_var = var_a;
> 
>     // caught by -Wint-to-enum (default)
>     var_a = VALUE_B;
>     var_b = VALUE_C;
>     anon_enum_var = VALUE_A;

Why does Wenum-mismatch not catch these?  Because it treats those
constants as ints?  Regardless of the cause, this seems wrong.  If you
explicitly declare a variable with enum type, assigning the wrong enum
constant to it should generate a enum-mismatch warning, not an
int-to-enum warning.  However, I understand if this proves hard to fix,
and generating *some* warning seems like an improvement over the current
behavior of generating *no* warning.

>     var_a = 0;
>     var_b = i;
>     anon_enum_var = 0;
>     anon_enum_var = i;

I'd also suggest including tests with enum constants casted to integers:

    var_a = (int) VALUE_A;
    var_a = (int) VALUE_B;

>     // caught only with -Wenum-to-int
>     i = var_a;
> }

> --- a/sparse.1
> +++ b/sparse.1
> @@ -149,6 +149,18 @@ Sparse issues these warnings by default.  To turn them off, use
>  \fB\-Wno\-enum\-mismatch\fR.
>  .
>  .TP
> +.B \-Wenum\-to\-int
> +Warn about casting of an \fBenum\fR type to int and thus possible lost of
> +information about the type.

This should not say "warn about casting", because it specifically
*doesn't* warn about casting.  s/casting of/converting/.

I don't think "possible loss of information" seems like the reason to
care about this warning.  These warnings just represent ways of getting
sparse to make you treat enums as distinct types from int.  I'd suggest
dropping the second half of the sentence.

I'd also suggest adding something like "An explicit cast to int will
suppress this warning.".

> +.TP
> +.B \-Wint\-to\-enum
> +Warn about casting of int (or incompatible enumeration constant) to \fBenum\fR.

OK, so the test represents *documented* behavior, but it still doesn't
seem right. :)  The "(or incompatible enumeration constant)" bit seems
like it should become part of Wenum-mismatch.

Or if necessary, perhaps part of some new flag, like
Wenum-constant-mismatch, but that seems like overkill.

s/casting of/converting/, as above.

I'd also suggest adding "An explicit cast to the enum type will suppress
this warning.".

- Josh Triplett
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Stephen Hemminger Sept. 2, 2009, 12:27 a.m. UTC | #2

On Tue, 1 Sep 2009 16:24:33 -0700
Josh Triplett <[e-mail]> wrote:

> On Tue, Sep 01, 2009 at 11:59:09PM +0200, Kamil Dudka wrote:
> > On Monday 31 of August 2009 22:53:54 Josh Triplett wrote:
> > > That seems quite sensible, and it even seems like something sparse
> > > should warn about by default.
> > >
> > > The reverse, warning about the assignment of an enum value to int, seems
> > > useful as well, but sparse shouldn't issue that warning by default
> > > because a lot of code just uses enum to define constants.
> > >
> > > Distinguishing between anonymous and named enums seems useful as well.
> > > An anonymous enum just creates some named constants but doesn't create a
> > > type to use them with, so assigning those constants to int shouldn't
> > > generate a warning.  (Corner case: "enum { ... } foo;".)
> > 
> > Here is my first take at casting from/to enum along with a simple test case. 
> > Any feedback welcome...
> 
> Thanks for writing the patch.  This looks really good so far.  I have a
> couple of comments on the cases you warn about and how you classify the
> warnings:
> 
> > static void foo(void) {
> >     enum ENUM_TYPE_A { VALUE_A } var_a;
> >     enum ENUM_TYPE_B { VALUE_B } var_b;
> >     enum /* anon. */ { VALUE_C } anon_enum_var;
> >     int i;
> > 
> >     // always OK
> >     var_a = VALUE_A;
> >     var_a = (enum ENUM_TYPE_A) VALUE_B;
> >     var_b = (enum ENUM_TYPE_B) i;
> >     i = (int) VALUE_A;
> 
> Fair enough; a cast should indeed make the warning go away.  Good catch
> to include the case of casting an enum value to a different enum type.
> I'd also suggest including some casts of values like 0 in the "always
> OK" section:
> 
>     var_a = (enum ENUM_TYPE_B) 0;
> 
> >     i = VALUE_B;
> 
> Why does this not generate a warning with Wenum-to-int?  You should have
> to cast VALUE_B to int.
> 
> >     anon_enum_var = VALUE_C;
> >     i = VALUE_C;
> >     i = anon_enum_var;
> 
> Excellent, this represents exactly the behavior I had in mind when I
> suggested the anonymous enum issue.
> 
> >     // already caught by -Wenum-mismatch (default)
> >     var_a = var_b;
> >     var_b = anon_enum_var;
> >     anon_enum_var = var_a;
> > 
> >     // caught by -Wint-to-enum (default)
> >     var_a = VALUE_B;
> >     var_b = VALUE_C;
> >     anon_enum_var = VALUE_A;
> 
> Why does Wenum-mismatch not catch these?  Because it treats those
> constants as ints?  Regardless of the cause, this seems wrong.  If you
> explicitly declare a variable with enum type, assigning the wrong enum
> constant to it should generate a enum-mismatch warning, not an
> int-to-enum warning.  However, I understand if this proves hard to fix,
> and generating *some* warning seems like an improvement over the current
> behavior of generating *no* warning.
> 
> >     var_a = 0;
> >     var_b = i;
> >     anon_enum_var = 0;
> >     anon_enum_var = i;
> 
> I'd also suggest including tests with enum constants casted to integers:
> 
>     var_a = (int) VALUE_A;
>     var_a = (int) VALUE_B;
> 
> >     // caught only with -Wenum-to-int
> >     i = var_a;
> > }
> 
> > --- a/sparse.1
> > +++ b/sparse.1
> > @@ -149,6 +149,18 @@ Sparse issues these warnings by default.  To turn them off, use
> >  \fB\-Wno\-enum\-mismatch\fR.
> >  .
> >  .TP
> > +.B \-Wenum\-to\-int
> > +Warn about casting of an \fBenum\fR type to int and thus possible lost of
> > +information about the type.
> 
> This should not say "warn about casting", because it specifically
> *doesn't* warn about casting.  s/casting of/converting/.
> 
> I don't think "possible loss of information" seems like the reason to
> care about this warning.  These warnings just represent ways of getting
> sparse to make you treat enums as distinct types from int.  I'd suggest
> dropping the second half of the sentence.
> 
> I'd also suggest adding something like "An explicit cast to int will
> suppress this warning.".
> 
> > +.TP
> > +.B \-Wint\-to\-enum
> > +Warn about casting of int (or incompatible enumeration constant) to \fBenum\fR.
> 
> OK, so the test represents *documented* behavior, but it still doesn't
> seem right. :)  The "(or incompatible enumeration constant)" bit seems
> like it should become part of Wenum-mismatch.
> 
> Or if necessary, perhaps part of some new flag, like
> Wenum-constant-mismatch, but that seems like overkill.
> 
> s/casting of/converting/, as above.
> 
> I'd also suggest adding "An explicit cast to the enum type will suppress
> this warning.".
> 
> - Josh Triplett


there is lots of code that does:

enum {
   my_register_1   = 0x001,
   my_register_2   = 0x002,
};


foo(void) {
   write_register(my_register_1, SETUP_VAL_X);
...

So going from enum to int must be optional, but int to enum should
trigger a warning.

I'll give it a pass on the kernel for giggles.

Daniel Barkalow Sept. 2, 2009, 5:56 p.m. UTC | #3

On Tue, 1 Sep 2009, Stephen Hemminger wrote:

> there is lots of code that does:
> 
> enum {
>    my_register_1   = 0x001,
>    my_register_2   = 0x002,
> };

It feels to me like the explicit numeric values are what make these 
constants sensible to use directly as ints, and that it's only sensible to 
use a non-constant value of an enum type as an int (without an explicit 
cast) if all of the enum values have explicit numeric values.

I think:

  enum {
    my_register_zero
    ...
    my_register_twdr
    my_register_twcr
    ...
  };

  void () {
    write_register(my_register_twdr, SETUP_TWDR);
  }

is asking for trouble in a way that this warning is about.

	-Daniel
*This .sig left intentionally blank*
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Kamil Dudka Sept. 2, 2009, 6:04 p.m. UTC | #4

On Wednesday 02 of September 2009 19:56:47 Daniel Barkalow wrote:
> It feels to me like the explicit numeric values are what make these
> constants sensible to use directly as ints, and that it's only sensible to
> use a non-constant value of an enum type as an int (without an explicit
> cast) if all of the enum values have explicit numeric values.
>
> I think:
>
>   enum {
>     my_register_zero
>     ...
>     my_register_twdr
>     my_register_twcr
>     ...
>   };
>
>   void () {
>     write_register(my_register_twdr, SETUP_TWDR);
>   }
>
> is asking for trouble in a way that this warning is about.

Both examples are too abstract for me -- missing declaration
of write_register(), etc. Please attach a minimal example as a file which I 
can compile and test. I'll check if the "trouble" is covered by the warnings 
or not, and perhaps implement what's missing. Thanks in advance!

Kamil
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Daniel Barkalow Sept. 2, 2009, 6:43 p.m. UTC | #5

On Wed, 2 Sep 2009, Kamil Dudka wrote:

> On Wednesday 02 of September 2009 19:56:47 Daniel Barkalow wrote:
> > It feels to me like the explicit numeric values are what make these
> > constants sensible to use directly as ints, and that it's only sensible to
> > use a non-constant value of an enum type as an int (without an explicit
> > cast) if all of the enum values have explicit numeric values.
> >
> > I think:
> >
> >   enum {
> >     my_register_zero
> >     ...
> >     my_register_twdr
> >     my_register_twcr
> >     ...
> >   };
> >
> >   void () {
> >     write_register(my_register_twdr, SETUP_TWDR);
> >   }
> >
> > is asking for trouble in a way that this warning is about.
> 
> Both examples are too abstract for me -- missing declaration
> of write_register(), etc. Please attach a minimal example as a file which I 
> can compile and test. I'll check if the "trouble" is covered by the warnings 
> or not, and perhaps implement what's missing. Thanks in advance!

enum {
  foo,
  bar
};

enum {
  baz = 1,
  qux = 2
};

void test(void) {
  int i = bar; // warn on this
  int j = qux; // okay
}

(Leaving aside the issue of whether the enum is anonymous)

In the "bar" case, an additional value added somewhere in the list 
(particularly if the list were long) might change "i" in a way that 
wouldn't necessarily be obvious to users of "i". In the "qux" case, "j" 
would only change if the "qux = 2" line were changed.

	-Daniel
*This .sig left intentionally blank*
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Josh Triplett Sept. 2, 2009, 6:56 p.m. UTC | #6

On Wed, Sep 02, 2009 at 02:43:52PM -0400, Daniel Barkalow wrote:
> On Wed, 2 Sep 2009, Kamil Dudka wrote:
> 
> > On Wednesday 02 of September 2009 19:56:47 Daniel Barkalow wrote:
> > > It feels to me like the explicit numeric values are what make these
> > > constants sensible to use directly as ints, and that it's only sensible to
> > > use a non-constant value of an enum type as an int (without an explicit
> > > cast) if all of the enum values have explicit numeric values.
> > >
> > > I think:
> > >
> > >   enum {
> > >     my_register_zero
> > >     ...
> > >     my_register_twdr
> > >     my_register_twcr
> > >     ...
> > >   };
> > >
> > >   void () {
> > >     write_register(my_register_twdr, SETUP_TWDR);
> > >   }
> > >
> > > is asking for trouble in a way that this warning is about.
> > 
> > Both examples are too abstract for me -- missing declaration
> > of write_register(), etc. Please attach a minimal example as a file which I 
> > can compile and test. I'll check if the "trouble" is covered by the warnings 
> > or not, and perhaps implement what's missing. Thanks in advance!
> 
> enum {
>   foo,
>   bar
> };
> 
> enum {
>   baz = 1,
>   qux = 2
> };
> 
> void test(void) {
>   int i = bar; // warn on this
>   int j = qux; // okay
> }
> 
> (Leaving aside the issue of whether the enum is anonymous)
> 
> In the "bar" case, an additional value added somewhere in the list 
> (particularly if the list were long) might change "i" in a way that 
> wouldn't necessarily be obvious to users of "i". In the "qux" case, "j" 
> would only change if the "qux = 2" line were changed.

I disagree with this.  In both cases, you've declared an anonymous enum,
and then used its values as constants.  In the former case, you might
just not care about the values except to compare against each other;
granted, you ought to use a named enum rather than an int in that case,
but nevertheless much code exists that uses int instead of enum types.

If sparse warned about using the values from an anonymous enum if not
declared with an explicit value, it might as well just warn about the
declaration of the enum in the first place, since you can't do anything
useful with it other than using its values as constants.  That seems
like a warning Sparse could have, with a separate warning flag
(-Wanonymous-enum-implicit-value or similar); however, I don't think
that warning should appear by default, nor should it appear as part of
one of the existing flags under discussion.  I don't think it seems
very sensible to have at all, really, but *shrug*.

- Josh Triplett
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Daniel Barkalow Sept. 2, 2009, 7:19 p.m. UTC | #7

On Wed, 2 Sep 2009, Josh Triplett wrote:

> On Wed, Sep 02, 2009 at 02:43:52PM -0400, Daniel Barkalow wrote:
> > On Wed, 2 Sep 2009, Kamil Dudka wrote:
> > 
> > > On Wednesday 02 of September 2009 19:56:47 Daniel Barkalow wrote:
> > > > It feels to me like the explicit numeric values are what make these
> > > > constants sensible to use directly as ints, and that it's only sensible to
> > > > use a non-constant value of an enum type as an int (without an explicit
> > > > cast) if all of the enum values have explicit numeric values.
> > > >
> > > > I think:
> > > >
> > > >   enum {
> > > >     my_register_zero
> > > >     ...
> > > >     my_register_twdr
> > > >     my_register_twcr
> > > >     ...
> > > >   };
> > > >
> > > >   void () {
> > > >     write_register(my_register_twdr, SETUP_TWDR);
> > > >   }
> > > >
> > > > is asking for trouble in a way that this warning is about.
> > > 
> > > Both examples are too abstract for me -- missing declaration
> > > of write_register(), etc. Please attach a minimal example as a file which I 
> > > can compile and test. I'll check if the "trouble" is covered by the warnings 
> > > or not, and perhaps implement what's missing. Thanks in advance!
> > 
> > enum {
> >   foo,
> >   bar
> > };
> > 
> > enum {
> >   baz = 1,
> >   qux = 2
> > };
> > 
> > void test(void) {
> >   int i = bar; // warn on this
> >   int j = qux; // okay
> > }
> > 
> > (Leaving aside the issue of whether the enum is anonymous)
> > 
> > In the "bar" case, an additional value added somewhere in the list 
> > (particularly if the list were long) might change "i" in a way that 
> > wouldn't necessarily be obvious to users of "i". In the "qux" case, "j" 
> > would only change if the "qux = 2" line were changed.
> 
> I disagree with this.  In both cases, you've declared an anonymous enum,
> and then used its values as constants.  In the former case, you might
> just not care about the values except to compare against each other;
> granted, you ought to use a named enum rather than an int in that case,
> but nevertheless much code exists that uses int instead of enum types.

I think anonymous vs named enums are one thing that should affect whether 
you get a warning, and explicit values vs implicit values are another 
factor, but I don't have an opinion on which way they should combine. 
Probably either using an explicit value or an anonymous enum should be 
okay by default, and the test above should use named enums.

	-Daniel
*This .sig left intentionally blank*
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Kamil Dudka Sept. 2, 2009, 7:58 p.m. UTC | #8

On Wednesday 02 of September 2009 21:19:46 Daniel Barkalow wrote:
> On Wed, 2 Sep 2009, Josh Triplett wrote:
> > On Wed, Sep 02, 2009 at 02:43:52PM -0400, Daniel Barkalow wrote:
> > > On Wed, 2 Sep 2009, Kamil Dudka wrote:
> > > > On Wednesday 02 of September 2009 19:56:47 Daniel Barkalow wrote:
> > > > > It feels to me like the explicit numeric values are what make these
> > > > > constants sensible to use directly as ints, and that it's only
> > > > > sensible to use a non-constant value of an enum type as an int
> > > > > (without an explicit cast) if all of the enum values have explicit
> > > > > numeric values.
> > > > >
> > > > > I think:
> > > > >
> > > > >   enum {
> > > > >     my_register_zero
> > > > >     ...
> > > > >     my_register_twdr
> > > > >     my_register_twcr
> > > > >     ...
> > > > >   };
> > > > >
> > > > >   void () {
> > > > >     write_register(my_register_twdr, SETUP_TWDR);
> > > > >   }
> > > > >
> > > > > is asking for trouble in a way that this warning is about.
> > > >
> > > > Both examples are too abstract for me -- missing declaration
> > > > of write_register(), etc. Please attach a minimal example as a file
> > > > which I can compile and test. I'll check if the "trouble" is covered
> > > > by the warnings or not, and perhaps implement what's missing. Thanks
> > > > in advance!
> > >
> > > enum {
> > >   foo,
> > >   bar
> > > };
> > >
> > > enum {
> > >   baz = 1,
> > >   qux = 2
> > > };
> > >
> > > void test(void) {
> > >   int i = bar; // warn on this
> > >   int j = qux; // okay
> > > }
> > >
> > > (Leaving aside the issue of whether the enum is anonymous)
> > >
> > > In the "bar" case, an additional value added somewhere in the list
> > > (particularly if the list were long) might change "i" in a way that
> > > wouldn't necessarily be obvious to users of "i". In the "qux" case, "j"
> > > would only change if the "qux = 2" line were changed.
> >
> > I disagree with this.  In both cases, you've declared an anonymous enum,
> > and then used its values as constants.  In the former case, you might
> > just not care about the values except to compare against each other;
> > granted, you ought to use a named enum rather than an int in that case,
> > but nevertheless much code exists that uses int instead of enum types.
>
> I think anonymous vs named enums are one thing that should affect whether
> you get a warning, and explicit values vs implicit values are another
> factor, but I don't have an opinion on which way they should combine.
> Probably either using an explicit value or an anonymous enum should be
> okay by default, and the test above should use named enums.

I think type-checking is the way to go, not values checking... Once you start 
to count with "explicit" vs. "implicit" enum values, you will sooner or later 
run in troubles like this:

enum { A, B = 3, C, D = A + 7 };
enum { Z = (C < D) ? A : B };

Kamil
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

From a5095bbdb9694effa4a771295a87b80bf84d3230 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <[e-mail]>
Date: Tue, 1 Sep 2009 23:51:29 +0200
Subject: [PATCH] add warnings enum-to-int and int-to-enum


Signed-off-by: Kamil Dudka <[e-mail]>
---
 evaluate.c   |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 expression.h |    1 +
 lib.c        |    4 +++
 lib.h        |    2 +
 parse.c      |    1 +
 sparse.1     |   12 ++++++++++
 6 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index 805ae90..47e50fb 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -235,16 +235,23 @@  static int is_same_type(struct expression *expr, struct symbol *new)
 }
 
 static void
+resolve_sym_node (struct symbol **psym)
+{
+	struct symbol *sym = *psym;
+	if (sym->type == SYM_NODE)
+		*psym = sym->ctype.base_type;
+}
+
+static void
 warn_for_different_enum_types (struct position pos,
 			       struct symbol *typea,
 			       struct symbol *typeb)
 {
 	if (!Wenum_mismatch)
 		return;
-	if (typea->type == SYM_NODE)
-		typea = typea->ctype.base_type;
-	if (typeb->type == SYM_NODE)
-		typeb = typeb->ctype.base_type;
+
+	resolve_sym_node(&typea);
+	resolve_sym_node(&typeb);
 
 	if (typea == typeb)
 		return;
@@ -256,6 +263,54 @@  warn_for_different_enum_types (struct position pos,
 	}
 }
 
+static void
+warn_for_enum_to_int_cast (struct expression *expr, struct symbol *typeb)
+{
+	struct position pos = expr->pos;
+	struct symbol *typea = expr->ctype;
+
+	if (!Wenum_to_int)
+		return;
+
+	resolve_sym_node(&typea);
+	resolve_sym_node(&typeb);
+
+	if (typea->type != SYM_ENUM || typeb->type != SYM_BASETYPE)
+		return;
+
+	if (typea->ident) {
+		warning(pos, "cast from");
+		info(pos, "    %s to", show_typename(typea));
+		info(pos, "    %s", show_typename(typeb));
+		return;
+	}
+}
+
+static void
+warn_for_int_to_enum_cast (struct expression *expr, struct symbol *typeb)
+{
+	struct position pos = expr->pos;
+	struct symbol *typea = expr->ctype;
+	struct symbol *enum_type = expr->enum_type;
+
+	if (!Wint_to_enum)
+		return;
+
+	resolve_sym_node(&typea);
+	resolve_sym_node(&typeb);
+
+	if (typea->type != SYM_BASETYPE || typeb->type != SYM_ENUM)
+		return;
+
+	if (!enum_type || (enum_type != typeb)) {
+		warning(pos, "cast from");
+		info(pos, "    %s to", show_typename((enum_type)
+					? enum_type
+					: typea));
+		info(pos, "    %s", show_typename(typeb));
+	}
+}
+
 /*
  * This gets called for implicit casts in assignments and
  * integer promotion. We often want to try to move the
@@ -268,6 +323,8 @@  static struct expression * cast_to(struct expression *old, struct symbol *type)
 	struct expression *expr;
 
 	warn_for_different_enum_types (old->pos, old->ctype, type);
+	warn_for_enum_to_int_cast (old, type);
+	warn_for_int_to_enum_cast (old, type);
 
 	if (old->ctype != &null_ctype && is_same_type(old, type))
 		return old;
diff --git a/expression.h b/expression.h
index 631224f..81f70ad 100644
--- a/expression.h
+++ b/expression.h
@@ -70,6 +70,7 @@  struct expression {
 		struct {
 			unsigned long long value;
 			unsigned taint;
+			struct symbol *enum_type;
 		};
 
 		// EXPR_FVALUE
diff --git a/lib.c b/lib.c
index 600939b..2f78bd5 100644
--- a/lib.c
+++ b/lib.c
@@ -199,6 +199,8 @@  int Wdecl = 1;
 int Wdefault_bitfield_sign = 0;
 int Wdo_while = 0;
 int Wenum_mismatch = 1;
+int Wenum_to_int = 0;
+int Wint_to_enum = 1;
 int Wnon_pointer_null = 1;
 int Wold_initializer = 1;
 int Wone_bit_signed_bitfield = 1;
@@ -380,6 +382,8 @@  static const struct warning {
 	{ "default-bitfield-sign", &Wdefault_bitfield_sign },
 	{ "do-while", &Wdo_while },
 	{ "enum-mismatch", &Wenum_mismatch },
+	{ "enum-to-int", &Wenum_to_int },
+	{ "int-to-enum", &Wint_to_enum },
 	{ "non-pointer-null", &Wnon_pointer_null },
 	{ "old-initializer", &Wold_initializer },
 	{ "one-bit-signed-bitfield", &Wone_bit_signed_bitfield },
diff --git a/lib.h b/lib.h
index b22fa93..962d49d 100644
--- a/lib.h
+++ b/lib.h
@@ -96,6 +96,8 @@  extern int Wdecl;
 extern int Wdefault_bitfield_sign;
 extern int Wdo_while;
 extern int Wenum_mismatch;
+extern int Wenum_to_int;
+extern int Wint_to_enum;
 extern int Wnon_pointer_null;
 extern int Wold_initializer;
 extern int Wone_bit_signed_bitfield;
diff --git a/parse.c b/parse.c
index 5e75242..76d4c58 100644
--- a/parse.c
+++ b/parse.c
@@ -791,6 +791,7 @@  static void cast_enum_list(struct symbol_list *list, struct symbol *base_type)
 		if (expr->type != EXPR_VALUE)
 			continue;
 		ctype = expr->ctype;
+		expr->enum_type = sym->ctype.base_type;
 		if (ctype->bit_size == base_type->bit_size)
 			continue;
 		cast_value(expr, base_type, expr, ctype);
diff --git a/sparse.1 b/sparse.1
index d7fe444..7eb0cda 100644
--- a/sparse.1
+++ b/sparse.1
@@ -149,6 +149,18 @@  Sparse issues these warnings by default.  To turn them off, use
 \fB\-Wno\-enum\-mismatch\fR.
 .
 .TP
+.B \-Wenum\-to\-int
+Warn about casting of an \fBenum\fR type to int and thus possible lost of
+information about the type.
+.
+.TP
+.B \-Wint\-to\-enum
+Warn about casting of int (or incompatible enumeration constant) to \fBenum\fR.
+
+Sparse issues these warnings by default.  To turn them off, use
+\fB\-Wno\-int\-to\-enum\fR.
+.
+.TP
 .B \-Wnon\-pointer\-null
 Warn about the use of 0 as a NULL pointer.
 
-- 
1.6.4.1