maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[lldb-vscode] Distinguish shadowed variables in the scopes request 

VSCode doesn't render multiple variables with the same name in the variables view. It only renders one of them. This is a situation that happens often when there are shadowed variables.
The nodejs debugger solves this by adding a number suffix to the variable, e.g. "x", "x2", "x3" are the different x variables in nested blocks.

In this patch I'm doing something similar, but the suffix is " @ <file_name:line>), e.g. "x @ main.cpp:17", "x @ main.cpp:21". The fallback would be an address if the source and line information is not present, which should be rare.

This fix is only needed for globals and locals. Children of variables don't suffer of this problem.

When there are shadowed variables
{F16182150}

Without shadowed variables
{F16182152}

Modifying these variables through the UI works

Reviewed By: clayborg

Differential Revision: https://reviews.llvm.org/D99989
This commit is contained in:
Walter Erquinigo 2021-04-21 15:06:44 -07:00
parent 3d8f2059b9
commit c9a0754b44
6 changed files with 141 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
View File

@ -85,7 +85,6 @@ class VSCodeTestCaseBase(TestBase):
# the right breakpoint matches and not worry about the actual
# location.
description = body['description']
print("description: %s" % (description))
for breakpoint_id in breakpoint_ids:
match_desc = 'breakpoint %s.' % (breakpoint_id)
if match_desc in description:

61
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
View File

