On Mon March 29 2010 20:48:38 Christopher Li wrote:
> On Mon, Mar 29, 2010 at 11:17 AM, Kamil Dudka <kdudka@xxxxxxxxxx> 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
Have you tried it with sparse without my patch applied? You indeed can assign
NS_KEYWORD to 'enum keyword' and sparse is silent ;-)
> 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.
Type-info makes sense for compilers, analyzers, etc. If you treat it as
plain-text information for humans with no underlying support of that tools,
it's equal to Hungarian notation IMO.
> > 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.
Sure, I don't object it anyhow. As a C++ programmer I've certainly different
attitude to type safety, thus biased. I believe kernel developers would more
likely agree with your point of view.
> 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.
Looks great, though it really wasn't my intention to fork anything :-)
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
