Michael137 added inline comments.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1481
+ // To fix this we scan base classes in reverse order to determine
+ // overlapping offsets. Wnen found we consider such class as empty
+ // base with all fields having [[no_unique_address]] attribute.
----------------
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1483
+ // base with all fields having [[no_unique_address]] attribute.
+ for (auto it = base_classes.rbegin(); it != base_classes.rend(); ++it) {
+ clang::CXXRecordDecl *prev_base_decl =
----------------
The main problem I still see with this is that if we have something like:
```
struct A : C, B {
};
```
then we mark `C`'s fields as empty and leave `B` as is. This still leads to the
same crash later on.
Perhaps we should mark we could check the size of the struct and decide based
on that which one is the "empty" one
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1498
+ ast->getASTContext(), clang::SourceRange()));
+ }
layout_info.base_offsets.insert(std::make_pair(
----------------
I think it would still be nice to have this as a private helper function on
`DWARFASTParserClang`. But don't feel very strongly about it so feel free to
ignore
================
Comment at: lldb/test/API/types/TestEmptyBase.py:2
+"""
+Test that recursive types are handled correctly.
+"""
----------------
Description needs fixing
================
Comment at: lldb/test/API/types/TestEmptyBase.py:22
+
+ self.sources = {
+ 'CXX_SOURCES': 'empty_base_type.cpp'}
----------------
Ideally this test would live in `lldb/test/API/lang/cpp/no_unique_address/`
The usual structure is that we have a `Makefile` in the same directory which
consists of:
```
CXX_SOURCES := main.cpp
include Makefile.rules
```
Then in the `test(self)` you just call `self.build()`. You don't need the
`setUp(self)` and `setTearDownCleanup(...)` calls then.
Check
`lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py` for
example.
================
Comment at: lldb/test/API/types/empty_base_type.cpp:1
+struct C
+{
----------------
Can we add more test cases. E.g.,:
```
struct A : C, B {
...
};
```
================
Comment at: lldb/test/API/types/empty_base_type.cpp:3
+{
+ long c,d;
+};
----------------
Just for sanity checking in the test
================
Comment at: lldb/test/API/types/empty_base_type.cpp:23
+{
+ long a,b;
+} _a;
----------------
same as above
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143347/new/
https://reviews.llvm.org/D143347
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
