[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D53597#1273087, @jingham wrote:

> It would be great not to have to use comments to express what these values mean.  OTOH, having to put in static casts whenever you want to or values together would be a pain, so if there's no way to do it without magic, I'm a little less enthused...
>
> But on the magic: It looks like BitMaskEnum.h pulls in MathTypes.h which pulls in Compiler.h which then pulls in llvm-config.h.  You are supposed to be able to build tools on top of lldb with just the headers that go in LLDB.framework, you are not required to have the full source tree for an lldb build.  I don't want to impose that restriction without very good reason, and fixing this wart doesn't rise to that level.  Anyway, so if we want to include BitMaskEnum.h we would be required to ship a bunch of llvm headers (including some build produced ones IIUC).  I don't think that's a very good idea.
>
> It also looks to me like the value of the largest item + 1 gets baked into the operator overloading code.  What would happen if we decided to add an element to the enum?


It's not the largest item +1, it's actually just that the largest item gets a second internal name.  If you add a new element to the enum you need to just make sure you update the `LLVM_MARK_AS_BITMASK_ENUM()` to contain the new largest value.

Anyway, your point about `llvm-config.h` is a good one, so what I can perhaps do instead is make something called `LLDB_DEFINE_BITMASK_ENUM(EnumName)` which basically just defines the overloads for a particular enum, then we can have it be a two step process.  1. Make the enum, 2. Call `LLDB_DEFINE_BITMASK_ENUM(Enum)`.  This way there's no external header dependencies.  I'll upload a new patch shortly.


https://reviews.llvm.org/D53597