https://github.com/ashgti updated 
https://github.com/llvm/llvm-project/pull/135684

>From cab7671780bde4d4e2e1370000f10ced6b5fe504 Mon Sep 17 00:00:00 2001
From: John Harrison <harj...@google.com>
Date: Mon, 14 Apr 2025 13:22:31 -0700
Subject: [PATCH 1/2] [lldb-dap] Imporve error reporting if a command's
 arguments fail to parse correctly.

Previously the error only contained the failed to parse JSON message, which has 
no additional context.

This improves the error messages and improves the consistency of handling 
properties in protocol structures. Updating the fields to use 
'ObjectMapper.map' instead of 'ObjectMapper.mapOptional' caught that adapterID 
was misspelled as well.
---
 lldb/tools/lldb-dap/Handler/RequestHandler.h  |  1 +
 lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp |  2 +-
 .../lldb-dap/Protocol/ProtocolRequests.cpp    | 42 ++++++++++++-------
 .../lldb-dap/Protocol/ProtocolRequests.h      |  4 +-
 .../tools/lldb-dap/Protocol/ProtocolTypes.cpp |  6 +--
 5 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h 
b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index 50795f8252de3..488628b224f53 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -124,6 +124,7 @@ class RequestHandler : public BaseRequestHandler {
     if (request.arguments && !fromJSON(request.arguments, arguments, root)) {
       std::string parse_failure;
       llvm::raw_string_ostream OS(parse_failure);
+      OS << "invalid arguments for request '" << request.command << "': ";
       root.printErrorContext(request.arguments, OS);
 
       protocol::ErrorMessage error_message;
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp 
b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
index af63cc803e545..bfd68448fb483 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
@@ -178,7 +178,7 @@ bool fromJSON(json::Value const &Params, Response &R, 
json::Path P) {
     return false;
   }
 
-  return O.map("success", R.success) && O.mapOptional("message", R.message) &&
+  return O.map("success", R.success) && O.map("message", R.message) &&
          mapRaw(Params, "body", R.body, P);
 }
 
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp 
b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index 7163399899f7e..3523f8ac87ec9 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -20,16 +20,16 @@ namespace lldb_dap::protocol {
 bool fromJSON(const llvm::json::Value &Params, CancelArguments &CA,
               llvm::json::Path P) {
   llvm::json::ObjectMapper O(Params, P);
-  return O && O.mapOptional("requestId", CA.requestId) &&
-         O.mapOptional("progressId", CA.progressId);
+  return O && O.map("requestId", CA.requestId) &&
+         O.map("progressId", CA.progressId);
 }
 
 bool fromJSON(const json::Value &Params, DisconnectArguments &DA,
               json::Path P) {
   json::ObjectMapper O(Params, P);
-  return O && O.mapOptional("restart", DA.restart) &&
-         O.mapOptional("terminateDebuggee", DA.terminateDebuggee) &&
-         O.mapOptional("suspendDebuggee", DA.suspendDebuggee);
+  return O && O.map("restart", DA.restart) &&
+         O.map("terminateDebuggee", DA.terminateDebuggee) &&
+         O.map("suspendDebuggee", DA.suspendDebuggee);
 }
 
 bool fromJSON(const llvm::json::Value &Params, PathFormat &PF,
@@ -75,23 +75,33 @@ bool fromJSON(const llvm::json::Value &Params, 
InitializeRequestArguments &IRA,
 
   const json::Object *O = Params.getAsObject();
 
-  for (auto &kv : ClientFeatureByKey)
-    if (std::optional<bool> v = O->getBoolean(kv.first()); v && *v)
+  for (auto &kv : ClientFeatureByKey) {
+    const json::Value *value_ref = O->get(kv.first());
+    if (!value_ref)
+      continue;
+
+    const std::optional<bool> value = value_ref->getAsBoolean();
+    if (!value) {
+      P.field(kv.first()).report("expected bool");
+      return false;
+    }
+
+    if (*value)
       IRA.supportedFeatures.insert(kv.second);
+  }
 
-  return OM.mapOptional("adatperID", IRA.adatperID) &&
-         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.mapOptional("$__lldb_sourceInitFile", IRA.lldbExtSourceInitFile);
+  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.map("pathFormat", IRA.pathFormat) &&
+         OM.map("$__lldb_sourceInitFile", IRA.lldbExtSourceInitFile);
 }
 
 bool fromJSON(const json::Value &Params, SourceArguments &SA, json::Path P) {
   json::ObjectMapper O(Params, P);
-  return O && O.mapOptional("source", SA.source) &&
+  return O && O.map("source", SA.source) &&
          O.map("sourceReference", SA.sourceReference);
 }
 
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h 
b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index 22d400fd494a5..6623dfa0db05c 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -100,7 +100,7 @@ enum PathFormat : unsigned { ePatFormatPath, ePathFormatURI 
};
 /// Arguments for `initialize` request.
 struct InitializeRequestArguments {
   /// The ID of the debug adapter.
-  std::string adatperID;
+  std::string adapterID;
 
   /// The ID of the client using this adapter.
   std::optional<std::string> clientID;
@@ -113,7 +113,7 @@ struct InitializeRequestArguments {
 
   /// Determines in what format paths are specified. The default is `path`,
   /// which is the native format.
-  std::optional<PathFormat> pathFormat = ePatFormatPath;
+  PathFormat pathFormat = ePatFormatPath;
 
   /// If true all line numbers are 1-based (default).
   std::optional<bool> linesStartAt1;
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp 
b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
index f4f0bf8dcea84..4d1e90215bbb4 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
@@ -38,9 +38,9 @@ bool fromJSON(const json::Value &Params, PresentationHint 
&PH, json::Path P) {
 
 bool fromJSON(const json::Value &Params, Source &S, json::Path P) {
   json::ObjectMapper O(Params, P);
-  return O && O.mapOptional("name", S.name) && O.mapOptional("path", S.path) &&
-         O.mapOptional("presentationHint", S.presentationHint) &&
-         O.mapOptional("sourceReference", S.sourceReference);
+  return O && O.map("name", S.name) && O.map("path", S.path) &&
+         O.map("presentationHint", S.presentationHint) &&
+         O.map("sourceReference", S.sourceReference);
 }
 
 json::Value toJSON(const ExceptionBreakpointsFilter &EBF) {

>From 2cfd5b94df6ee7a947f84c38d53563f9a405c720 Mon Sep 17 00:00:00 2001
From: John Harrison <harj...@google.com>
Date: Mon, 14 Apr 2025 14:42:55 -0700
Subject: [PATCH 2/2] Improving the error formatting.

---
 lldb/tools/lldb-dap/Handler/RequestHandler.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h 
b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index 488628b224f53..7e56c258ad78a 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -120,12 +120,13 @@ class RequestHandler : public BaseRequestHandler {
     }
 
     Args arguments;
-    llvm::json::Path::Root root;
-    if (request.arguments && !fromJSON(request.arguments, arguments, root)) {
+    llvm::json::Path::Root root("arguments");
+    if (request.arguments && !fromJSON(*request.arguments, arguments, root)) {
       std::string parse_failure;
       llvm::raw_string_ostream OS(parse_failure);
-      OS << "invalid arguments for request '" << request.command << "': ";
-      root.printErrorContext(request.arguments, OS);
+      OS << "invalid arguments for request '" << request.command
+         << "': " << llvm::toString(root.getError()) << "\n";
+      root.printErrorContext(*request.arguments, OS);
 
       protocol::ErrorMessage error_message;
       error_message.format = parse_failure;

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to