[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
Zachary Turner via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Tue Oct 23 11:54:10 PDT 2018
zturner created this revision.
zturner added reviewers: clayborg, jingham, labath.
Herald added subscribers: atanasyan, JDevlieghere, sdardis.
When we get the `resolve_scope` parameter from the SB API, it's a `uint32_t`. We then pass it through all of LLDB this way, as a uint32. This is unfortunate, because it means the user of an API never actually knows what they're dealing with. We can call it something like `resolve_scope` and have comments saying "this is a value from the `SymbolContextItem` enumeration, but it makes more sense to just have it actually *be* the correct type in the actual C++ type system to begin with. This way the person reading the code just knows what it is.
There is one thing here which I'm a little uncertain about, which is that I included a file from llvm `llvm/ADT/BitmaskEnum.h` from `lldb/lldb-enumerations.h`, which is a public header.` This is only done for convenience, and has two effects.
1. It allows us to use bitwise operations on enums so we don't have to write things like `Foo x = Foo(eFoo1 | eFoo2)`
2. More subtly, it inserts an enumerator into the enum. But (!) it doesn't change the value of any existing enumerators and it does so with a name that won't cause any clashes, so the important thing is that it shouldn't cause any name clashes.
Putting this all together, I think this should be an ABI-compatible change as far as SWIG is concerned, and I can't see any differences on my end. But if anyone can see any reason why we should be wary of this, speak up.
Assuming all goes well with this patch, I plan to repeat the same thing with `SymbolFile::GetTypes` which currently has a `uint32_t types_mask`
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 42145 bytes
Desc: not available
More information about the lldb-commits