This is an archive of the discontinued LLVM Phabricator instance.

[LLDB] Fix IOHandlerEditline::GetCurrentLines()
ClosedPublic

Authored by wallace on Aug 28 2023, 3:48 PM.

Details

Summary

This method was working as expected if LLDB_ENABLE_LIBEDIT is false, however, if it was true, then the variable m_current_lines_ptr was always pointing to an empty list, because Editline only updates its contents once the full input has been completed (see https://github.com/llvm/llvm-project/blob/main/lldb/source/Host/common/Editline.cpp#L1576).

A simple fix is to invoke Editline::GetInputAsStringList() from GetCurrentLines(), which is already used in many places as the common way to get the full input list.

Diff Detail

Event Timeline

wallace created this revision.Aug 28 2023, 3:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 28 2023, 3:48 PM
wallace requested review of this revision.Aug 28 2023, 3:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 28 2023, 3:48 PM

Is this covered by any tests?

lldb/source/Core/IOHandler.cpp
512

I think this would benefit from a comment explaining the difference?

lldb/source/Expression/REPL.cpp
535

for (const char *line_cstr : current_lines) ?

wallace added inline comments.Aug 29 2023, 2:45 PM
lldb/source/Expression/REPL.cpp
535

actually the current code is fine because we don't want to iterate through the entire set, we just want the first current_line_idx + 1 elements

wallace updated this revision to Diff 554712.Aug 30 2023, 7:28 AM
wallace marked an inline comment as done.

@aprantl , it turns out that there are no tests for this. I also don't know how easy it'd be to test this very specific feature, because it relies on the terminal not being affected by stuff like test runners.

aprantl accepted this revision.Sep 1 2023, 5:01 PM
This revision is now accepted and ready to land.Sep 1 2023, 5:01 PM
wallace closed this revision.Sep 1 2023, 5:52 PM