llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Druzhkov Sergei (DrSergei) <details> <summary>Changes</summary> This patch adds `readOnly` [attribute](https://microsoft.github.io/debug-adapter-protocol/specification#Types_VariablePresentationHint) for variables. When this attribute is returned for a variable, VS Code prevents editing (and grays out the `Set Value` button). Without this, users might be confused if the UI allows edits but the debug adapter often returns an error. I checked `SetValueFromCString` function implementation and found that it doesn't support aggregate data types, so I added simple check for them. Also, I found that I can update value of registers sets (but without any effects). So I also added checks for those as well. --- Full diff: https://github.com/llvm/llvm-project/pull/151884.diff 2 Files Affected: - (modified) lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py (+26-9) - (modified) lldb/tools/lldb-dap/ProtocolUtils.cpp (+8) ``````````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 a3a4bdaaf40a6..a55f080c3b71b 100644 --- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py +++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py @@ -65,6 +65,11 @@ def verify_values(self, verify_dict, actual, varref_dict=None, expression=None): self.assertNotIn( key, actual, 'key "%s" is not expected in %s' % (key, actual) ) + isReadOnly = verify_dict.get("readOnly", False) + attributes = actual.get("presentationHint", {}).get("attributes", []) + self.assertEqual( + isReadOnly, "readOnly" in attributes, "%s %s" % (verify_dict, actual) + ) hasVariablesReference = "variablesReference" in actual varRef = None if hasVariablesReference: @@ -179,8 +184,9 @@ def do_test_scopes_variables_setVariable_evaluate( "children": { "x": {"equals": {"type": "int", "value": "11"}}, "y": {"equals": {"type": "int", "value": "22"}}, - "buffer": {"children": buffer_children}, + "buffer": {"children": buffer_children, "readOnly": True}, }, + "readOnly": True, }, "x": {"equals": {"type": "int"}}, } @@ -456,8 +462,10 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo "buffer": { "children": buffer_children, "equals": {"indexedVariables": 16}, + "readOnly": True, }, }, + "readOnly": True, }, "x": { "equals": {"type": "int"}, @@ -540,7 +548,7 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo "children": { "x": {"equals": {"type": "int", "value": "11"}}, "y": {"equals": {"type": "int", "value": "22"}}, - "buffer": {"children": buffer_children}, + "buffer": {"children": buffer_children, "readOnly": True}, }, } @@ -634,11 +642,17 @@ def do_test_indexedVariables(self, enableSyntheticChildDebugging: bool): # "[raw]" child. raw_child_count = 1 if enableSyntheticChildDebugging else 0 verify_locals = { - "small_array": {"equals": {"indexedVariables": 5}}, - "large_array": {"equals": {"indexedVariables": 200}}, - "small_vector": {"equals": {"indexedVariables": 5 + raw_child_count}}, - "large_vector": {"equals": {"indexedVariables": 200 + raw_child_count}}, - "pt": {"missing": ["indexedVariables"]}, + "small_array": {"equals": {"indexedVariables": 5}, "readOnly": True}, + "large_array": {"equals": {"indexedVariables": 200}, "readOnly": True}, + "small_vector": { + "equals": {"indexedVariables": 5 + raw_child_count}, + "readOnly": True, + }, + "large_vector": { + "equals": {"indexedVariables": 200 + raw_child_count}, + "readOnly": True, + }, + "pt": {"missing": ["indexedVariables"], "readOnly": True}, } self.verify_variables(verify_locals, locals) @@ -652,7 +666,10 @@ def do_test_indexedVariables(self, enableSyntheticChildDebugging: bool): "[4]": {"equals": {"type": "int", "value": "0"}}, } if enableSyntheticChildDebugging: - verify_children["[raw]"] = ({"contains": {"type": ["vector"]}},) + verify_children["[raw]"] = { + "contains": {"type": ["vector"]}, + "readOnly": True, + } children = self.dap_server.request_variables(locals[2]["variablesReference"])[ "body" @@ -672,7 +689,7 @@ def test_return_variables(self): return_name: {"equals": {"type": "int", "value": "300"}}, "argc": {}, "argv": {}, - "pt": {}, + "pt": {"readOnly": True}, "x": {}, "return_result": {"equals": {"type": "int"}}, } diff --git a/lldb/tools/lldb-dap/ProtocolUtils.cpp b/lldb/tools/lldb-dap/ProtocolUtils.cpp index 775c82fbb7716..868c67ca72986 100644 --- a/lldb/tools/lldb-dap/ProtocolUtils.cpp +++ b/lldb/tools/lldb-dap/ProtocolUtils.cpp @@ -301,6 +301,14 @@ Variable CreateVariable(lldb::SBValue v, int64_t var_ref, bool format_hex, if (lldb::addr_t addr = v.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS) var.memoryReference = addr; + bool is_readonly = v.GetType().IsAggregateType() || + v.GetValueType() == lldb::eValueTypeRegisterSet; + if (is_readonly) { + if (!var.presentationHint) + var.presentationHint = {VariablePresentationHint()}; + var.presentationHint->attributes.push_back("readOnly"); + } + return var; } `````````` </details> https://github.com/llvm/llvm-project/pull/151884 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
