lemo accepted this revision. lemo added a comment. This revision is now accepted and ready to land.
Looks good to me, minor notes inline. ================ Comment at: lldb/lit/SymbolFile/NativePDB/tag-types.cpp:131 + +int main(int argc, char **argv) { + Struct S; ---------------- zturner wrote: > lemo wrote: > > a few suggestions for additional things to cover: > > - local classes > > - lambdas > > - instantiating class and function templates > > - explicit specializations > > - for classes, partial specializations > > - namespaces > > - anonymous namespace > Some of these I could probably handle right now. I tried to keep the scope > of the patch to "types which would be named", because it makes it easy to > write tests (the test can just do lookup by name, basically a WinDbg `dt` > test.). That makes local classes, lambdas, anonymous namespace, and anything > to do with functions out of scope. k. how about more basic stuff like? - pointer to pointer - const/volatile ================ Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:313 + case SimpleTypeKind::SByte: + return "signed chr"; + case SimpleTypeKind::Character16: ---------------- char ================ Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:335 + case SimpleTypeKind::Int64Quad: + return "__int64"; + case SimpleTypeKind::Int32: ---------------- int64_t ? ================ Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:344 + case SimpleTypeKind::UInt64Quad: + return "unsigned __int64"; + case SimpleTypeKind::HResult: ---------------- unsigned int64_t ? ================ Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:660 + ct = ct.GetPointerType(); + uint32_t pointer_size = 4; + switch (ti.getSimpleMode()) { ---------------- minor: = 0; ? (to make any potential mistakes more obvious) ================ Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:687 + CompilerType ct = m_clang->GetBasicType(bt); + size_t size = GetTypeSizeForSimpleKind(ti.getSimpleKind()); + ---------------- assert size > 0 ? ================ Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:189 +} \ No newline at end of file ---------------- nit: add empty line at the end of file ================ Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h:26 +namespace lldb_private { +class Type; +class CompilerType; ---------------- nit: vertical space between { and the next line https://reviews.llvm.org/D53511 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits