erichkeane added a comment. A few nits, but not nearly 'expert' enough to approve without giving everyone else some time to look this over.
================ Comment at: clang/lib/Frontend/InitPreprocessor.cpp:260 DefineFmt(Prefix + Twine(TypeWidth), Ty, TI, Builder); + DefineTypeWidth(Prefix + Twine(TypeWidth) + "_WIDTH__", Ty, TI, Builder); } ---------------- Minor-est of nits: It would seem that TypeSize and TypeWidth should go together, just because of how similar they are. Is there a reason to not put this line above the Fmt? ================ Comment at: clang/test/Headers/limits.cpp:33 -const bool char_is_signed = (char)-1 < (char)0; -_Static_assert(CHAR_MIN == (char_is_signed ? -CHAR_MAX-1 : 0), ""); -_Static_assert(CHAR_MAX == (char_is_signed ? -(CHAR_MIN+1) : (char)~0ULL), ""); +_Static_assert(CHAR_MIN == (((char)-1 < (char)0) ? -CHAR_MAX-1 : 0), ""); +_Static_assert(CHAR_MAX == (((char)-1 < (char)0) ? -(CHAR_MIN+1) : (char)~0ULL), ""); ---------------- Is this change related? Not sure I get what the change here is. ================ Comment at: clang/test/Headers/limits.cpp:62 +/* None of these are defined. */ +int BOOL_WIDTH, CHAR_WIDTH, SCHAR_WIDTH, UCHAR_WIDTH, USHRT_WIDTH, SHRT_WIDTH, + UINT_WIDTH, INT_WIDTH, ULONG_WIDTH, LONG_WIDTH, ULLONG_WIDTH, LLONG_WIDTH; ---------------- Hmm... this is somewhat clever... perhaps overly so? Can you improve the comment on 61? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115253/new/ https://reviews.llvm.org/D115253 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
