zequanwu added inline comments.
================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:459
+ uint64_t end_offset = offset + fields.back()->bit_size;
+ parent->fields.push_back(fields.back());
+ end_offset_map[end_offset].push_back(parent);
----------------
labath wrote:
> Can `parent` be a union here? Would the algorithm still be correct?
`parent` could only be union when the top level record is a union (at this line
`Member *parent = &record;`). That's the only case when we add an union into
`end_offset_map`. In that case, all the fields would start at the same offset
and we only iterate the loop `for (auto &pair : fields) {` once and then we are
done.
Otherwise, we only insert {end offset, struct/field} into `end_offset_map`
where field must be within an union.
================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:471
+ } else {
+ for (auto &field : fields) {
+ parent->fields.push_back(field);
----------------
labath wrote:
> IIUC, this code is reached when the `parent` object is a union. However, the
> parent is chosen such that it ends before the start of these new fields?
> Therefore its start address will be before the start of these fields as well.
> Is it correct to add the fields to the union under these circumstances, or am
> I misunderstanding something?
This is a special case to handle the case when top level record is an union.
That's the only case we would reach here. This is to avoid adding unnecessary
nested union. For example, the unit test `TestAnonymousUnionInUnion` would
become the following if we always add an anonymous union:
```
union {
union {
m1;
m2;
m3;
m4;
}
}
```
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
