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

aprantl added a comment. If we can come up with a counterexample where the heuristic in this patch is doing the wrong thin then I think emitting a DW_AT_LLVM_no_unique_address attribute sounds reasonable to me. But otherwise I don't see a problem with using a heuristic. Repository: rG LLVM

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

Michael137 added a comment. In D143347#4298613 , @kpdev42 wrote: > So what are next steps? Are we going for implementation of > `DW_AT_no_unique_address` (which is going to be a non-standard extension) ? > @dblaikie @aprantl @Michael137 in my opinion t

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

kpdev42 added a comment. So what are next steps? Are we going for implementation of `DW_AT_no_unique_address` (which is going to be a non-standard extension) ? @dblaikie @aprantl @Michael137 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143347/new

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

kpdev42 added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2212 m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType()); -if (record_decl) +if (record_decl) { + bool is_empty = true; Michael137 w

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

Michael137 added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2212 m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType()); -if (record_decl) +if (record_decl) { + bool is_empty = true; Generally

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

kpdev42 updated this revision to Diff 512945. kpdev42 added a comment. Thanks for pointing out the bug @Michael137 . It seems that clang assigns arbitrary offsets for non_unique_address so analyzing them brings me nowhere. In this patch I tried assigning no_unique_address to every empty structur

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

Michael137 added subscribers: aprantl, dblaikie. Michael137 added inline comments. Comment at: lldb/test/API/lang/cpp/no_unique_address/main.cpp:38 + long v = 42; +} _f3; + I haven't checked the reworked logic yet, but it still crashes on this: ``` self.expect_e

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

kpdev42 added inline comments. 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::CXXRecordD

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

kpdev42 added inline comments. 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::CXXRecordD

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

kpdev42 updated this revision to Diff 511683. kpdev42 edited the summary of this revision. kpdev42 added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143347/new/ https://reviews.llvm.org/D143347 Files: lldb/so

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

Michael137 added a comment. FYI, this bugfix was attempted back in 2021 here: https://reviews.llvm.org/D101237 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143347/new/ https://reviews.llvm.org/D143347

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

Michael137 added inline comments. Comment at: lldb/test/API/types/TestEmptyBase.py:25-42 +def test(self): +"""Test that recursive structs are displayed correctly.""" +self.build(dictionary=self.sources) +self.setTearDownCleanup(dictionary=self.sources)

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

Michael137 added inline comments. 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::CXXReco

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

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 fie

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

kpdev42 updated this revision to Diff 511362. kpdev42 added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143347/new/ https://reviews.llvm.org/D143347 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserC

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

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 desc

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

kpdev42 added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143347/new/ https://reviews.llvm.org/D143347 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/c

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

kpdev42 updated this revision to Diff 507272. kpdev42 added a comment. Update patch after landing D145057 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143347/new/ https://reviews.llvm.org/D143347 Files: lldb/s

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

kpdev42 added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:3896 + if (D->hasAttrs()) +ToField->setAttrs(D->getAttrs()); ToField->setAccess(D->getAccess()); balazske wrote: > The import of attributes is handled in function `ASTImporter::Impor

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

balazske added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:3896 + if (D->hasAttrs()) +ToField->setAttrs(D->getAttrs()); ToField->setAccess(D->getAccess()); The import of attributes is handled in function `ASTImporter::Import(Decl*)`. This

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

clayborg added a comment. Changes look fine to me. I would like someone that specializes in the expression parser to give the final ok though. Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp:551 const LayoutI

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

Michael137 added a comment. From a first glance looks ok but ideally the clang changes would be a separate review. Particularly the ASTImporter change could use a unittest Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143347/new/ https://reviews.l

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

kpdev42 created this revision. kpdev42 added reviewers: DavidSpickett, davide, clayborg, k8stone. kpdev42 added projects: LLVM, LLDB. Herald added subscribers: Michael137, JDevlieghere, martong. Herald added a reviewer: shafik. Herald added a reviewer: shafik. Herald added a project: All. kpdev42 r