Author: Shubham Sandeep Rastogi Date: 2023-07-11T15:16:03-07:00 New Revision: 78af051ff0e16db73201b1370e34206a6a4c1b93
URL: https://github.com/llvm/llvm-project/commit/78af051ff0e16db73201b1370e34206a6a4c1b93 DIFF: https://github.com/llvm/llvm-project/commit/78af051ff0e16db73201b1370e34206a6a4c1b93.diff LOG: Revert "[lldb-vscode] Creating a new flag for adjusting the behavior of evaluation repl expressions to allow users to more easily invoke lldb commands." This reverts commit 16317f1ced77e1d8711188f2fcc6c86a783d9c56. Reverting because of test failures: ******************** 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 https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/57586/ Added: Modified: lldb/tools/lldb-vscode/Options.td lldb/tools/lldb-vscode/README.md lldb/tools/lldb-vscode/VSCode.cpp lldb/tools/lldb-vscode/VSCode.h lldb/tools/lldb-vscode/lldb-vscode.cpp Removed: ################################################################################ diff --git a/lldb/tools/lldb-vscode/Options.td b/lldb/tools/lldb-vscode/Options.td index 8c51a994ebc270..a73671ad749c47 100644 --- a/lldb/tools/lldb-vscode/Options.td +++ b/lldb/tools/lldb-vscode/Options.td @@ -39,7 +39,3 @@ def debugger_pid: Separate<["--", "-"], "debugger-pid">, MetaVarName<"<pid>">, HelpText<"The PID of the lldb-vscode instance that sent the launchInTerminal " "request when using --launch-target.">; - -def repl_mode: S<"repl-mode">, - MetaVarName<"<mode>">, - 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 index 154ccefc5f5979..cd249ed2fac7c6 100644 --- a/lldb/tools/lldb-vscode/README.md +++ b/lldb/tools/lldb-vscode/README.md @@ -236,22 +236,3 @@ This will launch a server and then request a child debug session for a client. ] } ``` - -## 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=<mode>` -and may also be adjusted at runtime using the lldb command -`lldb-vscode repl-mode <mode>`. diff --git a/lldb/tools/lldb-vscode/VSCode.cpp b/lldb/tools/lldb-vscode/VSCode.cpp index f007e09eee1e1d..b1c6817c2128b9 100644 --- a/lldb/tools/lldb-vscode/VSCode.cpp +++ b/lldb/tools/lldb-vscode/VSCode.cpp @@ -44,8 +44,7 @@ VSCode::VSCode() configuration_done_sent(false), waiting_for_run_in_terminal(false), progress_event_reporter( [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }), - reverse_request_seq(0), repl_mode(ReplMode::Auto), - auto_repl_mode_collision_warning(false) { + reverse_request_seq(0) { 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,50 +392,6 @@ llvm::json::Value VSCode::CreateTopLevelScopes() { 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 (!frame.IsValid()) { - return ExpressionContext::Command; - } - - lldb::SBValue value = - frame.GetValueForVariablePath(text.data(), lldb::eDynamicDontRunTarget); - - lldb::SBCommandReturnObject result; - debugger.GetCommandInterpreter().ResolveCommand(text.data(), result); - - if (value.GetError().Success()) { - // Check if the expression is both a local variable and an lldb command. - if (result.Succeeded() && !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<std::string> &commands) { SendOutput(OutputType::Console, @@ -546,8 +501,7 @@ bool VSCode::HandleObject(const llvm::json::Object &object) { 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 } } @@ -682,7 +636,7 @@ void Variables::Clear() { expandable_variables.clear(); } -int64_t Variables::GetNewVariableReference(bool is_permanent) { +int64_t Variables::GetNewVariableRefence(bool is_permanent) { if (is_permanent) return next_permanent_var_ref++; return next_temporary_var_ref++; @@ -707,7 +661,7 @@ lldb::SBValue Variables::GetVariable(int64_t var_ref) const { int64_t Variables::InsertExpandableVariable(lldb::SBValue variable, bool is_permanent) { - int64_t var_ref = GetNewVariableReference(is_permanent); + int64_t var_ref = GetNewVariableRefence(is_permanent); if (is_permanent) expandable_permanent_variables.insert(std::make_pair(var_ref, variable)); else @@ -774,51 +728,4 @@ bool StartDebuggingRequestHandler::DoExecute( return true; } -bool ReplModeRequestHandler::DoExecute(lldb::SBDebugger debugger, - char **command, - lldb::SBCommandReturnObject &result) { - // Command format like: `repl-mode <variable|command|auto>?` - // 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/VSCode.h b/lldb/tools/lldb-vscode/VSCode.h index 4a2779fa4972a8..2d67da2bbc092e 100644 --- a/lldb/tools/lldb-vscode/VSCode.h +++ b/lldb/tools/lldb-vscode/VSCode.h @@ -84,14 +84,6 @@ enum class PacketStatus { 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); @@ -117,7 +109,7 @@ struct Variables { /// \return a new variableReference. /// Specify is_permanent as true for variable that should persist entire /// debug session. - int64_t GetNewVariableReference(bool is_permanent); + int64_t GetNewVariableRefence(bool is_permanent); /// \return the expandable variable corresponding with variableReference /// value of \p value. @@ -137,11 +129,6 @@ struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface { 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; @@ -186,9 +173,6 @@ struct VSCode { std::map<int /* request_seq */, ResponseCallback /* reply handler */> inflight_reverse_requests; StartDebuggingRequestHandler start_debugging_request_handler; - ReplModeRequestHandler repl_mode_request_handler; - ReplMode repl_mode; - bool auto_repl_mode_collision_warning; VSCode(); ~VSCode(); @@ -222,9 +206,6 @@ struct VSCode { llvm::json::Value CreateTopLevelScopes(); - ExpressionContext DetectExpressionContext(lldb::SBFrame &frame, - std::string &text); - void RunLLDBCommands(llvm::StringRef prefix, const std::vector<std::string> &commands); diff --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp b/lldb/tools/lldb-vscode/lldb-vscode.cpp index 8f82ab8e2e3e16..5815b0d22506b3 100644 --- a/lldb/tools/lldb-vscode/lldb-vscode.cpp +++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp @@ -1065,63 +1065,50 @@ void request_completions(const llvm::json::Object &request) { FillResponse(request, response); llvm::json::Object body; auto arguments = request.getObject("arguments"); - - // 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 = std::string(GetString(arguments, "text")); auto original_column = GetSigned(arguments, "column", text.size()); - auto original_line = GetSigned(arguments, "line", 1); - auto offset = original_column - 1; - if (original_line > 1) { - llvm::StringRef text_ref{text}; - ::llvm::SmallVector<::llvm::StringRef, 2> lines; - text_ref.split(lines, '\n'); - for (int i = 0; i < original_line - 1; i++) { - offset += lines[i].size(); - } - } + auto actual_column = original_column - 1; llvm::json::Array targets; - - if (g_vsc.DetectExpressionContext(frame, text) == - ExpressionContext::Variable) { - char command[] = "frame variable "; + // 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 { + char command[] = "expression -- "; text = command + text; - offset += strlen(command); + actual_column += strlen(command); } lldb::SBStringList matches; lldb::SBStringList descriptions; - - 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. - targets.reserve(matches.GetSize() - 1); - std::string common_pattern = matches.GetStringAtIndex(0); - 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; - } + 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; } - 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)); @@ -1236,17 +1223,12 @@ void request_evaluate(const llvm::json::Object &request) { llvm::json::Object body; auto arguments = request.getObject("arguments"); lldb::SBFrame frame = g_vsc.GetLLDBFrame(*arguments); - std::string expression = GetString(arguments, "expression").str(); + const auto expression = GetString(arguments, "expression"); llvm::StringRef context = GetString(arguments, "context"); - 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)}); + if (!expression.empty() && expression[0] == '`') { + auto result = + RunLLDBCommands(llvm::StringRef(), {std::string(expression.substr(1))}); EmplaceSafeString(body, "result", result); body.try_emplace("variablesReference", (int64_t)0); } else { @@ -1490,17 +1472,11 @@ void request_initialize(const llvm::json::Object &request) { g_vsc.debugger = lldb::SBDebugger::Create(source_init_file, log_cb, nullptr); auto cmd = g_vsc.debugger.GetCommandInterpreter().AddMultiwordCommand( - "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."); - } + "lldb-vscode", nullptr); cmd.AddCommand( - "repl-mode", &g_vsc.repl_mode_request_handler, - "Get or set the repl behavior of vscode-lldb evaluation requests."); + "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."); g_vsc.progress_event_thread = std::thread(ProgressEventThreadFunction); @@ -1543,16 +1519,22 @@ void request_initialize(const llvm::json::Object &request) { body.try_emplace("supportsGotoTargetsRequest", false); // The debug adapter supports the stepInTargetsRequest. body.try_emplace("supportsStepInTargetsRequest", 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)); - + // 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 modules request. body.try_emplace("supportsModulesRequest", true); // The set of additional module information exposed by the debug adapter. @@ -2111,7 +2093,6 @@ void request_scopes(const llvm::json::Object &request) { frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread()); frame.GetThread().SetSelectedFrame(frame.GetFrameID()); } - g_vsc.variables.locals = frame.GetVariables(/*arguments=*/true, /*locals=*/true, /*statics=*/false, @@ -3425,23 +3406,6 @@ int main(int argc, char *argv[]) { 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; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
