Re: Sparse crash when mixing int and enum in ternary operator

[Christopher Li <sparse@chrisli.org>]

On Mon, Mar 29, 2010 at 11:17 AM, Kamil Dudka <kdudka@redhat.com> wrote:
> On Mon March 29 2010 20:05:08 Christopher Li wrote:
>> Using enum namespace for member "namespace" has benefit here. It is clear
>> that which set of value it belongs to. E.g. if you assign SYM_NODE into
>> "namespace" member it *looks* is obvious wrong.
>
> We are able to catch assignment of SYM_NODE to 'enum namespace'.  But we are
> not able to catch (SYM_NODE | SYM_ENUM) to 'enum namespace', so that the patch
> makes no difference.  Or am I missing anything?

I am refering to your patch in other email:

struct symbol_op {
-          enum keyword type;
+         int type;

That is worse. Because you make type as plain int, I can assign NS_KEYWORD there
and you wouldn't able to catch any thing right? So you make the enum
so strict that
we can't use it any more.

Currently the compiler might not able to catch it. But it look obvious
wrong to human
eye if you assign other class of enum there.

>
> I don't think the code looks worse, nevertheless respect your attitude.

See my previous comment.

I just want to make sense of this thing. Currently I see a lot of down sides,
forcing people to do more explicit cast or avoid using the enum type completely.
What is the up side again?

Remember, too much warning can be a bad thing as well. It makes people
don't want
to use the tool or just turn off the warning completely. When I run sparse over
its own source code, I consider those warning useless because I don't
have a better
way to fix it. I think the source code is fine as it is.

BTW, I have move the enum warning to a new branch:

http://git.kernel.org/?p=devel/sparse/sparse.enum-warning.git;a=summary

Some people might find it useful in special occasions. I want to heard about it.

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