Skip to content

Commit 995eb4c

Browse files
authored
[lldb-dap] Add readOnly attribute for variables (#151884)
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.
1 parent 6a0416a commit 995eb4c

File tree

2 files changed

+34
-9
lines changed

2 files changed

+34
-9
lines changed

lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,11 @@ def verify_values(self, verify_dict, actual, varref_dict=None, expression=None):
6565
self.assertNotIn(
6666
key, actual, 'key "%s" is not expected in %s' % (key, actual)
6767
)
68+
isReadOnly = verify_dict.get("readOnly", False)
69+
attributes = actual.get("presentationHint", {}).get("attributes", [])
70+
self.assertEqual(
71+
isReadOnly, "readOnly" in attributes, "%s %s" % (verify_dict, actual)
72+
)
6873
hasVariablesReference = "variablesReference" in actual
6974
varRef = None
7075
if hasVariablesReference:
@@ -179,8 +184,9 @@ def do_test_scopes_variables_setVariable_evaluate(
179184
"children": {
180185
"x": {"equals": {"type": "int", "value": "11"}},
181186
"y": {"equals": {"type": "int", "value": "22"}},
182-
"buffer": {"children": buffer_children},
187+
"buffer": {"children": buffer_children, "readOnly": True},
183188
},
189+
"readOnly": True,
184190
},
185191
"x": {"equals": {"type": "int"}},
186192
}
@@ -444,8 +450,10 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo
444450
"buffer": {
445451
"children": buffer_children,
446452
"equals": {"indexedVariables": 16},
453+
"readOnly": True,
447454
},
448455
},
456+
"readOnly": True,
449457
},
450458
"x": {
451459
"equals": {"type": "int"},
@@ -528,7 +536,7 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo
528536
"children": {
529537
"x": {"equals": {"type": "int", "value": "11"}},
530538
"y": {"equals": {"type": "int", "value": "22"}},
531-
"buffer": {"children": buffer_children},
539+
"buffer": {"children": buffer_children, "readOnly": True},
532540
},
533541
}
534542

@@ -622,11 +630,17 @@ def do_test_indexedVariables(self, enableSyntheticChildDebugging: bool):
622630
# "[raw]" child.
623631
raw_child_count = 1 if enableSyntheticChildDebugging else 0
624632
verify_locals = {
625-
"small_array": {"equals": {"indexedVariables": 5}},
626-
"large_array": {"equals": {"indexedVariables": 200}},
627-
"small_vector": {"equals": {"indexedVariables": 5 + raw_child_count}},
628-
"large_vector": {"equals": {"indexedVariables": 200 + raw_child_count}},
629-
"pt": {"missing": ["indexedVariables"]},
633+
"small_array": {"equals": {"indexedVariables": 5}, "readOnly": True},
634+
"large_array": {"equals": {"indexedVariables": 200}, "readOnly": True},
635+
"small_vector": {
636+
"equals": {"indexedVariables": 5 + raw_child_count},
637+
"readOnly": True,
638+
},
639+
"large_vector": {
640+
"equals": {"indexedVariables": 200 + raw_child_count},
641+
"readOnly": True,
642+
},
643+
"pt": {"missing": ["indexedVariables"], "readOnly": True},
630644
}
631645
self.verify_variables(verify_locals, locals)
632646

@@ -640,7 +654,10 @@ def do_test_indexedVariables(self, enableSyntheticChildDebugging: bool):
640654
"[4]": {"equals": {"type": "int", "value": "0"}},
641655
}
642656
if enableSyntheticChildDebugging:
643-
verify_children["[raw]"] = ({"contains": {"type": ["vector"]}},)
657+
verify_children["[raw]"] = {
658+
"contains": {"type": ["vector"]},
659+
"readOnly": True,
660+
}
644661

645662
children = self.dap_server.request_variables(locals[2]["variablesReference"])[
646663
"body"
@@ -660,7 +677,7 @@ def test_return_variables(self):
660677
return_name: {"equals": {"type": "int", "value": "300"}},
661678
"argc": {},
662679
"argv": {},
663-
"pt": {},
680+
"pt": {"readOnly": True},
664681
"x": {},
665682
"return_result": {"equals": {"type": "int"}},
666683
}

lldb/tools/lldb-dap/ProtocolUtils.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,14 @@ Variable CreateVariable(lldb::SBValue v, int64_t var_ref, bool format_hex,
301301
if (lldb::addr_t addr = v.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS)
302302
var.memoryReference = addr;
303303

304+
bool is_readonly = v.GetType().IsAggregateType() ||
305+
v.GetValueType() == lldb::eValueTypeRegisterSet;
306+
if (is_readonly) {
307+
if (!var.presentationHint)
308+
var.presentationHint = {VariablePresentationHint()};
309+
var.presentationHint->attributes.push_back("readOnly");
310+
}
311+
304312
return var;
305313
}
306314

0 commit comments

Comments
 (0)