On 06/30/2012 08:08 PM, Jordan Rose wrote: > > On Jun 30, 2012, at 2:31 AM, Dmitri Gribenko wrote: > >> On Sat, Jun 30, 2012 at 12:55 AM, Enea Zaffanella >> <[email protected]> wrote: >>> In include/clang/AST/RawCommentList.h we have >>> >>> class RawComment { public: enum CommentKind { CK_Invalid, >>> ///< Invalid comment [...] >>> >>> In include/clang/AST/OperationKinds.h the following macro is >>> defined >>> >>> #define CK_Invalid ((CastKind) -1) >> >> Hi Enea, >> >> Thank you for noticing! >> >> Here is a patch that should fix this. >> >> Please review. >> >> Dmitri > > It's possible the reason this isn't used is because CastKind would > otherwise be only positive numbers and thus suitable for storing in > an unsigned bitfield without complaint. Maybe ~0U would be a better > choice of constant? > > Jordan
Imho, here there are two separate issues to be dealt with: 1) the name clash; 2) the use of the macro, which is a bit error prone and might bite again in the future. The proposed patch is addressing issue 2, but it won't "fix" 1. I know that enumeration clang::CastKind is not really clashing with the clang::RawComment::CommentKind enumeration, but there seems to be an implicit coding convention in clang whereby enumeration constants are provided with a disambiguating prefix telling where they come from. So when I read "CK_" in the sources, I immediately think about cast kinds. What about using "RCK_" as a prefix for the "RawCommentKind" enumeration ? As far as 2) is concerned, adding CK_Invalid to the CastKind enumeration should be considered for both pros and cons: on the one hand, it will get rid of the macro (which is good); on the other hand, it would require adding a new case statement to all switches handling a CastKind value. As for me, after addressing point 1) as said above, I would just replace the macro by something like static const CastKind CK_Invalid = /* name your own special value */; without changing the CastKind enumeration. Cheers, Enea. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
