[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

kwk abandoned this revision. kwk added a comment. I abandon this revision in favor of https://reviews.llvm.org/D120884. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120398/new/ https://reviews.llvm.org/D120398

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

HazardyKnusperkeks added a comment. In D120398#3353053 , @owenpan wrote: > Can we use `int8_t` instead of `unsigned char` to keep the enum types signed > (and update the changes made in D93758 )? +1 Repository: rG LL

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

owenpan added a comment. Can we use `int8_t` instead of `unsigned char` to keep the enum types signed (and update the changes made in D93758 )? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120398/new/ https://revi

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

kwk added a comment. In D120398#3352436 , @HazardyKnusperkeks wrote: > > That being said, I don't care that much that we declare the enums with a > type. But I care about consistency. Either land this, or revert D93758 >

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

HazardyKnusperkeks added a comment. Herald added a project: All. In D120398#3352024 , @Quuxplusone wrote: > In D120398#3351998 , > @MyDeveloperDay wrote: > >> Before this lands can we have a discussion about wha

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

MyDeveloperDay added a comment. In my own area I gave FormatStyle a copy constructor and made it private, it hardly caused any issues, this isn't something that we tend copy about very much, we tend copy construct it only when we do getLLVMStyle() to make another style say like google style sur

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

Quuxplusone added a comment. In D120398#3351998 , @MyDeveloperDay wrote: > Before this lands can we have a discussion about what clarity this gives us?, > because I think it makes the code more unreadable, but surely we are saving > just 7x(3 bytes) (t

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

MyDeveloperDay added a comment. Before this lands can we have a discussion about what clarity this gives us?, because I think it makes the code more unreadable, but surely we are saving just 7x(3 bytes) (the difference between and int and a unsigned char for 7 enums) Is saving 21 bytes valuabl

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

Quuxplusone added a comment. > Any runtime penalty? (Usually compilers choose a faster underlying type for > enums, not smaller) > The decision is made in the header (so I think it will always be `int` until > this doesn't fit). To decide what is faster the compiler would need to know > all

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. The decision is made in the header (so I think it will always be `int` until this doesn't fit). To decide what is faster the compiler would need to know all the usages. Which it can't. We have multiple times bit fiel

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

curdeius added a comment. Looks innocent but I have a few questions. What's the size difference that you observe? Any runtime penalty? (Usually compilers choose a faster underlying type for enums, not smaller) Also, is this change very relevant to clang-format? We should not copy FormatStyle a l

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

MyDeveloperDay added a comment. Is there a clang-tidy check for this? I mean if we are doing this because it helps reduce the size of FormatStyle and its recommended practice for small enums, then we should have a clang-tidy check that catches us on this. (no?) Repository: rG LLVM Github Mon

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

kwk updated this revision to Diff 411105. kwk added a comment. Rebased on upstream/main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120398/new/ https://reviews.llvm.org/D120398 Files: clang/include/clang/Format/Format.h Index: clang/include/

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

kwk created this revision. kwk requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Analogous to https://reviews.llvm.org/D93758, this patch uses `unsigned char` for all enums in `FormatStyle`. Repository: rG LLVM Github Monorepo https://re