llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: John Harrison (ashgti) <details> <summary>Changes</summary> Reverts llvm/llvm-project#<!-- -->181261 Breaking builds on linux, reverting while I investigate. --- Patch is 68.42 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/181930.diff 23 Files Affected: - (modified) lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py (+13-33) - (modified) lldb/test/API/tools/lldb-dap/variables/main.cpp (-4) - (modified) lldb/tools/lldb-dap/DAP.cpp (+20-17) - (modified) lldb/tools/lldb-dap/DAP.h (+4-6) - (modified) lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp (+1-1) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+8-6) - (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+5-5) - (modified) lldb/tools/lldb-dap/JSONUtils.h (+3-3) - (modified) lldb/tools/lldb-dap/LLDBUtils.cpp (+2-2) - (modified) lldb/tools/lldb-dap/LLDBUtils.h (+2-3) - (modified) lldb/tools/lldb-dap/Protocol/DAPTypes.h (+4-4) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp (+13-42) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolBase.h (+9-99) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+4-6) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+50-50) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+67-67) - (modified) lldb/tools/lldb-dap/ProtocolUtils.cpp (+3-2) - (modified) lldb/tools/lldb-dap/tool/lldb-dap.cpp (+2-2) - (modified) lldb/unittests/DAP/CMakeLists.txt (-1) - (removed) lldb/unittests/DAP/ProtocolBaseTest.cpp (-29) - (modified) lldb/unittests/DAP/ProtocolTypesTest.cpp (+1-1) - (modified) lldb/unittests/DAP/TestBase.cpp (+1-1) - (modified) lldb/unittests/DAP/VariablesTest.cpp (+1-2) ``````````diff diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py index 9de3e1af2dd20..10c67a94407e6 100644 --- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py +++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py @@ -193,8 +193,6 @@ def do_test_scopes_variables_setVariable_evaluate( }, "readOnly": True, }, - "valid_str": {}, - "malformed_str": {}, "x": {"equals": {"type": "int"}}, } @@ -347,9 +345,9 @@ def do_test_scopes_variables_setVariable_evaluate( verify_locals["argc"]["equals"]["value"] = "123" verify_locals["pt"]["children"]["x"]["equals"]["value"] = "111" - verify_locals["x @ main.cpp:23"] = {"equals": {"type": "int", "value": "89"}} - verify_locals["x @ main.cpp:25"] = {"equals": {"type": "int", "value": "42"}} - verify_locals["x @ main.cpp:27"] = {"equals": {"type": "int", "value": "72"}} + verify_locals["x @ main.cpp:19"] = {"equals": {"type": "int", "value": "89"}} + verify_locals["x @ main.cpp:21"] = {"equals": {"type": "int", "value": "42"}} + verify_locals["x @ main.cpp:23"] = {"equals": {"type": "int", "value": "72"}} self.verify_variables(verify_locals, self.dap_server.get_local_variables()) @@ -357,22 +355,22 @@ def do_test_scopes_variables_setVariable_evaluate( self.assertFalse(self.set_local("x2", 9)["success"]) self.assertFalse(self.set_local("x @ main.cpp:0", 9)["success"]) - self.assertTrue(self.set_local("x @ main.cpp:23", 19)["success"]) - self.assertTrue(self.set_local("x @ main.cpp:25", 21)["success"]) - self.assertTrue(self.set_local("x @ main.cpp:27", 23)["success"]) + self.assertTrue(self.set_local("x @ main.cpp:19", 19)["success"]) + self.assertTrue(self.set_local("x @ main.cpp:21", 21)["success"]) + self.assertTrue(self.set_local("x @ main.cpp:23", 23)["success"]) # The following should have no effect - self.assertFalse(self.set_local("x @ main.cpp:27", "invalid")["success"]) + self.assertFalse(self.set_local("x @ main.cpp:23", "invalid")["success"]) - verify_locals["x @ main.cpp:23"]["equals"]["value"] = "19" - verify_locals["x @ main.cpp:25"]["equals"]["value"] = "21" - verify_locals["x @ main.cpp:27"]["equals"]["value"] = "23" + verify_locals["x @ main.cpp:19"]["equals"]["value"] = "19" + verify_locals["x @ main.cpp:21"]["equals"]["value"] = "21" + verify_locals["x @ main.cpp:23"]["equals"]["value"] = "23" self.verify_variables(verify_locals, self.dap_server.get_local_variables()) # The plain x variable shold refer to the innermost x self.assertTrue(self.set_local("x", 22)["success"]) - verify_locals["x @ main.cpp:27"]["equals"]["value"] = "22" + verify_locals["x @ main.cpp:23"]["equals"]["value"] = "22" self.verify_variables(verify_locals, self.dap_server.get_local_variables()) @@ -389,9 +387,9 @@ def do_test_scopes_variables_setVariable_evaluate( names = [var["name"] for var in locals] # The first shadowed x shouldn't have a suffix anymore verify_locals["x"] = {"equals": {"type": "int", "value": "19"}} + self.assertNotIn("x @ main.cpp:19", names) + self.assertNotIn("x @ main.cpp:21", names) self.assertNotIn("x @ main.cpp:23", names) - self.assertNotIn("x @ main.cpp:25", names) - self.assertNotIn("x @ main.cpp:27", names) self.verify_variables(verify_locals, locals) @@ -462,22 +460,6 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo }, "readOnly": True, }, - "valid_str": { - "equals": { - "type": "const char *", - }, - "matches": { - "value": r'0x\w+ "πΆπ°LπΎπ CππΌπ΄π"', - }, - }, - "malformed_str": { - "equals": { - "type": "const char *", - }, - "matches": { - "value": r'0x\w+ "lone trailing \\x81\\x82 bytes"', - }, - }, "x": { "equals": {"type": "int"}, "missing": ["indexedVariables"], @@ -718,8 +700,6 @@ def test_return_variables(self): "argc": {}, "argv": {}, "pt": {"readOnly": True}, - "valid_str": {}, - "malformed_str": {}, "x": {}, "return_result": {"equals": {"type": "int"}}, } diff --git a/lldb/test/API/tools/lldb-dap/variables/main.cpp b/lldb/test/API/tools/lldb-dap/variables/main.cpp index 04fc62f02c22f..0e363001f2f42 100644 --- a/lldb/test/API/tools/lldb-dap/variables/main.cpp +++ b/lldb/test/API/tools/lldb-dap/variables/main.cpp @@ -5,7 +5,6 @@ struct PointType { int y; int buffer[BUFFER_SIZE]; }; -#include <cstdio> #include <vector> int g_global = 123; static int s_global = 234; @@ -17,9 +16,6 @@ int main(int argc, char const *argv[]) { PointType pt = {11, 22, {0}}; for (int i = 0; i < BUFFER_SIZE; ++i) pt.buffer[i] = i; - const char *valid_str = "πΆπ°LπΎπ CππΌπ΄π"; - const char *malformed_str = "lone trailing \x81\x82 bytes"; - printf("print malformed utf8 %s %s\n", valid_str, malformed_str); int x = s_global - g_global - pt.y; // breakpoint 1 { int x = 42; diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index fcfd1da3878e8..6d5e175ae29e0 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -124,7 +124,7 @@ static std::string capitalize(llvm::StringRef str) { llvm::StringRef DAP::debug_adapter_path = ""; DAP::DAP(Log &log, const ReplMode default_repl_mode, - const std::vector<String> &pre_init_commands, bool no_lldbinit, + std::vector<std::string> pre_init_commands, bool no_lldbinit, llvm::StringRef client_name, DAPTransport &transport, MainLoop &loop) : log(log), transport(transport), reference_storage(log), broadcaster("lldb-dap"), @@ -132,7 +132,7 @@ DAP::DAP(Log &log, const ReplMode default_repl_mode, [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }), repl_mode(default_repl_mode), no_lldbinit(no_lldbinit), m_client_name(client_name), m_loop(loop) { - configuration.preInitCommands = pre_init_commands; + configuration.preInitCommands = std::move(pre_init_commands); RegisterRequests(); } @@ -413,10 +413,9 @@ void DAP::SendOutput(OutputType o, const llvm::StringRef output) { if (end == llvm::StringRef::npos) end = output.size() - 1; llvm::json::Object event(CreateEventObject("output")); - llvm::json::Object body{ - {"category", category}, - {"output", protocol::String(output.slice(idx, end + 1))}, - }; + llvm::json::Object body; + body.try_emplace("category", category); + EmplaceSafeString(body, "output", output.slice(idx, end + 1).str()); event.try_emplace("body", std::move(body)); SendJSON(llvm::json::Value(std::move(event))); idx = end + 1; @@ -724,7 +723,7 @@ DAP::ResolveAssemblySource(lldb::SBAddress address) { } bool DAP::RunLLDBCommands(llvm::StringRef prefix, - llvm::ArrayRef<String> commands) { + llvm::ArrayRef<std::string> commands) { bool required_command_failed = false; std::string output = ::RunLLDBCommands( debugger, prefix, commands, required_command_failed, @@ -743,13 +742,15 @@ static llvm::Error createRunLLDBCommandsErrorMessage(llvm::StringRef category) { .c_str()); } -llvm::Error DAP::RunAttachCommands(llvm::ArrayRef<String> attach_commands) { +llvm::Error +DAP::RunAttachCommands(llvm::ArrayRef<std::string> attach_commands) { if (!RunLLDBCommands("Running attachCommands:", attach_commands)) return createRunLLDBCommandsErrorMessage("attach"); return llvm::Error::success(); } -llvm::Error DAP::RunLaunchCommands(llvm::ArrayRef<String> launch_commands) { +llvm::Error +DAP::RunLaunchCommands(llvm::ArrayRef<std::string> launch_commands) { if (!RunLLDBCommands("Running launchCommands:", launch_commands)) return createRunLLDBCommandsErrorMessage("launch"); return llvm::Error::success(); @@ -801,9 +802,9 @@ lldb::SBTarget DAP::CreateTarget(lldb::SBError &error) { // omitted at all), so it is good to leave the user an opportunity to specify // those. Any of those three can be left empty. auto target = this->debugger.CreateTarget( - /*filename=*/configuration.program.c_str(), - /*target_triple=*/configuration.targetTriple.c_str(), - /*platform_name=*/configuration.platformName.c_str(), + /*filename=*/configuration.program.data(), + /*target_triple=*/configuration.targetTriple.data(), + /*platform_name=*/configuration.platformName.data(), /*add_dependent_modules=*/true, // Add dependent modules. error); @@ -848,7 +849,8 @@ bool DAP::HandleObject(const Message &M) { }); auto handler_pos = request_handlers.find(req->command); - dispatcher.Set("client_data", "request_command:" + req->command); + dispatcher.Set("client_data", + llvm::Twine("request_command:", req->command).str()); if (handler_pos != request_handlers.end()) { handler_pos->second->Run(*req); } else { @@ -876,13 +878,14 @@ bool DAP::HandleObject(const Message &M) { // Result should be given, use null if not. if (resp->success) { (*response_handler)(resp->body); - dispatcher.Set("client_data", "response_command:" + resp->command); + dispatcher.Set("client_data", + llvm::Twine("response_command:", resp->command).str()); } else { llvm::StringRef message = "Unknown error, response failed"; if (resp->message) { message = std::visit(llvm::makeVisitor( - [](const String &message) -> llvm::StringRef { + [](const std::string &message) -> llvm::StringRef { return message; }, [](const protocol::ResponseMessage &message) @@ -966,7 +969,7 @@ void DAP::ClearCancelRequest(const CancelArguments &args) { template <typename T> static std::optional<T> getArgumentsIfRequest(const Request &req, - const protocol::String &command) { + llvm::StringLiteral command) { if (req.command != command) return std::nullopt; @@ -1264,7 +1267,7 @@ protocol::Capabilities DAP::GetCapabilities() { capabilities.exceptionBreakpointFilters = std::move(filters); // FIXME: This should be registered based on the supported languages? - std::vector<String> completion_characters; + std::vector<std::string> completion_characters; completion_characters.emplace_back("."); // FIXME: I wonder if we should remove this key... its very aggressive // triggering and accepting completions. diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 9457628aa0f42..a164cc484f4be 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -194,7 +194,7 @@ struct DAP final : public DAPTransport::MessageHandler { /// \param[in] loop /// Main loop associated with this instance. DAP(Log &log, const ReplMode default_repl_mode, - const std::vector<protocol::String> &pre_init_commands, bool no_lldbinit, + std::vector<std::string> pre_init_commands, bool no_lldbinit, llvm::StringRef client_name, DAPTransport &transport, lldb_private::MainLoop &loop); @@ -314,12 +314,10 @@ struct DAP final : public DAPTransport::MessageHandler { /// \b false if a fatal error was found while executing these commands, /// according to the rules of \a LLDBUtils::RunLLDBCommands. bool RunLLDBCommands(llvm::StringRef prefix, - llvm::ArrayRef<protocol::String> commands); + llvm::ArrayRef<std::string> commands); - llvm::Error - RunAttachCommands(llvm::ArrayRef<protocol::String> attach_commands); - llvm::Error - RunLaunchCommands(llvm::ArrayRef<protocol::String> launch_commands); + llvm::Error RunAttachCommands(llvm::ArrayRef<std::string> attach_commands); + llvm::Error RunLaunchCommands(llvm::ArrayRef<std::string> launch_commands); llvm::Error RunPreInitCommands(); llvm::Error RunInitCommands(); llvm::Error RunPreRunCommands(); diff --git a/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp index d24072c8cc05d..e1a3b15b4697b 100644 --- a/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp @@ -30,7 +30,7 @@ llvm::Expected<CompileUnitsResponseBody> CompileUnitsRequestHandler::Run( int num_modules = dap.target.GetNumModules(); for (int i = 0; i < num_modules; i++) { auto curr_module = dap.target.GetModuleAtIndex(i); - if (args->moduleId == curr_module.GetUUIDString()) { + if (args->moduleId == llvm::StringRef(curr_module.GetUUIDString())) { int num_units = curr_module.GetNumCompileUnits(); for (int j = 0; j < num_units; j++) { auto curr_unit = curr_module.GetCompileUnitAtIndex(j); diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp index 5e8c2163c838f..47ae9a7195a7d 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp @@ -37,7 +37,8 @@ using namespace lldb_dap::protocol; namespace lldb_dap { -static std::vector<const char *> MakeArgv(const llvm::ArrayRef<String> &strs) { +static std::vector<const char *> +MakeArgv(const llvm::ArrayRef<std::string> &strs) { // Create and return an array of "const char *", one for each C string in // "strs" and terminate the list with a NULL. This can be used for argument // vectors (argv) or environment vectors (envp) like those passed to the @@ -59,8 +60,9 @@ static uint32_t SetLaunchFlag(uint32_t flags, bool flag, return flags; } -static void SetupIORedirection(const std::vector<std::optional<String>> &stdio, - lldb::SBLaunchInfo &launch_info) { +static void +SetupIORedirection(const std::vector<std::optional<std::string>> &stdio, + lldb::SBLaunchInfo &launch_info) { for (const auto &[idx, value_opt] : llvm::enumerate(stdio)) { if (!value_opt) continue; @@ -189,7 +191,7 @@ void BaseRequestHandler::Run(const Request &request) { llvm::Error BaseRequestHandler::LaunchProcess( const protocol::LaunchRequestArguments &arguments) const { - const std::vector<String> &launchCommands = arguments.launchCommands; + const std::vector<std::string> &launchCommands = arguments.launchCommands; // Instantiate a launch info instance for the target. auto launch_info = dap.target.GetLaunchInfo(); @@ -338,8 +340,8 @@ void BaseRequestHandler::BuildErrorResponse( error_message.format = err.getMessage(); error_message.showUser = err.getShowUser(); error_message.id = err.convertToErrorCode().value(); - error_message.url = err.getURL().value_or(""); - error_message.urlLabel = err.getURLLabel().value_or(""); + error_message.url = err.getURL(); + error_message.urlLabel = err.getURLLabel(); protocol::ErrorResponseBody body; body.error = error_message; diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 460d2a46049c6..5bcc2f9c71c2d 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -462,10 +462,10 @@ std::pair<int64_t, bool> UnpackLocation(int64_t location_id) { /// See /// https://microsoft.github.io/debug-adapter-protocol/specification#Reverse_Requests_RunInTerminal llvm::json::Object CreateRunInTerminalReverseRequest( - llvm::StringRef program, const std::vector<protocol::String> &args, - const llvm::StringMap<protocol::String> &env, llvm::StringRef cwd, + llvm::StringRef program, const std::vector<std::string> &args, + const llvm::StringMap<std::string> &env, llvm::StringRef cwd, llvm::StringRef comm_file, lldb::pid_t debugger_pid, - const std::vector<std::optional<protocol::String>> &stdio, bool external) { + const std::vector<std::optional<std::string>> &stdio, bool external) { llvm::json::Object run_in_terminal_args; if (external) { // This indicates the IDE to open an external terminal window. @@ -488,10 +488,10 @@ llvm::json::Object CreateRunInTerminalReverseRequest( std::stringstream ss; std::string_view delimiter; - for (const std::optional<protocol::String> &file : stdio) { + for (const std::optional<std::string> &file : stdio) { ss << std::exchange(delimiter, ":"); if (file) - ss << file->str(); + ss << *file; } req_args.push_back(ss.str()); } diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index e12fb5b937bac..232b1810a3cf4 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -322,10 +322,10 @@ std::pair<int64_t, bool> UnpackLocation(int64_t location_id); /// A "runInTerminal" JSON object that follows the specification outlined by /// Microsoft. llvm::json::Object CreateRunInTerminalReverseRequest( - llvm::StringRef program, const std::vector<protocol::String> &args, - const llvm::StringMap<protocol::String> &env, llvm::StringRef cwd, + llvm::StringRef program, const std::vector<std::string> &args, + const llvm::StringMap<std::string> &env, llvm::StringRef cwd, llvm::StringRef comm_file, lldb::pid_t debugger_pid, - const std::vector<std::optional<protocol::String>> &stdio, bool external); + const std::vector<std::optional<std::string>> &stdio, bool external); /// Create a "Terminated" JSON object that contains statistics /// diff --git a/lldb/tools/lldb-dap/LLDBUtils.cpp b/lldb/tools/lldb-dap/LLDBUtils.cpp index 7e69813477b29..e7407e9da9efb 100644 --- a/lldb/tools/lldb-dap/LLDBUtils.cpp +++ b/lldb/tools/lldb-dap/LLDBUtils.cpp @@ -32,7 +32,7 @@ namespace lldb_dap { bool RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix, - const llvm::ArrayRef<protocol::String> &commands, + const llvm::ArrayRef<std::string> &commands, llvm::raw_ostream &strm, bool parse_command_directives, bool echo_commands) { if (commands.empty()) @@ -115,7 +115,7 @@ bool RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix, } std::string RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix, - const llvm::ArrayRef<protocol::String> &commands, + const llvm::ArrayRef<std::string> &commands, bool &required_command_failed, bool parse_command_directives, bool echo_commands) { required_command_failed = false; diff --git a/lldb/tools/lldb-dap/LLDBUtils.h b/lldb/tools/lldb-dap/LLDBUtils.h index 37757ada7ab4d..19174ba59654d 100644 --- a/lldb/tools/lldb-dap/LLDBUtils.h +++ b/lldb/tools/lldb-dap/LLDBUtils.h @@ -10,7 +10,6 @@ #define LLDB_TOOLS_LLDB_DAP_LLDBUTILS_H #include "DAPForward.h" -#include "Protocol/ProtocolBase.h" #include "lldb/API/SBDebugger.h" #include "lldb/API/SBEnvironment.h" #include "lldb/API/SBError.h" @@ -64,7 +63,7 @@ namespace lldb_dap { /// \b true, unless a command prefixed with \b ! fails and parsing of /// command directives is enabled. bool RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix, - const llvm::ArrayRef<protocol::String> &commands, + const llvm::ArrayRef<std::string> &commands, llvm::raw_ostream &strm, bool parse_command_directives, bool echo_commands); @@ -98,7 +97,7 @@ bool RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix, /// A std::string tha... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/181930 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
