Skip to content

Commit

Permalink
[LLDB] Fix assertion failure by removing CopyType in `std::coroutin…
Browse files Browse the repository at this point in the history
…e_handle` pretty printer

The pretty printer for `std::coroutine_handle` was running into
> Assertion failed: (target_ctx != source_ctx && "Can't import into itself")
from ClangASTImporter.h, line 270.

This commit fixes the issue by removing the `CopyType` call from the
pretty printer. While this call was necessary in the past, it seems to
be no longer required, at least all test cases are still passing. Maybe
something changed in the meantime around the handling of `TypesystemClang`
instances. I don't quite understand why `CopyType` was necessary earlier.

I am not sure how to add a regression test for this, though. It seems
the issue is already triggered by the exising `TestCoroutineHandle.py`,
but API tests seem to ignore all violations of `lldbassert` and still
report the test as "passed", even if assertions were triggered

Differential Revision: https://reviews.llvm.org/D143127

(cherry picked from commit 54d4a25)
  • Loading branch information
vogelsgesang authored and tru committed Feb 13, 2023
1 parent c319c03 commit 6dabe60
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 8 deletions.
7 changes: 2 additions & 5 deletions lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

#include "Coroutines.h"

#include "Plugins/ExpressionParser/Clang/ClangASTImporter.h"
#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
#include "lldb/Symbol/Function.h"
#include "lldb/Symbol/VariableList.h"
Expand Down Expand Up @@ -97,8 +96,7 @@ bool lldb_private::formatters::StdlibCoroutineHandleSummaryProvider(

lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd::
StdlibCoroutineHandleSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
: SyntheticChildrenFrontEnd(*valobj_sp),
m_ast_importer(std::make_unique<ClangASTImporter>()) {
: SyntheticChildrenFrontEnd(*valobj_sp) {
if (valobj_sp)
Update();
}
Expand Down Expand Up @@ -174,8 +172,7 @@ bool lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd::
if (Function *destroy_func =
ExtractDestroyFunction(target_sp, frame_ptr_addr)) {
if (CompilerType inferred_type = InferPromiseType(*destroy_func)) {
// Copy the type over to the correct `TypeSystemClang` instance
promise_type = m_ast_importer->CopyType(*ast_ctx, inferred_type);
promise_type = inferred_type;
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

namespace lldb_private {

class ClangASTImporter;

namespace formatters {

/// Summary provider for `std::coroutine_handle<T>` from libc++, libstdc++ and
Expand Down Expand Up @@ -50,7 +48,6 @@ class StdlibCoroutineHandleSyntheticFrontEnd
lldb::ValueObjectSP m_resume_ptr_sp;
lldb::ValueObjectSP m_destroy_ptr_sp;
lldb::ValueObjectSP m_promise_ptr_sp;
std::unique_ptr<lldb_private::ClangASTImporter> m_ast_importer;
};

SyntheticChildrenFrontEnd *
Expand Down

0 comments on commit 6dabe60

Please sign in to comment.