[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

This revision was automatically updated to reflect the committed changes. Closed by commit rG63b0f8c788d8: [RecordLayout] Fix ItaniumRecordLayoutBuilder so that is grabs the correct… (authored by shafik). Herald added projects: clang, LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, thanks for the patch! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83008/new/ https://reviews.llvm.org/D83008 ___ lldb-comm

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

shafik updated this revision to Diff 276407. shafik added a comment. Moved from shell test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83008/new/ https://reviews.llvm.org/D83008 Files: clang/lib/AST/RecordLayoutBuilder.cpp lldb/test/API/lang/cpp/alignas_base_class/Makefile lldb

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

shafik added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:2197 // least as many of them as possible). return RD->isTrivial() && RD->isCXX11StandardLayout(); } teemperor wrote: > See here for the POD check that we might get wrong

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

shafik updated this revision to Diff 276229. shafik marked 5 inline comments as done. shafik added a comment. - Removing spurious local variables in test - Simplifying the test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83008/new/ https://reviews.llvm.org/D83008 Files: clang/lib/A

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

shafik added a comment. In D83008#2136303 , @teemperor wrote: > I think we are talking about different things. My question was why is this a > 'Shell' test (like, a test in the `Shell` directory that uses FileCheck) and > not a test in the `API` director

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

teemperor added a comment. I think we are talking about different things. My question was why is this a 'Shell' test (like, a test in the `Shell` directory that uses FileCheck) and not a test in the `API` directory (using Python). An 'API' test could use the proper expression testing tools. And

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

shafik added a comment. In D83008#2131776 , @teemperor wrote: > Thanks for tracking this down, this is a really nasty bug... > > The fix itself is obviously fine, but I think I'm out of the loop regarding > the testing strategy. We talked about adding a C

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

shafik updated this revision to Diff 275865. shafik added a comment. Adding a second test that is not arm64 specific. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83008/new/ https://reviews.llvm.org/D83008 Files: clang/lib/AST/RecordLayoutBuilder.cpp lldb/test/Shell/Expr/Inputs/la

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

teemperor added a comment. In D83008#2131796 , @tschuett wrote: > You could try: > > clangxx_host -Xclang -fdump-record-layouts %p/Inputs/layout.cpp -emit-llvm > -o /dev/null > > > You could commit the record layouts without your fix to show the differe

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

tschuett added a comment. You could try: clangxx_host -Xclang -fdump-record-layouts %p/Inputs/layout.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83008/new/ https://reviews.llvm.org/D83008 ___ lldb-commits mailing list lldb-commits

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Thanks for tracking this down, this is a really nasty bug... The fix itself is obviously fine, but I think I'm out of the loop regarding the testing strategy. We talked about a

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

shafik marked an inline comment as done. shafik added a subscriber: rsmith. shafik added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1190 if (UseExternalLayout) { -// FIXME: This appears to be reversed. if (Base->IsVirtual) @rsm

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

shafik created this revision. shafik added reviewers: teemperor, jingham, jasonmolenda, friss. Herald added a subscriber: kristof.beyls. shafik marked an inline comment as done. shafik added a subscriber: rsmith. shafik added inline comments. Comment at: clang/lib/AST/RecordLayou