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

[Zachary Turner via Phabricator via lldb-commits]

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`


https://reviews.llvm.org/D53597

Files:
  lldb/include/lldb/Core/Address.h
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Core/ModuleList.h
  lldb/include/lldb/Symbol/CompileUnit.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolVendor.h
  lldb/include/lldb/Target/StackFrame.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SBAddress.cpp
  lldb/source/API/SBFrame.cpp
  lldb/source/API/SBModule.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Core/Address.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
  lldb/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
  lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
  lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Symbol/SymbolFile.cpp
  lldb/source/Symbol/SymbolVendor.cpp
  lldb/source/Target/StackFrame.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53597.170716.patch
Type: text/x-patch
Size: 42145 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20181023/c53024d8/attachment-0001.bin>