diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py @@ -440,8 +440,11 @@ print("invalid response") return None - def get_completions(self, text): - response = self.request_completions(text) + def get_completions(self, text, frameId=None): + if frameId is None: + stackFrame = self.get_stackFrame() + frameId = stackFrame["id"] + response = self.request_completions(text, frameId) return response["body"]["targets"] def get_scope_variables(self, scope_name, frameIndex=0, threadId=None): @@ -869,8 +872,10 @@ response = self.send_recv(command_dict) return response - def request_completions(self, text): + def request_completions(self, text, frameId=None): args_dict = {"text": text, "column": len(text)} + if frameId: + args_dict["frameId"] = frameId command_dict = { "command": "completions", "type": "request", diff --git a/lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py b/lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py --- a/lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py +++ b/lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py @@ -10,7 +10,7 @@ from lldbsuite.test.lldbtest import * -class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase): +class TestVSCode_completions(lldbvscode_testcase.VSCodeTestCaseBase): def verify_completions(self, actual_list, expected_list, not_expected_list=[]): for expected_item in expected_list: self.assertIn(expected_item, actual_list) @@ -43,7 +43,13 @@ "label": "var -- vector> &", } ], - [{"text": "var1", "label": "var1 -- int &"}], + [ + { + "text": "var", + "label": "var -- Show variables for the current stack frame. Defaults to all arguments and local variables in scope. Names of argument, local, file static and file global variables can be specified.", + }, + {"text": "var1", "label": "var1 -- int &"}, + ], ) # should see global keywords but not variables inside main diff --git a/lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py b/lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py --- a/lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py +++ b/lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py @@ -11,7 +11,7 @@ class TestVSCode_console(lldbvscode_testcase.VSCodeTestCaseBase): def check_lldb_command(self, lldb_command, contains_string, assert_msg): - response = self.vscode.request_evaluate("`%s" % (lldb_command)) + response = self.vscode.request_evaluate("`%s" % (lldb_command), context="repl") output = response["body"]["result"] self.assertIn( contains_string, diff --git a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py --- a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py +++ b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py @@ -37,7 +37,8 @@ ) symbols_path = self.getBuildArtifact(symbol_basename) self.vscode.request_evaluate( - "`%s" % ('target symbols add -s "%s" "%s"' % (program, symbols_path)) + "`%s" % ('target symbols add -s "%s" "%s"' % (program, symbols_path)), + context="repl", ) def checkSymbolsLoadedWithSize(): diff --git a/lldb/test/API/tools/lldb-vscode/startDebugging/TestVSCode_startDebugging.py b/lldb/test/API/tools/lldb-vscode/startDebugging/TestVSCode_startDebugging.py --- a/lldb/test/API/tools/lldb-vscode/startDebugging/TestVSCode_startDebugging.py +++ b/lldb/test/API/tools/lldb-vscode/startDebugging/TestVSCode_startDebugging.py @@ -13,8 +13,8 @@ class TestVSCode_startDebugging(lldbvscode_testcase.VSCodeTestCaseBase): def test_startDebugging(self): """ - Tests the "startDebugging" reverse request. It makes sure that the IDE can - start a child debug session. + Tests the "startDebugging" reverse request. It makes sure that the IDE can + start a child debug session. """ program = self.getBuildArtifact("a.out") source = "main.c" @@ -25,15 +25,17 @@ self.set_source_breakpoints(source, [breakpoint_line]) self.continue_to_next_stop() self.vscode.request_evaluate( - '`lldb-vscode startDebugging attach \'{"pid":321}\'', context='repl' + "`lldb-vscode startDebugging attach '{\"pid\":321}'", context="repl" ) + # Run an additional command to ensure the request and response were + # triggered. + self.stepOver() + self.assertEqual( - len(self.vscode.reverse_requests), - 1, - "make sure we got a reverse request" + len(self.vscode.reverse_requests), 1, "make sure we got a reverse request" ) request = self.vscode.reverse_requests[0] self.assertEqual(request["arguments"]["configuration"]["pid"], 321) - self.assertEqual(request["arguments"]["request"], "attach") \ No newline at end of file + self.assertEqual(request["arguments"]["request"], "attach") diff --git a/lldb/tools/lldb-vscode/Options.td b/lldb/tools/lldb-vscode/Options.td --- a/lldb/tools/lldb-vscode/Options.td +++ b/lldb/tools/lldb-vscode/Options.td @@ -39,3 +39,7 @@ MetaVarName<"">, HelpText<"The PID of the lldb-vscode instance that sent the launchInTerminal " "request when using --launch-target.">; + +def repl_mode: S<"repl-mode">, + MetaVarName<"">, + HelpText<"The mode for handling repl evaluation requests, supported modes: variable, command, auto.">; diff --git a/lldb/tools/lldb-vscode/README.md b/lldb/tools/lldb-vscode/README.md --- a/lldb/tools/lldb-vscode/README.md +++ b/lldb/tools/lldb-vscode/README.md @@ -236,3 +236,22 @@ ] } ``` + +## repl-mode + +Inspect or adjust the behavior of lldb-vscode repl evaluation requests. The +supported modes are `variable`, `command` and `auto`. + +* `variable` - Variable mode expressions are evaluated in the context of the + current frame. Use a `\`` prefix on the command to run an lldb command. +* `command` - Command mode expressions are evaluated as lldb commands, as a + result, values printed by lldb are always stringified representations of the + expression output. +* `auto` - Auto mode will attempt to infer if the expression represents an lldb + command or a variable expression. A heuristic is used to infer if the input + represents a variable or a command. Use a `\`` prefix to ensure an expression + is evaluated as a command. + +The initial repl-mode can be configured with the cli flag `--repl-mode=` +and may also be adjusted at runtime using the lldb command +`lldb-vscode repl-mode `. diff --git a/lldb/tools/lldb-vscode/VSCode.h b/lldb/tools/lldb-vscode/VSCode.h --- a/lldb/tools/lldb-vscode/VSCode.h +++ b/lldb/tools/lldb-vscode/VSCode.h @@ -84,6 +84,14 @@ JSONNotObject }; +enum class ReplMode { Variable = 0, Command, Auto }; + +/// The detected context of an expression based off the current repl mode. +enum class ExpressionContext { + Variable = 0, + Command, +}; + struct Variables { /// Variable_reference start index of permanent expandable variable. static constexpr int64_t PermanentVariableStartIndex = (1ll << 32); @@ -109,7 +117,7 @@ /// \return a new variableReference. /// Specify is_permanent as true for variable that should persist entire /// debug session. - int64_t GetNewVariableRefence(bool is_permanent); + int64_t GetNewVariableReference(bool is_permanent); /// \return the expandable variable corresponding with variableReference /// value of \p value. @@ -129,6 +137,11 @@ lldb::SBCommandReturnObject &result) override; }; +struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface { + bool DoExecute(lldb::SBDebugger debugger, char **command, + lldb::SBCommandReturnObject &result) override; +}; + struct VSCode { std::string debug_adaptor_path; InputStream input; @@ -173,6 +186,9 @@ std::map inflight_reverse_requests; StartDebuggingRequestHandler start_debugging_request_handler; + ReplModeRequestHandler repl_mode_request_handler; + ReplMode repl_mode; + bool auto_repl_mode_collision_warning; VSCode(); ~VSCode(); @@ -206,6 +222,9 @@ llvm::json::Value CreateTopLevelScopes(); + ExpressionContext DetectExpressionContext(lldb::SBFrame &frame, + std::string &text); + void RunLLDBCommands(llvm::StringRef prefix, const std::vector &commands); diff --git a/lldb/tools/lldb-vscode/VSCode.cpp b/lldb/tools/lldb-vscode/VSCode.cpp --- a/lldb/tools/lldb-vscode/VSCode.cpp +++ b/lldb/tools/lldb-vscode/VSCode.cpp @@ -45,7 +45,8 @@ configuration_done_sent(false), waiting_for_run_in_terminal(false), progress_event_reporter( [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }), - reverse_request_seq(0) { + reverse_request_seq(0), repl_mode(ReplMode::Auto), + auto_repl_mode_collision_warning(false) { const char *log_file_path = getenv("LLDBVSCODE_LOG"); #if defined(_WIN32) // Windows opens stdout and stdin in text mode which converts \n to 13,10 @@ -393,6 +394,57 @@ return llvm::json::Value(std::move(scopes)); } +ExpressionContext VSCode::DetectExpressionContext(lldb::SBFrame &frame, + std::string &text) { + // Include ` as an escape hatch. + if (!text.empty() && text[0] == '`') { + text = text.substr(1); + return ExpressionContext::Command; + } + + switch (repl_mode) { + case ReplMode::Variable: + return ExpressionContext::Variable; + case ReplMode::Command: + return ExpressionContext::Command; + case ReplMode::Auto: + // If the frame is invalid then there is no variables to complete, assume + // this is an lldb command instead. + if (!frame.IsValid()) { + return ExpressionContext::Command; + } + + lldb::SBCommandReturnObject result; + debugger.GetCommandInterpreter().ResolveCommand(text.data(), result); + + // If this command is a simple expression like `var + 1` check if there is + // a local variable name that is in the current expression. If so, ensure + // the expression runs in the variable context. + lldb::SBValueList variables = frame.GetVariables(true, true, true, true); + llvm::StringRef input = text; + for (uint32_t i = 0; i < variables.GetSize(); i++) { + llvm::StringRef name = variables.GetValueAtIndex(i).GetName(); + // Check both directions in case the input is a partial of a variable + // (e.g. input = `va` and local variable = `var1`). + if (input.contains(name) || name.contains(input)) { + if (!auto_repl_mode_collision_warning) { + llvm::errs() << "Variable expression '" << text + << "' is hiding an lldb command, prefix an expression " + "with ` to ensure it runs as a lldb command.\n"; + auto_repl_mode_collision_warning = true; + } + return ExpressionContext::Variable; + } + } + + if (result.Succeeded()) { + return ExpressionContext::Command; + } + } + + return ExpressionContext::Variable; +} + void VSCode::RunLLDBCommands(llvm::StringRef prefix, const std::vector &commands) { SendOutput(OutputType::Console, @@ -502,7 +554,8 @@ return true; // Success } else { if (log) - *log << "error: unhandled command \"" << command.data() << std::endl; + *log << "error: unhandled command \"" << command.data() << "\"" + << std::endl; return false; // Fail } } @@ -637,7 +690,7 @@ expandable_variables.clear(); } -int64_t Variables::GetNewVariableRefence(bool is_permanent) { +int64_t Variables::GetNewVariableReference(bool is_permanent) { if (is_permanent) return next_permanent_var_ref++; return next_temporary_var_ref++; @@ -662,7 +715,7 @@ int64_t Variables::InsertExpandableVariable(lldb::SBValue variable, bool is_permanent) { - int64_t var_ref = GetNewVariableRefence(is_permanent); + int64_t var_ref = GetNewVariableReference(is_permanent); if (is_permanent) expandable_permanent_variables.insert(std::make_pair(var_ref, variable)); else @@ -729,4 +782,51 @@ return true; } +bool ReplModeRequestHandler::DoExecute(lldb::SBDebugger debugger, + char **command, + lldb::SBCommandReturnObject &result) { + // Command format like: `repl-mode ?` + // If a new mode is not specified report the current mode. + if (!command || llvm::StringRef(command[0]).empty()) { + std::string mode; + switch (g_vsc.repl_mode) { + case ReplMode::Variable: + mode = "variable"; + break; + case ReplMode::Command: + mode = "command"; + break; + case ReplMode::Auto: + mode = "auto"; + break; + } + + result.Printf("lldb-vscode repl-mode %s.\n", mode.c_str()); + result.SetStatus(lldb::eReturnStatusSuccessFinishResult); + + return true; + } + + llvm::StringRef new_mode{command[0]}; + + if (new_mode == "variable") { + g_vsc.repl_mode = ReplMode::Variable; + } else if (new_mode == "command") { + g_vsc.repl_mode = ReplMode::Command; + } else if (new_mode == "auto") { + g_vsc.repl_mode = ReplMode::Auto; + } else { + lldb::SBStream error_message; + error_message.Printf("Invalid repl-mode '%s'. Expected one of 'variable', " + "'command' or 'auto'.\n", + new_mode.data()); + result.SetError(error_message.GetData()); + return false; + } + + result.Printf("lldb-vscode repl-mode %s set.\n", new_mode.data()); + result.SetStatus(lldb::eReturnStatusSuccessFinishNoResult); + return true; +} + } // namespace lldb_vscode diff --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp b/lldb/tools/lldb-vscode/lldb-vscode.cpp --- a/lldb/tools/lldb-vscode/lldb-vscode.cpp +++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp @@ -1065,50 +1065,62 @@ FillResponse(request, response); llvm::json::Object body; auto arguments = request.getObject("arguments"); - std::string text = std::string(GetString(arguments, "text")); + + // If we have a frame, try to set the context for variable completions. + lldb::SBFrame frame = g_vsc.GetLLDBFrame(*arguments); + if (frame.IsValid()) { + frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread()); + frame.GetThread().SetSelectedFrame(frame.GetFrameID()); + } + + std::string text = GetString(arguments, "text").str(); auto original_column = GetSigned(arguments, "column", text.size()); - auto actual_column = original_column - 1; + auto original_line = GetSigned(arguments, "line", 1); + auto offset = original_column - 1; + if (original_line > 1) { + llvm::SmallVector<::llvm::StringRef, 2> lines; + llvm::StringRef(text).split(lines, '\n'); + for (int i = 0; i < original_line - 1; i++) { + offset += lines[i].size(); + } + } llvm::json::Array targets; - // NOTE: the 'line' argument is not needed, as multiline expressions - // work well already - // TODO: support frameID. Currently - // g_vsc.debugger.GetCommandInterpreter().HandleCompletionWithDescriptions - // is frame-unaware. - - if (!text.empty() && text[0] == '`') { - text = text.substr(1); - actual_column--; - } else { + + if (g_vsc.DetectExpressionContext(frame, text) == + ExpressionContext::Variable) { char command[] = "expression -- "; text = command + text; - actual_column += strlen(command); + offset += strlen(command); } lldb::SBStringList matches; lldb::SBStringList descriptions; - g_vsc.debugger.GetCommandInterpreter().HandleCompletionWithDescriptions( - text.c_str(), actual_column, 0, -1, matches, descriptions); - size_t count = std::min((uint32_t)100, matches.GetSize()); - targets.reserve(count); - for (size_t i = 0; i < count; i++) { - std::string match = matches.GetStringAtIndex(i); - std::string description = descriptions.GetStringAtIndex(i); - - llvm::json::Object item; - - llvm::StringRef match_ref = match; - for (llvm::StringRef commit_point : {".", "->"}) { - if (match_ref.contains(commit_point)) { - match_ref = match_ref.rsplit(commit_point).second; + + if (g_vsc.debugger.GetCommandInterpreter().HandleCompletionWithDescriptions( + text.c_str(), offset, 0, 100, matches, descriptions)) { + // The first element is the common substring after the cursor position for + // all the matches. The rest of the elements are the matches so ignore the + // first result. + targets.reserve(matches.GetSize() - 1); + for (size_t i = 1; i < matches.GetSize(); i++) { + std::string match = matches.GetStringAtIndex(i); + std::string description = descriptions.GetStringAtIndex(i); + + llvm::json::Object item; + llvm::StringRef match_ref = match; + for (llvm::StringRef commit_point : {".", "->"}) { + if (match_ref.contains(commit_point)) { + match_ref = match_ref.rsplit(commit_point).second; + } } - } - EmplaceSafeString(item, "text", match_ref); + EmplaceSafeString(item, "text", match_ref); - if (description.empty()) - EmplaceSafeString(item, "label", match); - else - EmplaceSafeString(item, "label", match + " -- " + description); + if (description.empty()) + EmplaceSafeString(item, "label", match); + else + EmplaceSafeString(item, "label", match + " -- " + description); - targets.emplace_back(std::move(item)); + targets.emplace_back(std::move(item)); + } } body.try_emplace("targets", std::move(targets)); @@ -1223,12 +1235,17 @@ llvm::json::Object body; auto arguments = request.getObject("arguments"); lldb::SBFrame frame = g_vsc.GetLLDBFrame(*arguments); - const auto expression = GetString(arguments, "expression"); + std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); - if (!expression.empty() && expression[0] == '`') { - auto result = - RunLLDBCommands(llvm::StringRef(), {std::string(expression.substr(1))}); + if (context == "repl" && g_vsc.DetectExpressionContext(frame, expression) == + ExpressionContext::Command) { + // If we're evaluating a command relative to the current frame, set the + // focus_tid to the current frame for any thread related events. + if (frame.IsValid()) { + g_vsc.focus_tid = frame.GetThread().GetThreadID(); + } + auto result = RunLLDBCommands(llvm::StringRef(), {std::string(expression)}); EmplaceSafeString(body, "result", result); body.try_emplace("variablesReference", (int64_t)0); } else { @@ -1472,11 +1489,17 @@ g_vsc.debugger = lldb::SBDebugger::Create(source_init_file, log_cb, nullptr); auto cmd = g_vsc.debugger.GetCommandInterpreter().AddMultiwordCommand( - "lldb-vscode", nullptr); + "lldb-vscode", "Commands for managing lldb-vscode."); + if (GetBoolean(arguments, "supportsStartDebuggingRequest", false)) { + cmd.AddCommand( + "startDebugging", &g_vsc.start_debugging_request_handler, + "Sends a startDebugging request from the debug adapter to the client " + "to " + "start a child debug session of the same type as the caller."); + } cmd.AddCommand( - "startDebugging", &g_vsc.start_debugging_request_handler, - "Sends a startDebugging request from the debug adapter to the client to " - "start a child debug session of the same type as the caller."); + "repl-mode", &g_vsc.repl_mode_request_handler, + "Get or set the repl behavior of vscode-lldb evaluation requests."); g_vsc.progress_event_thread = std::thread(ProgressEventThreadFunction); @@ -1519,22 +1542,16 @@ body.try_emplace("supportsGotoTargetsRequest", false); // The debug adapter supports the stepInTargetsRequest. body.try_emplace("supportsStepInTargetsRequest", false); - // We need to improve the current implementation of completions in order to - // enable it again. For some context, this is how VSCode works: - // - VSCode sends a completion request whenever chars are added, the user - // triggers completion manually via CTRL-space or similar mechanisms, but - // not when there's a deletion. Besides, VSCode doesn't let us know which - // of these events we are handling. What is more, the use can paste or cut - // sections of the text arbitrarily. - // https://github.com/microsoft/vscode/issues/89531 tracks part of the - // issue just mentioned. - // This behavior causes many problems with the current way completion is - // implemented in lldb-vscode, as these requests could be really expensive, - // blocking the debugger, and there could be many concurrent requests unless - // the user types very slowly... We need to address this specific issue, or - // at least trigger completion only when the user explicitly wants it, which - // is the behavior of LLDB CLI, that expects a TAB. - body.try_emplace("supportsCompletionsRequest", false); + // The debug adapter supports the completions request. + body.try_emplace("supportsCompletionsRequest", true); + + llvm::json::Array completion_characters; + completion_characters.emplace_back("."); + completion_characters.emplace_back(" "); + completion_characters.emplace_back("\t"); + body.try_emplace("completionTriggerCharacters", + std::move(completion_characters)); + // The debug adapter supports the modules request. body.try_emplace("supportsModulesRequest", true); // The set of additional module information exposed by the debug adapter. @@ -2093,6 +2110,7 @@ frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread()); frame.GetThread().SetSelectedFrame(frame.GetFrameID()); } + g_vsc.variables.locals = frame.GetVariables(/*arguments=*/true, /*locals=*/true, /*statics=*/false, @@ -3406,6 +3424,23 @@ return EXIT_SUCCESS; } + if (input_args.hasArg(OPT_repl_mode)) { + llvm::opt::Arg *repl_mode = input_args.getLastArg(OPT_repl_mode); + llvm::StringRef repl_mode_value = repl_mode->getValue(); + if (repl_mode_value == "auto") { + g_vsc.repl_mode = ReplMode::Auto; + } else if (repl_mode_value == "variable") { + g_vsc.repl_mode = ReplMode::Variable; + } else if (repl_mode_value == "command") { + g_vsc.repl_mode = ReplMode::Command; + } else { + llvm::errs() + << "'" << repl_mode_value + << "' is not a valid option, use 'variable', 'command' or 'auto'.\n"; + return EXIT_FAILURE; + } + } + if (llvm::opt::Arg *target_arg = input_args.getLastArg(OPT_launch_target)) { if (llvm::opt::Arg *comm_file = input_args.getLastArg(OPT_comm_file)) { lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;