llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: John Harrison (ashgti) <details> <summary>Changes</summary> Updates `InitializeRequestArguments` to correctly follow the spec, see https://microsoft.github.io/debug-adapter-protocol/specification#Requests_Initialize. This should correct which fields are tracked as optional and simplifies some of the types to make sure they're meaningful (e.g. an `optional<bool>` isn't anymore helpful than a `bool` since undefined and false are basically equivalent). Fixes #<!-- -->170171 --- Full diff: https://github.com/llvm/llvm-project/pull/170350.diff 4 Files Affected: - (modified) lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp (+1-1) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+6-5) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+6-6) - (modified) lldb/unittests/DAP/ProtocolRequestsTest.cpp (+65-1) ``````````diff diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp index 53e1810a5b0e0..2d30e089447f1 100644 --- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp @@ -23,7 +23,7 @@ llvm::Expected<InitializeResponse> InitializeRequestHandler::Run( const InitializeRequestArguments &arguments) const { // Store initialization arguments for later use in Launch/Attach. dap.clientFeatures = arguments.supportedFeatures; - dap.sourceInitFile = arguments.lldbExtSourceInitFile.value_or(true); + dap.sourceInitFile = arguments.lldbExtSourceInitFile; return dap.GetCapabilities(); } diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp index d53a520ade39b..0a1d580bffd68 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -216,12 +216,13 @@ bool fromJSON(const json::Value &Params, InitializeRequestArguments &IRA, } return OM.map("adapterID", IRA.adapterID) && - OM.map("clientID", IRA.clientID) && - OM.map("clientName", IRA.clientName) && OM.map("locale", IRA.locale) && - OM.map("linesStartAt1", IRA.linesStartAt1) && - OM.map("columnsStartAt1", IRA.columnsStartAt1) && + OM.mapOptional("clientID", IRA.clientID) && + OM.mapOptional("clientName", IRA.clientName) && + OM.mapOptional("locale", IRA.locale) && + OM.mapOptional("linesStartAt1", IRA.linesStartAt1) && + OM.mapOptional("columnsStartAt1", IRA.columnsStartAt1) && OM.mapOptional("pathFormat", IRA.pathFormat) && - OM.map("$__lldb_sourceInitFile", IRA.lldbExtSourceInitFile); + OM.mapOptional("$__lldb_sourceInitFile", IRA.lldbExtSourceInitFile); } bool fromJSON(const json::Value &Params, Configuration &C, json::Path P) { diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 37fc2465f6a05..6a85033ae7ef2 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -108,23 +108,23 @@ struct InitializeRequestArguments { std::string adapterID; /// The ID of the client using this adapter. - std::optional<std::string> clientID; + std::string clientID; /// The human-readable name of the client using this adapter. - std::optional<std::string> clientName; + std::string clientName; /// The ISO-639 locale of the client using this adapter, e.g. en-US or de-CH. - std::optional<std::string> locale; + std::string locale; /// Determines in what format paths are specified. The default is `path`, /// which is the native format. PathFormat pathFormat = ePatFormatPath; /// If true all line numbers are 1-based (default). - std::optional<bool> linesStartAt1; + bool linesStartAt1 = true; /// If true all column numbers are 1-based (default). - std::optional<bool> columnsStartAt1; + bool columnsStartAt1 = true; /// The set of supported features reported by the client. llvm::DenseSet<ClientFeature> supportedFeatures; @@ -133,7 +133,7 @@ struct InitializeRequestArguments { /// @{ /// Source init files when initializing lldb::SBDebugger. - std::optional<bool> lldbExtSourceInitFile; + bool lldbExtSourceInitFile = true; /// @} }; diff --git a/lldb/unittests/DAP/ProtocolRequestsTest.cpp b/lldb/unittests/DAP/ProtocolRequestsTest.cpp index ba9aef1e5fcc5..a74c369924b8e 100644 --- a/lldb/unittests/DAP/ProtocolRequestsTest.cpp +++ b/lldb/unittests/DAP/ProtocolRequestsTest.cpp @@ -77,7 +77,7 @@ TEST(ProtocolRequestsTest, EvaluateArguments) { EXPECT_EQ(expected->expression, "hello world"); EXPECT_EQ(expected->context, eEvaluateContextRepl); - // Check required keys; + // Check required keys. EXPECT_THAT_EXPECTED(parse<EvaluateArguments>(R"({})"), FailedWithMessage("missing value at (root).expression")); } @@ -118,3 +118,67 @@ TEST(ProtocolRequestsTest, EvaluateResponseBody) { ASSERT_THAT_EXPECTED(expected_opt, llvm::Succeeded()); EXPECT_EQ(PrettyPrint(*expected_opt), PrettyPrint(body)); } + +TEST(ProtocolRequestsTest, InitializeRequestArguments) { + llvm::Expected<InitializeRequestArguments> expected = + parse<InitializeRequestArguments>(R"({"adapterID": "myid"})"); + ASSERT_THAT_EXPECTED(expected, llvm::Succeeded()); + EXPECT_EQ(expected->adapterID, "myid"); + + // Check optional keys. + expected = parse<InitializeRequestArguments>(R"({ + "adapterID": "myid", + "clientID": "myclientid", + "clientName": "lldb-dap-unit-tests", + "locale": "en-US", + "linesStartAt1": true, + "columnsStartAt1": true, + "pathFormat": "uri", + "supportsVariableType": true, + "supportsVariablePaging": true, + "supportsRunInTerminalRequest": true, + "supportsMemoryReferences": true, + "supportsProgressReporting": true, + "supportsInvalidatedEvent": true, + "supportsMemoryEvent": true, + "supportsArgsCanBeInterpretedByShell": true, + "supportsStartDebuggingRequest": true, + "supportsANSIStyling": true + })"); + ASSERT_THAT_EXPECTED(expected, llvm::Succeeded()); + EXPECT_EQ(expected->adapterID, "myid"); + EXPECT_EQ(expected->clientID, "myclientid"); + EXPECT_EQ(expected->clientName, "lldb-dap-unit-tests"); + EXPECT_EQ(expected->locale, "en-US"); + EXPECT_EQ(expected->linesStartAt1, true); + EXPECT_EQ(expected->columnsStartAt1, true); + EXPECT_EQ(expected->pathFormat, ePathFormatURI); + EXPECT_EQ(expected->supportedFeatures.contains(eClientFeatureVariableType), + true); + EXPECT_EQ( + expected->supportedFeatures.contains(eClientFeatureRunInTerminalRequest), + true); + EXPECT_EQ( + expected->supportedFeatures.contains(eClientFeatureMemoryReferences), + true); + EXPECT_EQ( + expected->supportedFeatures.contains(eClientFeatureProgressReporting), + true); + EXPECT_EQ( + expected->supportedFeatures.contains(eClientFeatureInvalidatedEvent), + true); + EXPECT_EQ(expected->supportedFeatures.contains(eClientFeatureMemoryEvent), + true); + EXPECT_EQ(expected->supportedFeatures.contains( + eClientFeatureArgsCanBeInterpretedByShell), + true); + EXPECT_EQ( + expected->supportedFeatures.contains(eClientFeatureStartDebuggingRequest), + true); + EXPECT_EQ(expected->supportedFeatures.contains(eClientFeatureANSIStyling), + true); + + // Check required keys. + EXPECT_THAT_EXPECTED(parse<InitializeRequestArguments>(R"({})"), + FailedWithMessage("missing value at (root).adapterID")); +} `````````` </details> https://github.com/llvm/llvm-project/pull/170350 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
