This is an archive of the discontinued LLVM Phabricator instance.

[lldb-vscode] Creating a new flag for adjusting the behavior of evaluation repl expressions to allow users to more easily invoke lldb commands.
ClosedPublic

Authored by ashgti on Jun 28 2023, 5:24 PM.

Details

Summary

This adds a new flag and lldb runtime command to allow users to manage the behavior of the lldb-vscode evaluate repl request.

When evaluating a repl context this now has runtime managed flag for control how the repl behaviors with the follow values:

  • variable - the existing behavior, with this mode requests are evaluted in the current frame context as variable expressions. To trigger a lldb command prefix an expression with ` and it will be evaluted as an lldb command.
  • command - all expressions are evaluated as lldb commands.
  • auto - An alternative mode that will attempt to determine if the expression is an lldb command or a variable expression. Based off the intepreted results the expression will be evaluted either as a command or an expression.

Additionally, I enabled completions and ensured they work with the new repl expression behavior to provide auto-completes.

RFC Discussion: https://discourse.llvm.org/t/rfc-lldb-vscode-evaluate-repl-behavior-and-improvements/71667

Diff Detail

Event Timeline

ashgti created this revision.Jun 28 2023, 5:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2023, 5:24 PM
ashgti requested review of this revision.Jun 28 2023, 5:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2023, 5:24 PM
ashgti updated this revision to Diff 535580.Jun 28 2023, 5:44 PM

Small tweak to the auto expression mode.

ashgti updated this revision to Diff 535590.Jun 28 2023, 5:57 PM

Another tweak to the auto mode behavior.

ashgti updated this revision to Diff 535591.Jun 28 2023, 5:58 PM

Formatting.

Harbormaster completed remote builds in B241947: Diff 535591.
ashgti updated this revision to Diff 535592.Jun 28 2023, 5:59 PM

Formatting

ashgti updated this revision to Diff 535911.Jun 29 2023, 11:01 AM

Adding ` as an escape hatch in auto mode to ensure lldb commands can always be run directly.

ashgti updated this revision to Diff 536031.Jun 29 2023, 3:25 PM

Pull + Rebase.

Hi wallace,

I created to improve the repl behavior of lldb-vscode allowing users to more easily run lldb commands. I started https://discourse.llvm.org/t/rfc-lldb-vscode-evaluate-repl-behavior-and-improvements/71667 as well to outline some of behaviors from this patch.

Some random comments, I'll leave the real review to the VSCode experts.

lldb/tools/lldb-vscode/JSONUtils.h
239 ↗(On Diff #536031)

Appears that this got clang-formatted accidentally. Formatting it is fine but put it in an NFC change if you're gonna do that.

If you aren't already using it, the clang-format-diff script will help you limit formatting to only what you've edited (with occasional extra bits): https://clang.llvm.org/docs/ClangFormat.html

lldb/tools/lldb-vscode/VSCode.cpp
797

There is AppendMessageWithFormat that you could use to avoid repeating the string here.

805

Would be useful if it also said: "repl-mode "foo". Expected one of "auto", .....".

806

SetError ends up calling CommandReturnObject::AppendError which will set the status to failed for you (I changed this a while back because I kept forgetting to set it).

ashgti updated this revision to Diff 537496.Jul 5 2023, 2:14 PM
ashgti marked 4 inline comments as done.

Updating the behavior of auto mode to try to evalute local variables over lldb commands.

ashgti added inline comments.Jul 5 2023, 2:15 PM
lldb/tools/lldb-vscode/JSONUtils.h
239 ↗(On Diff #536031)

I had another commit (https://reviews.llvm.org/D154029) that touched this area of the code so I think thats how this snuck in. Reverted this change.

lldb/tools/lldb-vscode/VSCode.cpp
797

Do you mean Printf? Switched to that instead.

ashgti updated this revision to Diff 537536.Jul 5 2023, 4:00 PM

Uploading latest diff

ashgti updated this revision to Diff 537538.Jul 5 2023, 4:09 PM

Removing a log statement that is not needed.

ashgti edited the summary of this revision. (Show Details)Jul 6 2023, 1:47 PM

Could you split the changes to the selected thread out of this? I'm seeing two features being implemented in this patch.
Other than that, it looks pretty good!

lldb/tools/lldb-vscode/Options.td
20–38

+1

lldb/tools/lldb-vscode/VSCode.cpp
766

use clang-format please

lldb/tools/lldb-vscode/VSCode.h
89

typo

ashgti updated this revision to Diff 539182.Jul 11 2023, 10:37 AM
ashgti marked an inline comment as done.

Apply clang-format and split out some parts of this into smaller commits.

ashgti marked 2 inline comments as done.Jul 11 2023, 10:38 AM
ashgti added inline comments.
lldb/tools/lldb-vscode/Options.td
20–38

Moving this into its own commit.

wallace accepted this revision.Jul 11 2023, 12:52 PM

nice!!

This revision is now accepted and ready to land.Jul 11 2023, 12:52 PM

It looks like this change is breaking some lldb tests. https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/57586/consoleFull#-1731262255a1ca8a51-895e-46c6-af87-ce24fa4cd561

  • TEST 'lldb-api :: tools/lldb-vscode/console/TestVSCode_console.py' FAILED ****

Script:

/usr/local/Frameworks/Python.framework/Versions/3.10/bin/python3.10 /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --codesign-identity lldb_codesign --env LLVM_LIBS_DIR=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./lib --env LLVM_INCLUDE_DIR=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/include --env LLVM_TOOLS_DIR=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin --libcxx-include-dir /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/include/c++/v1 --libcxx-library-dir /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lib --arch x86_64 --build-dir /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex --lldb-module-cache-dir /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin/lldb --compiler /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin/clang --dsymutil /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin/dsymutil --llvm-tools-dir /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin --lldb-libs-dir /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./lib --build-dir /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex -t --env TERM=vt100 /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/API/tools/lldb-vscode/console -p TestVSCode_console.py

I am reverting the change for now, I apologize for any inconvenience

ashgti marked an inline comment as done.Jul 11 2023, 3:19 PM

I am reverting the change for now, I apologize for any inconvenience

Thanks for reverting that, I'll take a look at the tests and double check them on linux as well.