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

Arthur O'Dwyer via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 1 10:14:19 PST 2022


Quuxplusone added a comment.

In D120398#3351998 <https://reviews.llvm.org/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) (the difference between and int and a unsigned char for 7 enums)
>
> Is saving 21 bytes valuable?

Peanut gallery says: I think giving every enum a concrete underlying type is a //good// practice, so I don't think this makes anything less readable. (That is, I'd even favor adding `: int` to each of these enums, over the status quo where the type is merely implied — we've seen that people can be unsure of the exact behavior.)
I can imagine that someone might object that hard-coding one byte for each enum might one day cause problems if we want more than 256 possible settings for any given enum, but that's not likely, is it?
Re space savings, OP says that they're saving 21 bytes //per object// and that they have "lots" of objects in their particular use-case, even if clang-format itself doesn't.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120398/new/

https://reviews.llvm.org/D120398



More information about the cfe-commits mailing list