aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
LGTM, but you should add a release note for the change. ================ Comment at: clang/lib/Basic/TypeTraits.cpp:64 +#define TYPE_TRAIT_N(Spelling, Name, Key) 0, +#include "clang/Basic/TokenKinds.def" +}; ---------------- inclyc wrote: > inclyc wrote: > > shafik wrote: > > > @aaron.ballman do we really have to include this three times? We are > > > defining different macros so shouldn't we be able to include is just > > > once? > > > > > > I see we do this in several other places but a few we don't. > > I've tried > > > > ``` > > #define TYPE_TRAIT(N, I, K) N, > > #include "clang/Basic/TokenKinds.def" > > ``` > > > > But using enum `TypeTrait` as array index seems to have incorrect mapping. > > @aaron.ballman do we really have to include this three times? We are > > defining different macros so shouldn't we be able to include is just once? > > > > I see we do this in several other places but a few we don't. > > Type trait definitions in `#include "clang/Basic/TokenKinds.def"` may not be > sorted by the number of arguments. > > Example: > > ``` > #define TYPE_TRAIT_1(some_stuff) > #define TYPE_TRAIT_N(some_stuff) > #define TYPE_TRAIT_2(some_stuff) > ``` > > Might be necessary to include this 3 times to get sorted layouts, like > > `[1, 1, 1, 2, 2, 2, 0, 0]` Yeah, I think we ultimately either need to reorder the TokenKinds.def file to be in sorted order (fragile, surprising, tight coupling) or we have to do this dance because we need the initialization itself to be sorted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131423/new/ https://reviews.llvm.org/D131423 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
