faisalv added inline comments.
================ Comment at: clang/include/clang/Sema/DeclSpec.h:1837 /// Actually a FunctionDefinitionKind. - unsigned FunctionDefinition : 2; + FunctionDefinitionKind FunctionDefinition : 2; ---------------- aaron.ballman wrote: > faisalv wrote: > > aaron.ballman wrote: > > > I think we need to keep this as `unsigned` because some compilers > > > struggle with bit-fields of enumeration types (even when the enumeration > > > underlying type is fixed): https://godbolt.org/z/P8x8Kz > > As Barry had reminded me - this warning was deemed a bug: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51242. Are you sure we should > > still tailor our code to appease it? Is there a config file we can use to > > #define an ENUM_UNSIGNED_BITFIELD(x) or some such - that does the right > > thing for most compilers - (and are we even comfortable from a style-guide > > perpective, with such a conditional-define strategy? > > > > Your thoughts? > > > > Thanks! > The warning in GCC was a bug, but the fact that GCC issues the warning means > `-Werror` builds will not be able to handle it. GCC 9.2 is really recent, so > we can't just bump the supported version of GCC to 9.3 to avoid the issue. We > could use macros to work around it for GCC, but IIRC, MSVC also had some > hiccups over the years with using an enumeration as a bit-field member (I > seem to recall it not wanting to pack the bits with surrounding fields, but I > could be remembering incorrectly). I'm not certain whether macros are worth > the effort, but my personal inclination is to just stick with `unsigned` > unless there's a really big win from coming up with something more complex. Well - the biggest downside of making it unsigned (vs leaving it as an enum) is that each assignment or initialization now requires a static_cast. What would you folks suggest: 1) do not modernize such enums into scoped enums 2) scope these enums - sticking to unsigned bit-fields - and add static_casts 3) teach the bots to ignore that gcc warning? (is this even an option) Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91035/new/ https://reviews.llvm.org/D91035 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
