lemo accepted this revision.
lemo added a comment.
This revision is now accepted and ready to land.
looks good. a few comments inline.
================
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?
================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbSymUid.h:135
+ static PdbSymUid makeGlobalVariableUid(uint32_t offset) {
+ PdbSymUid uid;
+ uid.m_uid.cu_sym.modi = 0;
----------------
= {}; ? (to avoid uninitialized bits)
================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:892
+static DWARFExpression MakeGlobalLocationExpression(uint16_t section,
+ uint32_t offset,
----------------
assert that section > 0 ? (as precondition)
================
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 ?
================
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
================
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
https://reviews.llvm.org/D53731
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
