[Lldb-commits] [PATCH] D143127: [LLDB] Fix assertion failure by removing `CopyType` in `std::coroutine_handle` pretty printer

avogelsgesang added a comment. @labath @aprantl Thanks for your reviews! I would like to backport this fix together with the fix from https://reviews.llvm.org/D132815 to the 16.0 release branch now. For that I would need your approval on https://github.com/llvm/llvm-project-release-prs/pull/254

[Lldb-commits] [PATCH] D143127: [LLDB] Fix assertion failure by removing `CopyType` in `std::coroutine_handle` pretty printer

This revision was automatically updated to reflect the committed changes. Closed by commit rG54d4a2550d31: [LLDB] Fix assertion failure by removing `CopyType` in `std::coroutine_handle`… (authored by avogelsgesang). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[Lldb-commits] [PATCH] D143127: [LLDB] Fix assertion failure by removing `CopyType` in `std::coroutine_handle` pretty printer

avogelsgesang added a comment. Agree, other pretty-printers also don't use `ASTImporter`. I think the call was still needed originally in https://github.com/llvm/llvm-project/commit/01f4c305fae9ff2f165ce0f635a90f8e2292308c, because the promise_type was combined with newly created AST types whic

[Lldb-commits] [PATCH] D143127: [LLDB] Fix assertion failure by removing `CopyType` in `std::coroutine_handle` pretty printer

avogelsgesang added a comment. > This change makes me a little nervous, but mostly because I don't quite > understand the design here. > So ASTImporter does not have a check whether src context and dst context are > identical? I am completely with you. I also don't understand the design of `AST

[Lldb-commits] [PATCH] D143127: [LLDB] Fix assertion failure by removing `CopyType` in `std::coroutine_handle` pretty printer

avogelsgesang created this revision. avogelsgesang added reviewers: labath, aprantl. Herald added a subscriber: ChuanqiXu. Herald added a project: All. avogelsgesang requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The pretty printer for `st

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

This revision was automatically updated to reflect the committed changes. Closed by commit rG8aa313755118: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer (authored by avogelsgesang). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

avogelsgesang added a comment. Since I am still blocked on solving the Apple-specific issues in https://reviews.llvm.org/D132735 and couldn't solve them in time for LLVM-16, I now rebased this change directly on `main`, so that it no longer depends on the refactorings from the other commit. My

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

avogelsgesang updated this revision to Diff 493583. avogelsgesang marked an inline comment as done. avogelsgesang added a comment. fix crash in cases where the promise type is unknown even after an attempt at devirtualization Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[Lldb-commits] [PATCH] D132735: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer

avogelsgesang added a comment. I reapplied this patch locally on top of the latest `main` and tried to reproduce the issue on my Mac using > cmake -B build -G Ninja -C lldb/cmake/caches/Apple-lldb-macOS.cmake > -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi" -DLLVM_ENABLE_PROJECTS="clang;lldb" > llv

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

avogelsgesang added a comment. The issue with this change was that if devirtualization is failing, then in the line lldb::ValueObjectSP promise = CreateValueObjectFromAddress( "promise", frame_ptr_addr + 2 * ptr_size, exe_ctx, promise_type); the `promise_type` is `void`. Creating an o

[Lldb-commits] [PATCH] D132735: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer

avogelsgesang added a subscriber: jasonmolenda. avogelsgesang added a comment. As discussed via email, the reason for the test failures on green dragon is, that green dragon seems to be using some compiler which passes the test if not (is_clang and self.expectedCompilerVersion(["<", "16"])):

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

