HazardyKnusperkeks added a comment. Herald added a project: All. In D120398#3352024 <https://reviews.llvm.org/D120398#3352024>, @Quuxplusone wrote:
> 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 also like a defined type for enums (I even have multiple enums based on bool). > 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? I think an option with more than 256 variants would be a horror. But even if we need that, we can just increase the type, since we have no API or ABI stability in clang-format. We regulary change a bool to an enum. 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 <https://reviews.llvm.org/D93758>. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120398/new/ https://reviews.llvm.org/D120398 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
