[Lldb-commits] [lldb] b8d38bb - Fix dap variable value format issue (#90799)

via lldb-commits lldb-commits at lists.llvm.org
Fri May 3 13:36:27 PDT 2024


Author: jeffreytan81
Date: 2024-05-03T13:36:23-07:00
New Revision: b8d38bb56d59bee39872fee348a07f79c12f51ae

URL: https://github.com/llvm/llvm-project/commit/b8d38bb56d59bee39872fee348a07f79c12f51ae
DIFF: https://github.com/llvm/llvm-project/commit/b8d38bb56d59bee39872fee348a07f79c12f51ae.diff

LOG: Fix dap variable value format issue (#90799)

While adding a UI feature in VSCode to toggle hex/dec in variables view
window. I noticed that it does not work after second toggle. Then I
noticed that there is a bug that we only explicitly set hex format not
reset back to default during further toggle. The new test demonstrates
the bug.

This PR resets the format back to default if not using hex. One
complexity is that, we explicitly set registers value format to
AddressInfo, which shouldn't be overridden by default or hex settings.

---------

Co-authored-by: jeffreytan81 <jeffreytan at fb.com>

Added: 
    

Modified: 
    lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
    lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
    lldb/tools/lldb-dap/JSONUtils.cpp

Removed: 
    


################################################################################
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 5838281bcb1a10f..e2126d67a5fe778 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
@@ -448,7 +448,7 @@ def get_completions(self, text, frameId=None):
         response = self.request_completions(text, frameId)
         return response["body"]["targets"]
 
-    def get_scope_variables(self, scope_name, frameIndex=0, threadId=None):
+    def get_scope_variables(self, scope_name, frameIndex=0, threadId=None, is_hex=None):
         stackFrame = self.get_stackFrame(frameIndex=frameIndex, threadId=threadId)
         if stackFrame is None:
             return []
@@ -462,7 +462,7 @@ def get_scope_variables(self, scope_name, frameIndex=0, threadId=None):
         for scope in frame_scopes:
             if scope["name"] == scope_name:
                 varRef = scope["variablesReference"]
-                variables_response = self.request_variables(varRef)
+                variables_response = self.request_variables(varRef, is_hex=is_hex)
                 if variables_response:
                     if "body" in variables_response:
                         body = variables_response["body"]
@@ -476,9 +476,9 @@ def get_global_variables(self, frameIndex=0, threadId=None):
             "Globals", frameIndex=frameIndex, threadId=threadId
         )
 
-    def get_local_variables(self, frameIndex=0, threadId=None):
+    def get_local_variables(self, frameIndex=0, threadId=None, is_hex=None):
         return self.get_scope_variables(
-            "Locals", frameIndex=frameIndex, threadId=threadId
+            "Locals", frameIndex=frameIndex, threadId=threadId, is_hex=is_hex
         )
 
     def get_registers(self, frameIndex=0, threadId=None):
@@ -486,28 +486,32 @@ def get_registers(self, frameIndex=0, threadId=None):
             "Registers", frameIndex=frameIndex, threadId=threadId
         )
 
-    def get_local_variable(self, name, frameIndex=0, threadId=None):
-        locals = self.get_local_variables(frameIndex=frameIndex, threadId=threadId)
+    def get_local_variable(self, name, frameIndex=0, threadId=None, is_hex=None):
+        locals = self.get_local_variables(
+            frameIndex=frameIndex, threadId=threadId, is_hex=is_hex
+        )
         for local in locals:
             if "name" in local and local["name"] == name:
                 return local
         return None
 
-    def get_local_variable_value(self, name, frameIndex=0, threadId=None):
+    def get_local_variable_value(self, name, frameIndex=0, threadId=None, is_hex=None):
         variable = self.get_local_variable(
-            name, frameIndex=frameIndex, threadId=threadId
+            name, frameIndex=frameIndex, threadId=threadId, is_hex=is_hex
         )
         if variable and "value" in variable:
             return variable["value"]
         return None
 