avogelsgesang added a comment. @jasonmolenda I am a bit confused about your revert. Did you indeed revert this change here? Afaict, you reverted https://reviews.llvm.org/D132735 and https://reviews.llvm.org/D132815, but left this commit in place. I did not actually read through your code changes

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. avogelsgesang marked an inline comment as done. Closed by commit rGcd3091a88f7c: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer (authore

[Lldb-commits] [PATCH] D132735: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4346318f5c70: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty… (authored by avogelsgesang). Changed prior to commit: ht

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

This revision was automatically updated to reflect the committed changes. Closed by commit rG01f4c305fae9: Reapply "[LLDB] Devirtualize coroutine promise types for `std… (authored by avogelsgesang). Changed prior to commit: https://reviews.llvm.org/D132624?vs=475327&id=476743#toc Repository:

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

avogelsgesang added a comment. @aprantl I still had to adjust the code slightly to copy the type from the `ASTContext for '.../a.out'` to the `scratch ASTContext` (without this, my code was triggering an assert). See https://reviews.llvm.org/D132624?vs=474793&id=475327 for the related changes.

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

avogelsgesang updated this revision to Diff 475327. avogelsgesang added a comment. Fix ownership of CompilerType between "scratch typesystem" and "module type system" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132624/new/ https://reviews.llvm.o

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

This revision was automatically updated to reflect the committed changes. Closed by commit rG558db7787005: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle` (authored by avogelsgesang). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

avogelsgesang added a comment. @labath @aprantl Thank you for your reviews on https://reviews.llvm.org/D132815 and https://reviews.llvm.org/D132735 I would appreciate if you also have time to review this change here, as it is a pre-requisite for the other two changes :) Repository: rG LLVM G

[Lldb-commits] [PATCH] D132735: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer

avogelsgesang updated this revision to Diff 461235. avogelsgesang marked an inline comment as done. avogelsgesang added a comment. rename `IsNoopResumeDestroy` -> `IsNoopCoroFunction` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132735/new/ https:

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

avogelsgesang marked 4 inline comments as done. avogelsgesang added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp:19 -static ValueObjectSP GetCoroFramePtrFromHandle(ValueObject &valobj) { - ValueObjectSP valobj_sp(valobj.GetNonSyntheticValu

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

avogelsgesang added a comment. > wondering if we couldn't fix this by creating the (non-pointer) object using > the CreateValueObjectFromAddress function, as above, but then actually use > valobj->AddressOf as the synthetic child yes, that worked quite nicely Repository: rG LLVM Github Mono

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

avogelsgesang updated this revision to Diff 461232. avogelsgesang marked an inline comment as done. avogelsgesang added a comment. address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132815/new/ https://reviews.llvm.org/D132815 F

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

avogelsgesang added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp:246-248 + DataExtractor data(&promise_addr, sizeof(promise_addr), + process_sp->GetByteOrder(), + process_sp->GetAddressByteSize()); ---

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

avogelsgesang added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp:246-248 + DataExtractor data(&promise_addr, sizeof(promise_addr), + process_sp->GetByteOrder(), + process_sp->GetAddressByteSize()); ---

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

avogelsgesang added a comment. @labath D132815 addresses your point: With that change, the `promise` member now is typed as a pointer. I would prefer to land the patches in the order D132624 (this review), D132735

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

avogelsgesang created this revision. avogelsgesang added reviewers: labath, dblaikie, aprantl, ChuanqiXu, mib. Herald added a project: All. avogelsgesang requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. So far, the pretty printer for `std::c

[Lldb-commits] [PATCH] D132735: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer

avogelsgesang updated this revision to Diff 456155. avogelsgesang added a comment. fix test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132735/new/ https://reviews.llvm.org/D132735 Files: lldb/source/Plugins/Language/CPlusPlus/Coroutines.

[Lldb-commits] [PATCH] D132735: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer

avogelsgesang updated this revision to Diff 456062. avogelsgesang added a comment. fix offset Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132735/new/ https://reviews.llvm.org/D132735 Files: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp

[Lldb-commits] [PATCH] D132735: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer

avogelsgesang updated this revision to Diff 456060. avogelsgesang added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132735/new/ https://reviews.llvm.org/D132735 Files: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

avogelsgesang updated this revision to Diff 456055. avogelsgesang added a comment. disable tests on gcc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132624/new/ https://reviews.llvm.org/D132624 Files: lldb/source/Plugins/Language/CPlusPlus/Coro

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

avogelsgesang added a comment. > I don't know much about coroutines, but it seems like your goal is to format > them like a linked list actually, my preferred goal would be to show them as a logical, user-level thread. Such that you can type thread backtrace cxxcoro:0xb2a0 to get th

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

avogelsgesang added a comment. > I don't know much about coroutines, but it seems like your goal is to format > them like a linked list Yes, and no. Coroutines are a compiler-provided type-erasure mechanism which (among other things) erase the type of the contained promise. One common use case

[Lldb-commits] [PATCH] D132735: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer

avogelsgesang updated this revision to Diff 455893. avogelsgesang added a comment. Exclude unsupported compilers from test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132735/new/ https://reviews.llvm.org/D132735 Files: lldb/source/Plugins

[Lldb-commits] [PATCH] D132735: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer

avogelsgesang created this revision. avogelsgesang added reviewers: ChuanqiXu, aprantl, dblaikie, ychen, jryans, JDevlieghere. Herald added a project: All. avogelsgesang requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. With this commit, the

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

avogelsgesang added a comment. > leads me to believe that the data formatter is creating the synthetic > children as non pointer objects (e.g. by automatically dereferencing any > nested pointers), even though the structures clearly contain some pointers > inside Yes, that is correct. The form

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

avogelsgesang added a comment. > The only concern is that if it would be not so easy to read if there are too > many levels? (Consider the 30levels example in D132451 > ). If it would be better to print ... at > certain level? Or this is already handled by lldb

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

avogelsgesang created this revision. avogelsgesang added reviewers: Chuanfeng, aprantl, dblaikie, ychen, jryans, JDevlieghere. Herald added subscribers: ChuanqiXu, Prazek. Herald added a project: All. avogelsgesang requested review of this revision. Herald added a project: LLDB. Herald added a sub

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

This revision was automatically updated to reflect the committed changes. Closed by commit rG91389000abe8: [LLDB] Add data formatter for std::coroutine_handle (authored by avogelsgesang). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132415/new/ htt

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

avogelsgesang added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp:39 + CompilerType coro_func_type = ast_ctx.CreateFunctionType( + /*result_type*/ void_type, /*args*/ &void_type, /*num_args*/ 1, + /*is_variadic*/ false, /*qualifiers

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

avogelsgesang updated this revision to Diff 455111. avogelsgesang marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132415/new/ https://reviews.llvm.org/D132415 Files: clang/docs/tools/clang-formatted-files.txt lldb/packages/Python/lldbsuite/test/lldbtes

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

avogelsgesang added inline comments. Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py:15 + +class TestCoroutineHandle(TestBase): +def do_test(self, stdlib_type): jingham wrote: > apra

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

avogelsgesang updated this revision to Diff 455022. avogelsgesang marked 4 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132415/new/ https://reviews.llvm.org/D132415 Files: clang/docs/tools/clang-formatted-files.txt lldb/packages/Python/lldbsuite/test/lldbtes

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

avogelsgesang marked an inline comment as done. avogelsgesang added inline comments. Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py:44 + +lldbutil.run_to_source_breakpoint(self, '// Break after

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

avogelsgesang updated this revision to Diff 454911. avogelsgesang marked 6 inline comments as done. avogelsgesang added a comment. addressed comments from @aprantl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132415/new/ https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

avogelsgesang updated this revision to Diff 454830. avogelsgesang added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132415/new/ https://reviews.llvm.org/D132415 Files: clang/docs/tools/clang-formatted-files.txt lldb/p

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

avogelsgesang updated this revision to Diff 454728. avogelsgesang added a comment. remove unrelated changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132415/new/ https://reviews.llvm.org/D132415 Files: clang/docs/tools/clang-formatted-files.

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

avogelsgesang added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:1139 + AddCXXSynthetic( // XXX + cpp_category_sp, I need to remove this. This is a left-over from an earlier implementation sketch Repository:

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

avogelsgesang created this revision. avogelsgesang added reviewers: ChuanqiXu, aprantl, labath, mib, JDevlieghere. Herald added a subscriber: mgorny. Herald added a project: All. avogelsgesang requested review of this revision. Herald added projects: clang, LLDB. Herald added subscribers: lldb-comm