On Mon, Mar 29, 2010 at 11:05:08AM -0700, Christopher Li wrote: > On Sat, Mar 27, 2010 at 2:29 AM, Josh Triplett <josh@xxxxxxxxxxxxxxxx> wrote: > >> .type = (enum keyword) (KW_SPECIFIER | KW_SHORT), > >> lookup_keyword(token->ident, (enum namespace) (NS_KEYWORD | NS_TYPEDEF)); > > > > That looks wrong. .type doesn't contain a value of type "enum keyword", > > it contains the bitwise or of such values, which won't represent a valid > > enum value. Thus, .type should have an integral type, not an enum type. > > The same goes for the second parameter of lookup_keyword. > > According to the new "strict enum" rules which don't allow enum type to accept > value other than the enum list above, that is right. > > But I argue that this strict enum rules does not make sense, after this change, > the code don't not gain any thing. In fact, it looks worse. > > Sparse use a loosely define enum namespace for member "namespace". It means > it has all this basic enum value, from NS_MACRO to NS_KEYWORD. In my mind, > all combination of this value consider enum namespace as well. It is > just that I can't > list all of them in the enum list. > > 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. No language that I know of with an enumerated type has such a behavior, including C. However, we could easily produce a type that *does* have the behavior you want, with __attribute__((bitwise)). That would produce a type that doesn't mix with int, but allows bitwise operations like | and &. That seems like a much better fit than enums. > It also helps people understand which set of value belongs to > "namespace" member. > Make "namespace" a plain int, that message is lost. It become very > confusing for new comer > what value was allowed in this int type. I agree that we should have *some* better documentation than just "int", but C alone doesn't do that. Because C has historically just treated "enum" as "int", people abuse the enum types in many different ways because compilers didn't complain. In particular, the use of enum to define a set of related constants rather than #define or const works in some cases (when the constants get used as standalone values) and not in others (bitwise flags). If you *really* want to allow "bitwise enums", what about allowing __attribute__((bitwise)) on the enum definition? That would help clarify what the enum really means. It seems like a mistake to me to allow 0x3 in an enum whose values only allow 0x1 and 0x2, but at least tagging it with bitwise would document that usage and distinguish it from enums that really do list all the permissible values. > So back to my point. It seems making the enum more strict is just make up rules > and gain nothing in real life. It makes code looks worse just to make > strict enum type > happy. On the contrary, making enum more strict helps improve static typing, preventing many abuses of enums. This only seems like a problem in code that abused enums, like Sparse did. :) - Josh Triplett -- 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