-    def get_local_variable_child(self, name, child_name, frameIndex=0, threadId=None):
+    def get_local_variable_child(
+        self, name, child_name, frameIndex=0, threadId=None, is_hex=None
+    ):
         local = self.get_local_variable(name, frameIndex, threadId)
         if local["variablesReference"] == 0:
             return None
-        children = self.request_variables(local["variablesReference"])["body"][
-            "variables"
-        ]
+        children = self.request_variables(local["variablesReference"], is_hex=is_hex)[
+            "body"
+        ]["variables"]
         for child in children:
             if child["name"] == child_name:
                 return child
@@ -1035,12 +1039,16 @@ def request_threads(self):
             self.threads = None
         return response
 
-    def request_variables(self, variablesReference, start=None, count=None):
+    def request_variables(
+        self, variablesReference, start=None, count=None, is_hex=None
+    ):
         args_dict = {"variablesReference": variablesReference}
         if start is not None:
             args_dict["start"] = start
         if count is not None:
             args_dict["count"] = count
+        if is_hex is not None:
+            args_dict["format"] = {"hex": is_hex}
         command_dict = {
             "command": "variables",
             "type": "request",

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 d886d0776ce58bc..ab7dfb5216ae190 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -754,3 +754,43 @@ def test_darwin_dwarf_missing_obj_with_symbol_ondemand_enabled(self):
         """
         initCommands = ["settings set symbols.load-on-demand true"]
         self.darwin_dwarf_missing_obj(initCommands)
+
+    @no_debug_info_test
+    @skipIfWindows
+    @skipIfRemote
+    def test_value_format(self):
+        """
+        Test that toggle variables value format between decimal and hexical works.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        source = "main.cpp"
+        breakpoint1_line = line_number(source, "// breakpoint 1")
+        lines = [breakpoint1_line]
+
+        breakpoint_ids = self.set_source_breakpoints(source, lines)
+        self.assertEqual(
+            len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
+        )
+        self.continue_to_breakpoints(breakpoint_ids)
+
+        # Verify locals value format decimal
+        is_hex = False
+        var_pt_x = self.dap_server.get_local_variable_child("pt", "x", is_hex=is_hex)
+        self.assertEquals(var_pt_x["value"], "11")
+        var_pt_y = self.dap_server.get_local_variable_child("pt", "y", is_hex=is_hex)
+        self.assertEquals(var_pt_y["value"], "22")
+
+        # Verify locals value format hexical
+        is_hex = True
+        var_pt_x = self.dap_server.get_local_variable_child("pt", "x", is_hex=is_hex)
+        self.assertEquals(var_pt_x["value"], "0x0000000b")
+        var_pt_y = self.dap_server.get_local_variable_child("pt", "y", is_hex=is_hex)
+        self.assertEquals(var_pt_y["value"], "0x00000016")
+
+        # Toggle and verify locals value format decimal again
+        is_hex = False
+        var_pt_x = self.dap_server.get_local_variable_child("pt", "x", is_hex=is_hex)
+        self.assertEquals(var_pt_x["value"], "11")
+        var_pt_y = self.dap_server.get_local_variable_child("pt", "y", is_hex=is_hex)
+        self.assertEquals(var_pt_y["value"], "22")

diff  --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 788c1bc843db338..bec277332bcf091 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -754,7 +754,6 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame) {
   } else {
     object.try_emplace("line", 0);
     object.try_emplace("column", 0);
-    object.try_emplace("presentationHint", "subtle");
   }
 
   const auto pc = frame.GetPC();
@@ -987,8 +986,14 @@ VariableDescription::VariableDescription(lldb::SBValue v, bool format_hex,
   display_type_name =
       !raw_display_type_name.empty() ? raw_display_type_name : NO_TYPENAME;
 
-  if (format_hex)
-    v.SetFormat(lldb::eFormatHex);
+  // Only format hex/default if there is no existing special format.
+  if (v.GetFormat() == lldb::eFormatDefault ||
+      v.GetFormat() == lldb::eFormatHex) {
+    if (format_hex)
+      v.SetFormat(lldb::eFormatHex);
+    else
+      v.SetFormat(lldb::eFormatDefault);
+  }
 
   llvm::raw_string_ostream os_display_value(display_value);
 


        


More information about the lldb-commits mailing list