[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1482
+    if (prev_base) {
+      clang::CXXRecordDecl *prev_base_decl =
+          prev_base->getType()->getAsCXXRecordDecl();
----------------
Should we add a comment describing why this block is necessary?

Perhaps even putting it into its own helper function


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1487
+        assert(it != layout_info.base_offsets.end());
+        if (it->second.getQuantity() == member_byte_offset) {
+          prev_base_decl->markEmpty();
----------------
The idea seem reasonable in general.

Though this wouldn't work for overlapping member offsets. E.g.,:

```
struct C
{
 long c,d;
};

struct D
{
};

struct B
{
  [[no_unique_address]] D x;
};

struct E
{
  [[no_unique_address]] D x;
};

struct Foo : B, E, C {

};
```

Here `B` and `C` have offset `0x0`, but `E` can have offset `0x1`. So we wouldn't attach the attribute for `E` and would still crash. Maybe we should check for overlapping offsets, not just equal


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143347/new/

https://reviews.llvm.org/D143347