labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Looks good. I suggested some changes, which I hope will reduce duplication and
better empasize the differences between the various branches in the code. I
think I understand the algorithm now, and I believe it is ok, though I can't
escape the feeling that this could have been done in a simpler way.
================
Comment at:
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:459-469
+ if (parent->kind == Member::Struct) {
+ parent->fields.push_back(std::move(fields.back()));
+ end_offset_map[end_offset].push_back(parent);
+ } else {
+ lldbassert(parent == &record &&
+ "If parent is union, it must be the top level record.");
+ MemberUP anonymous_struct = std::make_unique<Member>(Member::Struct);
----------------
The current version will create a needless nested struct in case of a union
with a single member. I think it should be possible to just insert a single
field first, and let it be converted to a struct later -- if it is necessary.
================
Comment at:
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:471-491
+ if (parent->kind == Member::Struct) {
+ MemberUP anonymous_union = std::make_unique<Member>(Member::Union);
+ for (auto &field : fields) {
+ int64_t bit_size = field->bit_size;
+ anonymous_union->fields.push_back(std::move(field));
+ end_offset_map[offset + bit_size].push_back(
+ anonymous_union->fields.back().get());
----------------
This should be equivalent but shorter, and -- I hope -- more obvious.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134849/new/
https://reviews.llvm.org/D134849
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
