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. ---------------- lemo wrote: > 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()) ---------------- lemo wrote: > 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; ---------------- lemo wrote: > 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; } ---------------- lemo wrote: > 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. https://reviews.llvm.org/D53731 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