@ -110,6 +110,9 @@ class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase):
'y': {'equals': {'type': 'int', 'value': '22'}},
'buffer': {'children': buffer_children}
}
},
'x': {
'equals': {'type': 'int'}
}
}
verify_globals = {
@ -221,3 +224,61 @@ class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase):
value = response['body']['variables'][0]['value']
self.assertEqual(value, '111',
'verify pt.x got set to 111 (111 != %s)' % (value))
# We check shadowed variables and that a new get_local_variables request
# gets the right data
breakpoint2_line = line_number(source, '// breakpoint 2')
lines = [breakpoint2_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['argc']['equals']['value'] = '123'
verify_locals['pt']['children']['x']['equals']['value'] = '111'
verify_locals['x @ main.cpp:17'] = {'equals': {'type': 'int', 'value': '89'}}
verify_locals['x @ main.cpp:19'] = {'equals': {'type': 'int', 'value': '42'}}
verify_locals['x @ main.cpp:21'] = {'equals': {'type': 'int', 'value': '72'}}
self.verify_variables(verify_locals, self.vscode.get_local_variables())
# Now we verify that we correctly change the name of a variable with and without differentiator suffix
self.assertFalse(self.vscode.request_setVariable(1, "x2", 9)['success'])
self.assertFalse(self.vscode.request_setVariable(1, "x @ main.cpp:0", 9)['success'])
self.assertTrue(self.vscode.request_setVariable(1, "x @ main.cpp:17", 17)['success'])
self.assertTrue(self.vscode.request_setVariable(1, "x @ main.cpp:19", 19)['success'])
self.assertTrue(self.vscode.request_setVariable(1, "x @ main.cpp:21", 21)['success'])
# The following should have no effect
self.assertFalse(self.vscode.request_setVariable(1, "x @ main.cpp:21", "invalid")['success'])
verify_locals['x @ main.cpp:17']['equals']['value'] = '17'
verify_locals['x @ main.cpp:19']['equals']['value'] = '19'
verify_locals['x @ main.cpp:21']['equals']['value'] = '21'
self.verify_variables(verify_locals, self.vscode.get_local_variables())
# The plain x variable shold refer to the innermost x
self.assertTrue(self.vscode.request_setVariable(1, "x", 22)['success'])
verify_locals['x @ main.cpp:21']['equals']['value'] = '22'
self.verify_variables(verify_locals, self.vscode.get_local_variables())
# In breakpoint 3, there should be no shadowed variables
breakpoint3_line = line_number(source, '// breakpoint 3')
lines = [breakpoint3_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)
locals = self.vscode.get_local_variables()
names = [var['name'] for var in locals]
# The first shadowed x shouldn't have a suffix anymore
verify_locals['x'] = {'equals': {'type': 'int', 'value': '17'}}
self.assertNotIn('x @ main.cpp:17', names)
self.assertNotIn('x @ main.cpp:19', names)
self.assertNotIn('x @ main.cpp:21', names)
self.verify_variables(verify_locals, locals)

10
lldb/test/API/tools/lldb-vscode/variables/main.cpp
View File

@ -14,5 +14,13 @@ int main(int argc, char const *argv[]) {
PointType pt = { 11,22, {0}};
for (int i=0; i<BUFFER_SIZE; ++i)
pt.buffer[i] = i;
return s_global - g_global - pt.y; // breakpoint 1
int x = s_global - g_global - pt.y; // breakpoint 1
{
int x = 42;
{
int x = 72;
s_global = x; // breakpoint 2
}
}
return 0; // breakpoint 3
}

28
lldb/tools/lldb-vscode/JSONUtils.cpp
View File

@ -17,6 +17,7 @@
#include "lldb/API/SBBreakpoint.h"
#include "lldb/API/SBBreakpointLocation.h"
#include "lldb/API/SBDeclaration.h"
#include "lldb/API/SBValue.h"
#include "lldb/Host/PosixApi.h"
@ -904,6 +905,25 @@ llvm::json::Value CreateThreadStopped(lldb::SBThread &thread,
return llvm::json::Value(std::move(event));
}
std::string CreateUniqueVariableNameForDisplay(lldb::SBValue v,
bool is_name_duplicated) {
lldb::SBStream name_builder;
const char *name = v.GetName();
name_builder.Print(name ? name : "<null>");
if (is_name_duplicated) {
name_builder.Print(" @ ");
lldb::SBDeclaration declaration = v.GetDeclaration();
std::string file_name(declaration.GetFileSpec().GetFilename());
const uint32_t line = declaration.GetLine();
if (!file_name.empty() && line > 0)
name_builder.Printf("%s:%u", file_name.c_str(), line);
else
name_builder.Print(v.GetLocation());
}
return name_builder.GetData();
}
// "Variable": {
// "type": "object",
// "description": "A Variable is a name/value pair. Optionally a variable
@ -967,10 +987,12 @@ llvm::json::Value CreateThreadStopped(lldb::SBThread &thread,
// "required": [ "name", "value", "variablesReference" ]
// }
llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
int64_t varID, bool format_hex) {
int64_t varID, bool format_hex,
bool is_name_duplicated) {
llvm::json::Object object;
auto name = v.GetName();
EmplaceSafeString(object, "name", name ? name : "<null>");
EmplaceSafeString(object, "name",
CreateUniqueVariableNameForDisplay(v, is_name_duplicated));
if (format_hex)
v.SetFormat(lldb::eFormatHex);
SetValueForKey(v, object, "value");

19
lldb/tools/lldb-vscode/JSONUtils.h
View File

@ -399,6 +399,14 @@ llvm::json::Value CreateThread(lldb::SBThread &thread);
/// definition outlined by Microsoft.
llvm::json::Value CreateThreadStopped(lldb::SBThread &thread, uint32_t stop_id);
/// VSCode can't display two variables with the same name, so we need to
/// distinguish them by using a suffix.
///
/// If the source and line information is present, we use it as the suffix.
/// Otherwise, we fallback to the variable address or register location.
std::string CreateUniqueVariableNameForDisplay(lldb::SBValue v,
bool is_name_duplicated);
/// Create a "Variable" object for a LLDB thread object.
///
/// This function will fill in the following keys in the returned
@ -435,11 +443,20 @@ llvm::json::Value CreateThreadStopped(lldb::SBThread &thread, uint32_t stop_id);
/// It set to true the variable will be formatted as hex in
/// the "value" key value pair for the value of the variable.
///
/// \param[in] is_name_duplicated
/// Whether the same variable name appears multiple times within the same
/// context (e.g. locals). This can happen due to shadowed variables in
/// nested blocks.
///
/// As VSCode doesn't render two of more variables with the same name, we
/// apply a suffix to distinguish duplicated variables.
///
/// \return
/// A "Variable" JSON object with that follows the formal JSON
/// definition outlined by Microsoft.
llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
int64_t varID, bool format_hex);
int64_t varID, bool format_hex,
bool is_name_duplicated = false);
llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit unit);

