Skip to content

[lldb] Omit --show-globals in help target var #85855

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 12 additions & 14 deletions lldb/source/Interpreter/OptionGroupVariable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ static constexpr OptionDefinition g_variable_options[] = {
"Specify a summary string to use to format the variable output."},
};

static constexpr auto g_num_frame_options = 4;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this number computable? Or does it need to be hand-maintained?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly it seems we need to hand-maintain it. The basic thing we need to maintain is "here is an array of options for target var, and here is an array of options for frame var", and one array is a suffix of the other. And they have to be arrays, because of the indexing that is done on them.
Given all of that, I don't think there is a clean way to avoid having to maintain this constant, but the hope is that with the new approach it becomes harder to make the mistake that introduced this bug.

static const auto g_variable_options_noframe =
llvm::ArrayRef<OptionDefinition>(g_variable_options)
.drop_front(g_num_frame_options);

static Status ValidateNamedSummary(const char *str, void *) {
if (!str || !str[0])
return Status("must specify a valid named summary");
Expand Down Expand Up @@ -77,9 +82,9 @@ OptionGroupVariable::SetOptionValue(uint32_t option_idx,
llvm::StringRef option_arg,
ExecutionContext *execution_context) {
Status error;
if (!include_frame_options)
option_idx += 3;
const int short_option = g_variable_options[option_idx].short_option;
llvm::ArrayRef<OptionDefinition> variable_options =
include_frame_options ? g_variable_options : g_variable_options_noframe;
const int short_option = variable_options[option_idx].short_option;
switch (short_option) {
case 'r':
use_regex = true;
Expand Down Expand Up @@ -128,16 +133,9 @@ void OptionGroupVariable::OptionParsingStarting(
summary_string.Clear();
}

#define NUM_FRAME_OPTS 3

llvm::ArrayRef<OptionDefinition> OptionGroupVariable::GetDefinitions() {
auto result = llvm::ArrayRef(g_variable_options);
// Show the "--no-args", "--no-locals" and "--show-globals" options if we are
// showing frame specific options
if (include_frame_options)
return result;

// Skip the "--no-args", "--no-locals" and "--show-globals" options if we are
// not showing frame specific options (globals only)
return result.drop_front(NUM_FRAME_OPTS);
// Show the "--no-args", "--no-recognized-args", "--no-locals" and
// "--show-globals" options if we are showing frame specific options
return include_frame_options ? g_variable_options
: g_variable_options_noframe;
}
10 changes: 10 additions & 0 deletions lldb/test/API/functionalities/target_var/TestTargetVar.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ class targetCommandTestCase(TestBase):
def testTargetVarExpr(self):
self.build()
lldbutil.run_to_name_breakpoint(self, "main")
self.expect(
"help target variable",
substrs=[
"--no-args",
"--no-recognized-args",
"--no-locals",
"--show-globals",
],
matching=False,
)
self.expect("target variable i", substrs=["i", "42"])
self.expect(
"target variable var", patterns=["\(incomplete \*\) var = 0[xX](0)*dead"]
Expand Down