[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
Zachary Turner via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Fri Oct 26 01:17:12 PDT 2018
zturner added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbSymUid.h:46
+ // due to the debug magic at the beginning of the stream.
+ uint64_t global : 1; // True if this is from the globals stream.
+ uint64_t modi : 16; // For non-global, this is the 0-based index of module.
> 30 + 1 != 32 - what's going on?
There's supposed to be a `uint64_t unused : 1;` here. Thanks for noticing.
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:913
+ uint32_t section_idx = section - 1;
+ if (section_idx >= section_list->GetSize())
> comment explaining the - 1 ?
I'm going to be totally honest here. I don't understand this code at all. I copied it from the `SymbolFilePDB` implementation. It seems to work :-/
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:936
+ lldb::ValueType scope = eValueTypeInvalid;
+ TypeIndex ti;
+ llvm::StringRef name;
> pls explicitly initialize all the locals
`TypeIndex` has a constructor, but I should definitely initialize the `lldb::addr_t`
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:1406
+ TypeSP type_sp = CreateAndCacheType(uid);
+ return &*type_sp;
> type_sp->get() seems cleaner / more readable
You know, I kept trying to write that, and I was like "what silly person on the C++ standards committee forgot to put a `get()` method on `std::shared_ptr<>`. Did they really forget this or am I just going crazy?" Turns out it's a bug in MSVC intellisense where it doesn't show it for some reason. So I took that to mean it didn't exist. Shame on me.
More information about the lldb-commits