On Wednesday 02 of September 2009 17:21:46 Josh Triplett wrote:
> Typo. I meant ENUM_TYPE_A:
>
> var_a = (enum ENUM_TYPE_A) 0;
>
> Hopefully that makes more sense as an "always OK" test. :)
>
> Another crazy test for the "always OK" section:
>
> anon_enum_var = (__typeof__(anon_enum_var)) 0;
> anon_enum_var = (__typeof__(anon_enum_var)) VALUE_A;
All three cases pass for me, even with -Wenum-to-int. No problem here.
> > - warn_for_different_enum_types (old->pos, old->ctype, type);
> > + warn_for_different_enum_types (old->pos, old->ctype, type,
> > + old->enum_type);
>
> At this point, it might make more sense to just pass old, rather than
> three of its fields. Would that work for the other callers of this
> function? In any case, that change can wait until after this patch.
At first glance the only change would be in usual_conversions() in reporting
slightly different location in some rear cases -- no big deal I think.
But in fact I haven't investigated yet how to trigger all the particular
possibilities of warn_for_different_enum_types() invocation to test this
properly. I'll give it a try later today.
> > + warn_for_enum_to_int_cast (old, type);
> > + warn_for_int_to_enum_cast (old, type);
>
> I just realized that both of these functions need renaming as well:
> s/cast/conversion/ or similar. As with the manpage changes before,
> "cast" describes the case it *doesn't* warn about.
My line of thinking was "implied vs. explicit cast" ... but I am fine with
the rename. It'll be more obvious.
> > --- a/sparse.1
> > +++ b/sparse.1
> > @@ -149,6 +149,19 @@ Sparse issues these warnings by default. To turn
> > them off, use \fB\-Wno\-enum\-mismatch\fR.
> > .
> > .TP
> > +.B \-Wenum\-to\-int
> > +Warn about converting an \fBenum\fR type to int. An explicit cast to int
> > will +suppress this warning.
> > +.
> > +.TP
> > +.B \-Wint\-to\-enum
> > +Warn about converting int to \fBenum\fR type. An explicit cast to the
> > right +\fBenum\fR type will suppress this warning.
> > +
> > +Sparse issues these warnings by default. To turn them off, use
> > +\fB\-Wno\-int\-to\-enum\fR.
>
> This manpage text looks good to me.
Thanks for review!
> You could include this test case in the patch, as an addition to the
> test suite.
No problem I think. We have generally two choices:
1) The whole current test-enum.c as a test-case running with all three
warnings enabled.
2) Split the test to three parts, each for one separate -W option, and
then run it as three separate test-cases.
I think the second choice has better coverage. What's your opinion?
Kamil
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