38
lldb/tools/lldb-vscode/lldb-vscode.cpp
View File

@ -41,6 +41,7 @@
#include <set>
#include <sstream>
#include <thread>
#include <unordered_map>
#include <vector>
#include "llvm/ADT/ArrayRef.h"
@ -2716,7 +2717,9 @@ void request_setVariable(const llvm::json::Object &request) {
// This is a reference to the containing variable/scope
const auto variablesReference =
GetUnsigned(arguments, "variablesReference", 0);
const auto name = GetString(arguments, "name");
llvm::StringRef name = GetString(arguments, "name");
bool is_duplicated_variable_name = name.find(" @") != llvm::StringRef::npos;
const auto value = GetString(arguments, "value");
// Set success to false just in case we don't find the variable by name
response.try_emplace("success", false);
@ -2758,14 +2761,10 @@ void request_setVariable(const llvm::json::Object &request) {
break;
}
// Find the variable by name in the correct scope and hope we don't have
// multiple variables with the same name. We search backwards because
// the list of variables has the top most variables first and variables
// in deeper scopes are last. This means we will catch the deepest
// variable whose name matches which is probably what the user wants.
for (int64_t i = end_idx - 1; i >= start_idx; --i) {
auto curr_variable = g_vsc.variables.GetValueAtIndex(i);
llvm::StringRef variable_name(curr_variable.GetName());
lldb::SBValue curr_variable = g_vsc.variables.GetValueAtIndex(i);
std::string variable_name = CreateUniqueVariableNameForDisplay(
curr_variable, is_duplicated_variable_name);
if (variable_name == name) {
variable = curr_variable;
if (curr_variable.MightHaveChildren())
@ -2774,6 +2773,9 @@ void request_setVariable(const llvm::json::Object &request) {
}
}
} else {
// This is not under the globals or locals scope, so there are no duplicated
// names.
// We have a named item within an actual variable so we need to find it
// withing the container variable by name.
const int64_t var_idx = VARREF_TO_VARIDX(variablesReference);
@ -2810,6 +2812,8 @@ void request_setVariable(const llvm::json::Object &request) {
EmplaceSafeString(body, "message", std::string(error.GetCString()));
}
response["success"] = llvm::json::Value(success);
} else {
response["success"] = llvm::json::Value(false);
}
response.try_emplace("body", std::move(body));
@ -2925,12 +2929,26 @@ void request_variables(const llvm::json::Object &request) {
break;
}
const int64_t end_idx = start_idx + ((count == 0) ? num_children : count);
// We first find out which variable names are duplicated
std::unordered_map<const char *, int> variable_name_counts;
for (auto i = start_idx; i < end_idx; ++i) {
lldb::SBValue variable = g_vsc.variables.GetValueAtIndex(i);
if (!variable.IsValid())
break;
variables.emplace_back(
CreateVariable(variable, VARIDX_TO_VARREF(i), i, hex));
variable_name_counts[variable.GetName()]++;
}
// Now we construct the result with unique display variable names
for (auto i = start_idx; i < end_idx; ++i) {
lldb::SBValue variable = g_vsc.variables.GetValueAtIndex(i);
const char *name = variable.GetName();
if (!variable.IsValid())
break;
variables.emplace_back(CreateVariable(variable, VARIDX_TO_VARREF(i), i,
hex,
variable_name_counts[name] > 1));
}
} else {
// We are expanding a variable that has children, so we will return its