Fix command escape bug in lldb-dap

[jeffreytan81]

#69238 caused breakage in VSCode debug console usage -- the user's input is always treated as commands instead of expressions (the same behavior as if empty command escape prefix is specified).

The bug is in one overload of GetString() which did not respect the default value of "`". But more important, I am puzzled to find out why the regression is not caught by lldb test (testdap_evaluate). Turns out #69238 specifies commandEscapePrefix default value in test framework to be "`" while VSCode will default not specify any commandEscapePrefix at all. Changing it to None will fail testdap_evaluate. We should align the default behavior between DAP client and testcase.

This patches fixes the bug in GetString() and changed the default value of commandEscapePrefix in testcases to be None (be consistent with IDE).

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (jeffreytan81)

Changes

#69238 caused breakage in VSCode debug console usage -- the user's input is always treated as commands instead of expressions (the same behavior as if empty command escape prefix is specified).

The bug is in one overload of GetString() which did not respect the default value of "`". But more important, I am puzzled to find out why the regression is not caught by lldb test (testdap_evaluate). Turns out #69238 specifies commandEscapePrefix default value in test framework to be "`" while VSCode will default not specify any commandEscapePrefix at all. Changing it to None will fail testdap_evaluate. We should align the default behavior between DAP client and testcase.

This patches fixes the bug in GetString() and changed the default value of commandEscapePrefix in testcases to be None (be consistent with IDE).

---

Full diff: https://github.com/llvm/llvm-project/pull/72902.diff

3 Files Affected:

• (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+1-1)
• (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+2-2)
• (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+1-1)

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 518e3b9cf5bab33..bb863bb87191763 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -731,7 +731,7 @@ def request_launch(
         postRunCommands=None,
         enableAutoVariableSummaries=False,
         enableSyntheticChildDebugging=False,
-        commandEscapePrefix="`",
+        commandEscapePrefix=None,
         customFrameFormat=None,
         customThreadFormat=None,
     ):
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index 0cf9d4fde49488f..4ccd6014e54be6a 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -354,7 +354,7 @@ def launch(
         postRunCommands=None,
         enableAutoVariableSummaries=False,
         enableSyntheticChildDebugging=False,
-        commandEscapePrefix="`",
+        commandEscapePrefix=None,
         customFrameFormat=None,
         customThreadFormat=None,
     ):
@@ -434,7 +434,7 @@ def build_and_launch(
         lldbDAPEnv=None,
         enableAutoVariableSummaries=False,
         enableSyntheticChildDebugging=False,
-        commandEscapePrefix="`",
+        commandEscapePrefix=None,
         customFrameFormat=None,
         customThreadFormat=None,
     ):
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 50ade02801529c3..03a43f9da87f241 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -57,7 +57,7 @@ llvm::StringRef GetString(const llvm::json::Object *obj, llvm::StringRef key,
                           llvm::StringRef defaultValue) {
   if (obj == nullptr)
     return defaultValue;
-  return GetString(*obj, key);
+  return GetString(*obj, key, defaultValue);
 }
 
 // Gets an unsigned integer from a JSON object using the key, or returns the

[walter-erquinigo]

owch!

[bulbazord]

LGTM. It may be a good idea to move these APIs over to use std::optional instead of taking a default value so we can use 'value_or' instead of having to write logic to handle the default value through a parameter.

[jeffreytan81]

LGTM. It may be a good idea to move these APIs over to use std::optional instead of taking a default value so we can use 'value_or' instead of having to write logic to handle the default value through a parameter.

My goal is fixing the regression in this diff. I will let @walter-erquinigo to comment/follow-up on this suggestion.