[Lldb-commits] [lldb] [lldb] Add options to "statistics dump" to control what sections are dumped (PR #95075)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Jun 17 13:16:05 PDT 2024
https://github.com/royitaqi updated https://github.com/llvm/llvm-project/pull/95075
>From f57f98f22425d3c869621b43b65da705d50b5934 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 10 Jun 2024 17:04:11 -0700
Subject: [PATCH 01/15] Add --targets and --modules option to statistics dump
---
lldb/include/lldb/Target/Statistics.h | 2 ++
lldb/source/Commands/CommandObjectStats.cpp | 6 ++++++
lldb/source/Commands/Options.td | 16 +++++++++++++---
lldb/source/Target/Statistics.cpp | 21 ++++++++++++++-------
4 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index c04d529290fff..b24520b72a95d 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -133,6 +133,8 @@ struct ConstStringStats {
struct StatisticsOptions {
bool summary_only = false;
bool load_all_debug_info = false;
+ bool include_targets = false;
+ bool include_modules = false;
bool include_transcript = false;
};
diff --git a/lldb/source/Commands/CommandObjectStats.cpp b/lldb/source/Commands/CommandObjectStats.cpp
index 1935b0fdfadfb..1347b1ba25b1d 100644
--- a/lldb/source/Commands/CommandObjectStats.cpp
+++ b/lldb/source/Commands/CommandObjectStats.cpp
@@ -81,6 +81,12 @@ class CommandObjectStatsDump : public CommandObjectParsed {
case 'f':
m_stats_options.load_all_debug_info = true;
break;
+ case 'r':
+ m_stats_options.include_targets = true;
+ break;
+ case 'm':
+ m_stats_options.include_modules = true;
+ break;
case 't':
m_stats_options.include_transcript = true;
break;
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index cee5a81d3796b..45f46334d5c73 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -1416,16 +1416,26 @@ let Command = "trace schema" in {
}
let Command = "statistics dump" in {
- def statistics_dump_all: Option<"all-targets", "a">, Group<1>,
+ def statistics_dump_all: Option<"all-targets", "a">, GroupRange<1, 2>,
Desc<"Include statistics for all targets.">;
def statistics_dump_summary: Option<"summary", "s">, Group<1>,
Desc<"Dump only high-level summary statistics. "
"Exclude targets, modules, breakpoints etc... details.">;
- def statistics_dump_force: Option<"load-all-debug-info", "f">, Group<1>,
+ def statistics_dump_force: Option<"load-all-debug-info", "f">, GroupRange<1, 2>,
Desc<"Dump the total possible debug info statistics. "
"Force loading all the debug information if not yet loaded, and collect "
"statistics with those.">;
- def statistics_dump_transcript: Option<"transcript", "t">, Group<1>,
+ def statistics_dump_targets: Option<"targets", "r">, Group<2>,
+ Desc<"Dump statistics for the targets, including breakpoints, expression "
+ "evaluations, frame variables, etc. "
+ "If both the '--targets' and the '--modules' options are specified, a "
+ "list of module identifiers will be added to the 'targets' section.">;
+ def statistics_dump_modules: Option<"modules", "m">, Group<2>,
+ Desc<"Dump statistics for the modules, including time and size of various "
+ "aspects of the module and debug information, type system, path, etc. "
+ "If both the '--targets' and the '--modules' options are specified, a "
+ "list of module identifiers will be added to the 'targets' section.">;
+ def statistics_dump_transcript: Option<"transcript", "t">, Group<2>,
Desc<"If the setting interpreter.save-transcript is enabled and this "
"option is specified, include a JSON array with all commands the user and/"
"or scripts executed during a debug session.">;
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 2a5300012511a..13c430d73990e 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -226,6 +226,8 @@ llvm::json::Value DebuggerStats::ReportStatistics(
const bool summary_only = options.summary_only;
const bool load_all_debug_info = options.load_all_debug_info;
+ const bool include_targets = options.include_targets;
+ const bool include_modules = options.include_modules;
const bool include_transcript = options.include_transcript;
json::Array json_targets;
@@ -347,13 +349,15 @@ llvm::json::Value DebuggerStats::ReportStatistics(
{"totalSymbolTableStripped", num_stripped_modules},
};
- if (target) {
- json_targets.emplace_back(target->ReportStatistics(options));
- } else {
- for (const auto &target : debugger.GetTargetList().Targets())
+ if (!summary_only || include_targets) {
+ if (target) {
json_targets.emplace_back(target->ReportStatistics(options));
+ } else {
+ for (const auto &target : debugger.GetTargetList().Targets())
+ json_targets.emplace_back(target->ReportStatistics(options));
+ }
+ global_stats.try_emplace("targets", std::move(json_targets));
}
- global_stats.try_emplace("targets", std::move(json_targets));
ConstStringStats const_string_stats;
json::Object json_memory{
@@ -362,11 +366,14 @@ llvm::json::Value DebuggerStats::ReportStatistics(
global_stats.try_emplace("memory", std::move(json_memory));
if (!summary_only) {
json::Value cmd_stats = debugger.GetCommandInterpreter().GetStatistics();
- global_stats.try_emplace("modules", std::move(json_modules));
global_stats.try_emplace("commands", std::move(cmd_stats));
}
- if (include_transcript) {
+ if (!summary_only || include_modules) {
+ global_stats.try_emplace("modules", std::move(json_modules));
+ }
+
+ if (!summary_only || include_transcript) {
// When transcript is available, add it to the to-be-returned statistics.
//
// NOTE:
>From abfa4fd52f059f9500b223d43bfe50829ad376f2 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 10 Jun 2024 17:04:11 -0700
Subject: [PATCH 02/15] Add --targets and --modules option to statistics dump
---
lldb/include/lldb/Target/Statistics.h | 2 +
lldb/source/Commands/CommandObjectStats.cpp | 6 ++
lldb/source/Commands/Options.td | 16 ++-
lldb/source/Target/Statistics.cpp | 33 ++++--
.../commands/statistics/basic/TestStats.py | 102 ++++++++++++++++--
5 files changed, 140 insertions(+), 19 deletions(-)
diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index c04d529290fff..b24520b72a95d 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -133,6 +133,8 @@ struct ConstStringStats {
struct StatisticsOptions {
bool summary_only = false;
bool load_all_debug_info = false;
+ bool include_targets = false;
+ bool include_modules = false;
bool include_transcript = false;
};
diff --git a/lldb/source/Commands/CommandObjectStats.cpp b/lldb/source/Commands/CommandObjectStats.cpp
index 1935b0fdfadfb..1347b1ba25b1d 100644
--- a/lldb/source/Commands/CommandObjectStats.cpp
+++ b/lldb/source/Commands/CommandObjectStats.cpp
@@ -81,6 +81,12 @@ class CommandObjectStatsDump : public CommandObjectParsed {
case 'f':
m_stats_options.load_all_debug_info = true;
break;
+ case 'r':
+ m_stats_options.include_targets = true;
+ break;
+ case 'm':
+ m_stats_options.include_modules = true;
+ break;
case 't':
m_stats_options.include_transcript = true;
break;
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index cee5a81d3796b..45f46334d5c73 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -1416,16 +1416,26 @@ let Command = "trace schema" in {
}
let Command = "statistics dump" in {
- def statistics_dump_all: Option<"all-targets", "a">, Group<1>,
+ def statistics_dump_all: Option<"all-targets", "a">, GroupRange<1, 2>,
Desc<"Include statistics for all targets.">;
def statistics_dump_summary: Option<"summary", "s">, Group<1>,
Desc<"Dump only high-level summary statistics. "
"Exclude targets, modules, breakpoints etc... details.">;
- def statistics_dump_force: Option<"load-all-debug-info", "f">, Group<1>,
+ def statistics_dump_force: Option<"load-all-debug-info", "f">, GroupRange<1, 2>,
Desc<"Dump the total possible debug info statistics. "
"Force loading all the debug information if not yet loaded, and collect "
"statistics with those.">;
- def statistics_dump_transcript: Option<"transcript", "t">, Group<1>,
+ def statistics_dump_targets: Option<"targets", "r">, Group<2>,
+ Desc<"Dump statistics for the targets, including breakpoints, expression "
+ "evaluations, frame variables, etc. "
+ "If both the '--targets' and the '--modules' options are specified, a "
+ "list of module identifiers will be added to the 'targets' section.">;
+ def statistics_dump_modules: Option<"modules", "m">, Group<2>,
+ Desc<"Dump statistics for the modules, including time and size of various "
+ "aspects of the module and debug information, type system, path, etc. "
+ "If both the '--targets' and the '--modules' options are specified, a "
+ "list of module identifiers will be added to the 'targets' section.">;
+ def statistics_dump_transcript: Option<"transcript", "t">, Group<2>,
Desc<"If the setting interpreter.save-transcript is enabled and this "
"option is specified, include a JSON array with all commands the user and/"
"or scripts executed during a debug session.">;
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 2a5300012511a..15c7dfe07ac87 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -108,6 +108,10 @@ TargetStats::ToJSON(Target &target,
json::Object target_metrics_json;
ProcessSP process_sp = target.GetProcessSP();
const bool summary_only = options.summary_only;
+ const bool include_targets = options.include_targets;
+ const bool include_modules = options.include_modules;
+ const bool include_transcript = options.include_transcript;
+ const bool include_default_sections = !summary_only && !include_targets && !include_modules && !include_transcript;
if (!summary_only) {
CollectStats(target);
@@ -117,8 +121,9 @@ TargetStats::ToJSON(Target &target,
target_metrics_json.try_emplace(m_expr_eval.name, m_expr_eval.ToJSON());
target_metrics_json.try_emplace(m_frame_var.name, m_frame_var.ToJSON());
- target_metrics_json.try_emplace("moduleIdentifiers",
- std::move(json_module_uuid_array));
+ if (include_default_sections || include_modules)
+ target_metrics_json.try_emplace("moduleIdentifiers",
+ std::move(json_module_uuid_array));
if (m_launch_or_attach_time && m_first_private_stop_time) {
double elapsed_time =
@@ -226,7 +231,10 @@ llvm::json::Value DebuggerStats::ReportStatistics(
const bool summary_only = options.summary_only;
const bool load_all_debug_info = options.load_all_debug_info;
+ const bool include_targets = options.include_targets;
+ const bool include_modules = options.include_modules;
const bool include_transcript = options.include_transcript;
+ const bool include_default_sections = !summary_only && !include_targets && !include_modules && !include_transcript;
json::Array json_targets;
json::Array json_modules;
@@ -314,7 +322,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
if (module_stat.debug_info_had_incomplete_types)
++num_modules_with_incomplete_types;
- if (!summary_only) {
+ if (include_default_sections || include_modules) {
module_stat.identifier = (intptr_t)module;
module_stat.path = module->GetFileSpec().GetPath();
if (ConstString object_name = module->GetObjectName()) {
@@ -347,13 +355,15 @@ llvm::json::Value DebuggerStats::ReportStatistics(
{"totalSymbolTableStripped", num_stripped_modules},
};
- if (target) {
- json_targets.emplace_back(target->ReportStatistics(options));
- } else {
- for (const auto &target : debugger.GetTargetList().Targets())
+ if (include_default_sections || include_targets) {
+ if (target) {
json_targets.emplace_back(target->ReportStatistics(options));
+ } else {
+ for (const auto &target : debugger.GetTargetList().Targets())
+ json_targets.emplace_back(target->ReportStatistics(options));
+ }
+ global_stats.try_emplace("targets", std::move(json_targets));
}
- global_stats.try_emplace("targets", std::move(json_targets));
ConstStringStats const_string_stats;
json::Object json_memory{
@@ -362,11 +372,14 @@ llvm::json::Value DebuggerStats::ReportStatistics(
global_stats.try_emplace("memory", std::move(json_memory));
if (!summary_only) {
json::Value cmd_stats = debugger.GetCommandInterpreter().GetStatistics();
- global_stats.try_emplace("modules", std::move(json_modules));
global_stats.try_emplace("commands", std::move(cmd_stats));
}
- if (include_transcript) {
+ if (include_default_sections || include_modules) {
+ global_stats.try_emplace("modules", std::move(json_modules));
+ }
+
+ if (include_default_sections || include_transcript) {
// When transcript is available, add it to the to-be-returned statistics.
//
// NOTE:
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index ebe6166eb45e5..2132d2df03b4b 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -657,16 +657,106 @@ def test_transcript_happy_path(self):
# The second "statistics dump" in the transcript should have no output
self.assertNotIn("output", transcript[2])
- def test_transcript_should_not_exist_when_not_asked_for(self):
+ def test_sections_existence(self):
"""
- Test "statistics dump" and the transcript information.
+ Test "statistics dump" and the existence of sections when different
+ options are given.
"""
self.build()
exe = self.getBuildArtifact("a.out")
target = self.createTestTarget(file_path=exe)
self.runCmd("settings set interpreter.save-transcript true")
- self.runCmd("version")
- # Verify the output of a first "statistics dump"
- debug_stats = self.get_stats() # Not with "--transcript"
- self.assertNotIn("transcript", debug_stats)
+ test_cases = [
+ { # statistics dump
+ "options": "",
+ "expect": {
+ "commands": True,
+ "targets": True,
+ "targets.moduleIdentifiers": True,
+ "targets.breakpoints": True,
+ "modules": True,
+ "transcript": True,
+ },
+ },
+ { # statistics dump --summary
+ "options": " --summary",
+ "expect": {
+ "commands": False,
+ "targets": False,
+ "targets.moduleIdentifiers": False,
+ "targets.breakpoints": False,
+ "modules": False,
+ "transcript": False,
+ },
+ },
+ { # statistics dump --targets
+ "options": " --targets",
+ "expect": {
+ "commands": True,
+ "targets": True,
+ "targets.moduleIdentifiers": False,
+ "targets.breakpoints": True,
+ "modules": False,
+ "transcript": False,
+ },
+ },
+ { # statistics dump --modules
+ "options": " --modules",
+ "expect": {
+ "commands": True,
+ "targets": False,
+ "targets.moduleIdentifiers": False,
+ "targets.breakpoints": False,
+ "modules": True,
+ "transcript": False,
+ },
+ },
+ { # statistics dump --targets --modules
+ "options": " --targets --modules",
+ "expect": {
+ "commands": True,
+ "targets": True,
+ "targets.moduleIdentifiers": True,
+ "targets.breakpoints": True,
+ "modules": True,
+ "transcript": False,
+ },
+ },
+ { # statistics dump --transcript
+ "options": " --transcript",
+ "expect": {
+ "commands": True,
+ "targets": False,
+ "targets.moduleIdentifiers": False,
+ "targets.breakpoints": False,
+ "modules": False,
+ "transcript": True,
+ },
+ },
+ ]
+
+ for test_case in test_cases:
+ options = test_case["options"]
+ debug_stats = self.get_stats(options)
+ # The following fields should always exist
+ self.assertIn("totalDebugInfoEnabled", debug_stats, "Global stats should always exist")
+ self.assertIn("memory", debug_stats, "'memory' should always exist")
+ # The following fields should exist/not exist depending on the test case
+ for field_name in test_case["expect"]:
+ idx = field_name.find(".")
+ if idx == -1:
+ # `field` is a top-level field
+ exists = field_name in debug_stats
+ should_exist = test_case["expect"][field_name]
+ should_exist_string = "" if should_exist else "not "
+ self.assertEqual(exists, should_exist, f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'")
+ else:
+ # `field` is a string of "<top-level field>.<second-level field>"
+ top_level_field_name = field_name[0:idx]
+ second_level_field_name = field_name[idx+1:]
+ for top_level_field in debug_stats[top_level_field_name] if top_level_field_name in debug_stats else []:
+ exists = second_level_field_name in top_level_field
+ should_exist = test_case["expect"][field_name]
+ should_exist_string = "" if should_exist else "not "
+ self.assertEqual(exists, should_exist, f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'")
>From 358497da7fb85394aceca4a14ea55e7649e5d4db Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 10 Jun 2024 21:54:40 -0700
Subject: [PATCH 03/15] Fix C++ format
---
lldb/source/Target/Statistics.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 15c7dfe07ac87..bbb556925eb1f 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -111,7 +111,8 @@ TargetStats::ToJSON(Target &target,
const bool include_targets = options.include_targets;
const bool include_modules = options.include_modules;
const bool include_transcript = options.include_transcript;
- const bool include_default_sections = !summary_only && !include_targets && !include_modules && !include_transcript;
+ const bool include_default_sections = !summary_only && !include_targets &&
+ !include_modules && !include_transcript;
if (!summary_only) {
CollectStats(target);
@@ -234,7 +235,8 @@ llvm::json::Value DebuggerStats::ReportStatistics(
const bool include_targets = options.include_targets;
const bool include_modules = options.include_modules;
const bool include_transcript = options.include_transcript;
- const bool include_default_sections = !summary_only && !include_targets && !include_modules && !include_transcript;
+ const bool include_default_sections = !summary_only && !include_targets &&
+ !include_modules && !include_transcript;
json::Array json_targets;
json::Array json_modules;
>From 32e51e0e4f1debfc3338af8a8f226f02ec7187f7 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 10 Jun 2024 21:55:44 -0700
Subject: [PATCH 04/15] Fix python format
---
.../commands/statistics/basic/TestStats.py | 36 +++++++++++++------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index 2132d2df03b4b..db254e36c4b6c 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -668,7 +668,7 @@ def test_sections_existence(self):
self.runCmd("settings set interpreter.save-transcript true")
test_cases = [
- { # statistics dump
+ { # statistics dump
"options": "",
"expect": {
"commands": True,
@@ -679,7 +679,7 @@ def test_sections_existence(self):
"transcript": True,
},
},
- { # statistics dump --summary
+ { # statistics dump --summary
"options": " --summary",
"expect": {
"commands": False,
@@ -690,7 +690,7 @@ def test_sections_existence(self):
"transcript": False,
},
},
- { # statistics dump --targets
+ { # statistics dump --targets
"options": " --targets",
"expect": {
"commands": True,
@@ -701,7 +701,7 @@ def test_sections_existence(self):
"transcript": False,
},
},
- { # statistics dump --modules
+ { # statistics dump --modules
"options": " --modules",
"expect": {
"commands": True,
@@ -712,7 +712,7 @@ def test_sections_existence(self):
"transcript": False,
},
},
- { # statistics dump --targets --modules
+ { # statistics dump --targets --modules
"options": " --targets --modules",
"expect": {
"commands": True,
@@ -723,7 +723,7 @@ def test_sections_existence(self):
"transcript": False,
},
},
- { # statistics dump --transcript
+ { # statistics dump --transcript
"options": " --transcript",
"expect": {
"commands": True,
@@ -740,7 +740,9 @@ def test_sections_existence(self):
options = test_case["options"]
debug_stats = self.get_stats(options)
# The following fields should always exist
- self.assertIn("totalDebugInfoEnabled", debug_stats, "Global stats should always exist")
+ self.assertIn(
+ "totalDebugInfoEnabled", debug_stats, "Global stats should always exist"
+ )
self.assertIn("memory", debug_stats, "'memory' should always exist")
# The following fields should exist/not exist depending on the test case
for field_name in test_case["expect"]:
@@ -750,13 +752,25 @@ def test_sections_existence(self):
exists = field_name in debug_stats
should_exist = test_case["expect"][field_name]
should_exist_string = "" if should_exist else "not "
- self.assertEqual(exists, should_exist, f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'")
+ self.assertEqual(
+ exists,
+ should_exist,
+ f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'",
+ )
else:
# `field` is a string of "<top-level field>.<second-level field>"
top_level_field_name = field_name[0:idx]
- second_level_field_name = field_name[idx+1:]
- for top_level_field in debug_stats[top_level_field_name] if top_level_field_name in debug_stats else []:
+ second_level_field_name = field_name[idx + 1 :]
+ for top_level_field in (
+ debug_stats[top_level_field_name]
+ if top_level_field_name in debug_stats
+ else []
+ ):
exists = second_level_field_name in top_level_field
should_exist = test_case["expect"][field_name]
should_exist_string = "" if should_exist else "not "
- self.assertEqual(exists, should_exist, f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'")
+ self.assertEqual(
+ exists,
+ should_exist,
+ f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'",
+ )
>From a8fc16d5f31fd45258b5b7f950cd080a9d83680a Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 10 Jun 2024 22:10:00 -0700
Subject: [PATCH 05/15] Add options into SBStatisticsOptions
---
lldb/include/lldb/API/SBStatisticsOptions.h | 9 ++++++++
lldb/source/API/SBStatisticsOptions.cpp | 24 +++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/lldb/include/lldb/API/SBStatisticsOptions.h b/lldb/include/lldb/API/SBStatisticsOptions.h
index a0055135e36c2..35ebe3db1c53b 100644
--- a/lldb/include/lldb/API/SBStatisticsOptions.h
+++ b/lldb/include/lldb/API/SBStatisticsOptions.h
@@ -25,6 +25,15 @@ class LLDB_API SBStatisticsOptions {
void SetSummaryOnly(bool b);
bool GetSummaryOnly();
+ void SetIncludeTargets(bool b);
+ bool GetIncludeTargets() const;
+
+ void SetIncludeModules(bool b);
+ bool GetIncludeModules() const;
+
+ void SetIncludeTranscript(bool b);
+ bool GetIncludeTranscript() const;
+
/// If set to true, the debugger will load all debug info that is available
/// and report statistics on the total amount. If this is set to false, then
/// only report statistics on the currently loaded debug information.
diff --git a/lldb/source/API/SBStatisticsOptions.cpp b/lldb/source/API/SBStatisticsOptions.cpp
index 7e826c4c93ebc..7cf8664955ae7 100644
--- a/lldb/source/API/SBStatisticsOptions.cpp
+++ b/lldb/source/API/SBStatisticsOptions.cpp
@@ -44,6 +44,30 @@ void SBStatisticsOptions::SetSummaryOnly(bool b) {
bool SBStatisticsOptions::GetSummaryOnly() { return m_opaque_up->summary_only; }
+void SBStatisticsOptions::SetIncludeTargets(bool b) {
+ m_opaque_up->include_targets = b;
+}
+
+bool SBStatisticsOptions::GetIncludeTargets() const {
+ return m_opaque_up->include_targets;
+}
+
+void SBStatisticsOptions::SetIncludeModules(bool b) {
+ m_opaque_up->include_modules = b;
+}
+
+bool SBStatisticsOptions::GetIncludeModules() const {
+ return m_opaque_up->include_modules;
+}
+
+void SBStatisticsOptions::SetIncludeTranscript(bool b) {
+ m_opaque_up->include_transcript = b;
+}
+
+bool SBStatisticsOptions::GetIncludeTranscript() const {
+ return m_opaque_up->include_transcript;
+}
+
void SBStatisticsOptions::SetReportAllAvailableDebugInfo(bool b) {
m_opaque_up->load_all_debug_info = b;
}
>From fb0108c7e78692bf5f919daf4fe48045e4397357 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Thu, 13 Jun 2024 16:45:00 -0700
Subject: [PATCH 06/15] Reframe the options by composition
---
.../lldb/Interpreter/OptionArgParser.h | 2 +
lldb/include/lldb/Interpreter/Options.h | 2 +-
lldb/include/lldb/Target/Statistics.h | 6 +-
lldb/source/Commands/CommandObjectStats.cpp | 19 ++-
lldb/source/Commands/Options.td | 13 +-
lldb/source/Interpreter/OptionArgParser.cpp | 13 ++
lldb/source/Interpreter/Options.cpp | 1 +
lldb/source/Target/Statistics.cpp | 16 +--
.../commands/statistics/basic/TestStats.py | 113 +++++++++++++-----
9 files changed, 133 insertions(+), 52 deletions(-)
diff --git a/lldb/include/lldb/Interpreter/OptionArgParser.h b/lldb/include/lldb/Interpreter/OptionArgParser.h
index 66b16112fce96..9bae7674ea2f1 100644
--- a/lldb/include/lldb/Interpreter/OptionArgParser.h
+++ b/lldb/include/lldb/Interpreter/OptionArgParser.h
@@ -29,6 +29,8 @@ struct OptionArgParser {
static bool ToBoolean(llvm::StringRef s, bool fail_value, bool *success_ptr);
+ static bool ToBoolean(llvm::StringRef option_name, llvm::StringRef option_arg, bool fail_value, Status &error);
+
static char ToChar(llvm::StringRef s, char fail_value, bool *success_ptr);
static int64_t ToOptionEnum(llvm::StringRef s,
diff --git a/lldb/include/lldb/Interpreter/Options.h b/lldb/include/lldb/Interpreter/Options.h
index 9a6a17c2793fa..a95111262c25f 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -217,7 +217,7 @@ class Options {
void OptionsSetUnion(const OptionSet &set_a, const OptionSet &set_b,
OptionSet &union_set);
-
+
// Subclasses must reset their option values prior to starting a new option
// parse. Each subclass must override this function and revert all option
// settings to default values.
diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index b24520b72a95d..268de2f1d9686 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -133,9 +133,9 @@ struct ConstStringStats {
struct StatisticsOptions {
bool summary_only = false;
bool load_all_debug_info = false;
- bool include_targets = false;
- bool include_modules = false;
- bool include_transcript = false;
+ bool include_targets = true;
+ bool include_modules = true;
+ bool include_transcript = true;
};
/// A class that represents statistics for a since lldb_private::Target.
diff --git a/lldb/source/Commands/CommandObjectStats.cpp b/lldb/source/Commands/CommandObjectStats.cpp
index 1347b1ba25b1d..929a79180070f 100644
--- a/lldb/source/Commands/CommandObjectStats.cpp
+++ b/lldb/source/Commands/CommandObjectStats.cpp
@@ -11,6 +11,7 @@
#include "lldb/Host/OptionParser.h"
#include "lldb/Interpreter/CommandOptionArgumentTable.h"
#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Interpreter/OptionArgParser.h"
#include "lldb/Target/Target.h"
using namespace lldb;
@@ -77,18 +78,30 @@ class CommandObjectStatsDump : public CommandObjectParsed {
break;
case 's':
m_stats_options.summary_only = true;
+ // In the "summary" mode, the following sections should be omitted by
+ // default unless their corresponding options are explicitly given.
+ // If such options were processed before 's', `m_seen_options` should
+ // contain them, and we will skip setting them to `false` here. If such
+ // options are not yet processed, we will set them to `false` here, and
+ // they will be overwritten when the options are processed.
+ if (m_seen_options.find((int)'r') == m_seen_options.end())
+ m_stats_options.include_targets = false;
+ if (m_seen_options.find((int)'m') == m_seen_options.end())
+ m_stats_options.include_modules = false;
+ if (m_seen_options.find((int)'t') == m_seen_options.end())
+ m_stats_options.include_transcript = false;
break;
case 'f':
m_stats_options.load_all_debug_info = true;
break;
case 'r':
- m_stats_options.include_targets = true;
+ m_stats_options.include_targets = OptionArgParser::ToBoolean("--targets", option_arg, true /* doesn't matter */, error);
break;
case 'm':
- m_stats_options.include_modules = true;
+ m_stats_options.include_modules = OptionArgParser::ToBoolean("--modules", option_arg, true /* doesn't matter */, error);
break;
case 't':
- m_stats_options.include_transcript = true;
+ m_stats_options.include_transcript = OptionArgParser::ToBoolean("--transcript", option_arg, true /* doesn't matter */, error);
break;
default:
llvm_unreachable("Unimplemented option");
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 45f46334d5c73..b7e8fa22cdaa4 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -1416,26 +1416,29 @@ let Command = "trace schema" in {
}
let Command = "statistics dump" in {
- def statistics_dump_all: Option<"all-targets", "a">, GroupRange<1, 2>,
+ def statistics_dump_all: Option<"all-targets", "a">, Group<1>,
Desc<"Include statistics for all targets.">;
def statistics_dump_summary: Option<"summary", "s">, Group<1>,
Desc<"Dump only high-level summary statistics. "
"Exclude targets, modules, breakpoints etc... details.">;
- def statistics_dump_force: Option<"load-all-debug-info", "f">, GroupRange<1, 2>,
+ def statistics_dump_force: Option<"load-all-debug-info", "f">, Group<1>,
Desc<"Dump the total possible debug info statistics. "
"Force loading all the debug information if not yet loaded, and collect "
"statistics with those.">;
- def statistics_dump_targets: Option<"targets", "r">, Group<2>,
+ def statistics_dump_targets: Option<"targets", "r">, Group<1>,
+ Arg<"Boolean">,
Desc<"Dump statistics for the targets, including breakpoints, expression "
"evaluations, frame variables, etc. "
"If both the '--targets' and the '--modules' options are specified, a "
"list of module identifiers will be added to the 'targets' section.">;
- def statistics_dump_modules: Option<"modules", "m">, Group<2>,
+ def statistics_dump_modules: Option<"modules", "m">, Group<1>,
+ Arg<"Boolean">,
Desc<"Dump statistics for the modules, including time and size of various "
"aspects of the module and debug information, type system, path, etc. "
"If both the '--targets' and the '--modules' options are specified, a "
"list of module identifiers will be added to the 'targets' section.">;
- def statistics_dump_transcript: Option<"transcript", "t">, Group<2>,
+ def statistics_dump_transcript: Option<"transcript", "t">, Group<1>,
+ Arg<"Boolean">,
Desc<"If the setting interpreter.save-transcript is enabled and this "
"option is specified, include a JSON array with all commands the user and/"
"or scripts executed during a debug session.">;
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp b/lldb/source/Interpreter/OptionArgParser.cpp
index 9a8275128ede9..963828d6bdda1 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -35,6 +35,19 @@ bool OptionArgParser::ToBoolean(llvm::StringRef ref, bool fail_value,
return fail_value;
}
+bool OptionArgParser::ToBoolean(llvm::StringRef option_name, llvm::StringRef option_arg, bool fail_value, Status &error)
+{
+ bool parse_success;
+ const bool arg_value =
+ ToBoolean(option_arg, fail_value, &parse_success);
+ if (!parse_success)
+ error.SetErrorStringWithFormat(
+ "Invalid boolean value for option '%s': '%s'",
+ option_name.str().c_str(),
+ option_arg.empty() ? "<null>" : option_arg.str().c_str());
+ return arg_value;
+}
+
char OptionArgParser::ToChar(llvm::StringRef s, char fail_value,
bool *success_ptr) {
if (success_ptr)
diff --git a/lldb/source/Interpreter/Options.cpp b/lldb/source/Interpreter/Options.cpp
index 4e7d074ace1b8..65d95013fc903 100644
--- a/lldb/source/Interpreter/Options.cpp
+++ b/lldb/source/Interpreter/Options.cpp
@@ -18,6 +18,7 @@
#include "lldb/Interpreter/CommandInterpreter.h"
#include "lldb/Interpreter/CommandObject.h"
#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Interpreter/OptionArgParser.h"
#include "lldb/Target/Target.h"
#include "lldb/Utility/StreamString.h"
#include "llvm/ADT/STLExtras.h"
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index bbb556925eb1f..716f80a7fd6bb 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -108,11 +108,7 @@ TargetStats::ToJSON(Target &target,
json::Object target_metrics_json;
ProcessSP process_sp = target.GetProcessSP();
const bool summary_only = options.summary_only;
- const bool include_targets = options.include_targets;
const bool include_modules = options.include_modules;
- const bool include_transcript = options.include_transcript;
- const bool include_default_sections = !summary_only && !include_targets &&
- !include_modules && !include_transcript;
if (!summary_only) {
CollectStats(target);
@@ -122,7 +118,7 @@ TargetStats::ToJSON(Target &target,
target_metrics_json.try_emplace(m_expr_eval.name, m_expr_eval.ToJSON());
target_metrics_json.try_emplace(m_frame_var.name, m_frame_var.ToJSON());
- if (include_default_sections || include_modules)
+ if (include_modules)
target_metrics_json.try_emplace("moduleIdentifiers",
std::move(json_module_uuid_array));
@@ -235,8 +231,6 @@ llvm::json::Value DebuggerStats::ReportStatistics(
const bool include_targets = options.include_targets;
const bool include_modules = options.include_modules;
const bool include_transcript = options.include_transcript;
- const bool include_default_sections = !summary_only && !include_targets &&
- !include_modules && !include_transcript;
json::Array json_targets;
json::Array json_modules;
@@ -324,7 +318,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
if (module_stat.debug_info_had_incomplete_types)
++num_modules_with_incomplete_types;
- if (include_default_sections || include_modules) {
+ if (include_modules) {
module_stat.identifier = (intptr_t)module;
module_stat.path = module->GetFileSpec().GetPath();
if (ConstString object_name = module->GetObjectName()) {
@@ -357,7 +351,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
{"totalSymbolTableStripped", num_stripped_modules},
};
- if (include_default_sections || include_targets) {
+ if (include_targets) {
if (target) {
json_targets.emplace_back(target->ReportStatistics(options));
} else {
@@ -377,11 +371,11 @@ llvm::json::Value DebuggerStats::ReportStatistics(
global_stats.try_emplace("commands", std::move(cmd_stats));
}
- if (include_default_sections || include_modules) {
+ if (include_modules) {
global_stats.try_emplace("modules", std::move(json_modules));
}
- if (include_default_sections || include_transcript) {
+ if (include_transcript) {
// When transcript is available, add it to the to-be-returned statistics.
//
// NOTE:
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index db254e36c4b6c..f19ec89bfe777 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -1,6 +1,7 @@
import lldb
import json
import os
+import re
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil
@@ -635,7 +636,7 @@ def test_transcript_happy_path(self):
self.runCmd("version")
# Verify the output of a first "statistics dump"
- debug_stats = self.get_stats("--transcript")
+ debug_stats = self.get_stats("--transcript true")
self.assertIn("transcript", debug_stats)
transcript = debug_stats["transcript"]
self.assertEqual(len(transcript), 2)
@@ -645,7 +646,7 @@ def test_transcript_happy_path(self):
self.assertNotIn("output", transcript[1])
# Verify the output of a second "statistics dump"
- debug_stats = self.get_stats("--transcript")
+ debug_stats = self.get_stats("--transcript true")
self.assertIn("transcript", debug_stats)
transcript = debug_stats["transcript"]
self.assertEqual(len(transcript), 3)
@@ -665,92 +666,107 @@ def test_sections_existence(self):
self.build()
exe = self.getBuildArtifact("a.out")
target = self.createTestTarget(file_path=exe)
- self.runCmd("settings set interpreter.save-transcript true")
+ # Create some transcript so that it can be tested.
+ self.runCmd("settings set interpreter.save-transcript true")
+ self.runCmd("version")
+ self.runCmd("b main")
+ # But disable transcript so that it won't change during verification
+ self.runCmd("settings set interpreter.save-transcript false")
+
+ # Expectation
+ should_always_exist_or_not = {
+ "totalDebugInfoEnabled": True,
+ "memory": True,
+ }
test_cases = [
- { # statistics dump
+ { # Everything mode
"options": "",
"expect": {
"commands": True,
"targets": True,
"targets.moduleIdentifiers": True,
"targets.breakpoints": True,
+ "targets.expressionEvaluation": True,
"modules": True,
"transcript": True,
},
},
- { # statistics dump --summary
+ { # Summary mode
"options": " --summary",
"expect": {
"commands": False,
"targets": False,
"targets.moduleIdentifiers": False,
"targets.breakpoints": False,
+ "targets.expressionEvaluation": False,
"modules": False,
"transcript": False,
},
},
- { # statistics dump --targets
- "options": " --targets",
+ { # Summary mode with targets
+ "options": " --summary --targets=true",
"expect": {
- "commands": True,
+ "commands": False,
"targets": True,
"targets.moduleIdentifiers": False,
- "targets.breakpoints": True,
+ "targets.breakpoints": False,
+ "targets.expressionEvaluation": False,
+ "targets.totalSharedLibraryEventHitCount": True,
"modules": False,
"transcript": False,
},
},
- { # statistics dump --modules
- "options": " --modules",
+ { # Summary mode with modules
+ "options": " --summary --modules=true",
"expect": {
- "commands": True,
+ "commands": False,
"targets": False,
"targets.moduleIdentifiers": False,
"targets.breakpoints": False,
+ "targets.expressionEvaluation": True,
"modules": True,
"transcript": False,
},
},
- { # statistics dump --targets --modules
- "options": " --targets --modules",
+ { # Everything mode but without modules and transcript
+ "options": " --modules=false --transcript=false",
"expect": {
"commands": True,
"targets": True,
- "targets.moduleIdentifiers": True,
+ "targets.moduleIdentifiers": False,
"targets.breakpoints": True,
- "modules": True,
+ "targets.expressionEvaluation": True,
+ "modules": False,
"transcript": False,
},
},
- { # statistics dump --transcript
- "options": " --transcript",
+ { # Everything mode but without modules
+ "options": " --modules=false",
"expect": {
"commands": True,
- "targets": False,
+ "targets": True,
"targets.moduleIdentifiers": False,
- "targets.breakpoints": False,
+ "targets.breakpoints": True,
+ "targets.expressionEvaluation": True,
"modules": False,
"transcript": True,
},
},
]
+ # Verification
for test_case in test_cases:
options = test_case["options"]
debug_stats = self.get_stats(options)
- # The following fields should always exist
- self.assertIn(
- "totalDebugInfoEnabled", debug_stats, "Global stats should always exist"
- )
- self.assertIn("memory", debug_stats, "'memory' should always exist")
- # The following fields should exist/not exist depending on the test case
- for field_name in test_case["expect"]:
+ # Verify that each field should exist (or not)
+ expectation = {**should_always_exist_or_not, **test_case["expect"]}
+ for field_name in expectation:
idx = field_name.find(".")
if idx == -1:
# `field` is a top-level field
exists = field_name in debug_stats
- should_exist = test_case["expect"][field_name]
+ should_exist = expectation[field_name]
should_exist_string = "" if should_exist else "not "
self.assertEqual(
exists,
@@ -767,10 +783,49 @@ def test_sections_existence(self):
else []
):
exists = second_level_field_name in top_level_field
- should_exist = test_case["expect"][field_name]
+ should_exist = expectation[field_name]
should_exist_string = "" if should_exist else "not "
self.assertEqual(
exists,
should_exist,
f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'",
)
+
+ def test_order_of_options_do_not_matter(self):
+ """
+ Test "statistics dump" and the order of options.
+ """
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ target = self.createTestTarget(file_path=exe)
+
+ # Create some transcript so that it can be tested.
+ self.runCmd("settings set interpreter.save-transcript true")
+ self.runCmd("version")
+ self.runCmd("b main")
+ # But disable transcript so that it won't change during verification
+ self.runCmd("settings set interpreter.save-transcript false")
+
+ # The order of the following options shouldn't matter
+ test_cases = [
+ (" --summary", " --targets=true"),
+ (" --summary", " --targets=false"),
+ (" --summary", " --modules=true"),
+ (" --summary", " --modules=false"),
+ (" --summary", " --transcript=true"),
+ (" --summary", " --transcript=false"),
+ ]
+
+ # Verification
+ for options in test_cases:
+ debug_stats_0 = self.get_stats(options[0] + options[1])
+ debug_stats_1 = self.get_stats(options[1] + options[0])
+ # Redact all numbers
+ debug_stats_0 = re.sub(r"\d+", "0", json.dumps(debug_stats_0))
+ debug_stats_1 = re.sub(r"\d+", "0", json.dumps(debug_stats_1))
+ # Verify that the two output are the same
+ self.assertEqual(
+ debug_stats_0,
+ debug_stats_1,
+ f"The order of options '{options[0]}' and '{options[1]}' should not matter",
+ )
>From 111ee64a5935dac64d71688280540834213043e6 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Thu, 13 Jun 2024 16:51:28 -0700
Subject: [PATCH 07/15] Fix format
---
lldb/include/lldb/Interpreter/OptionArgParser.h | 3 ++-
lldb/include/lldb/Interpreter/Options.h | 2 +-
lldb/source/Commands/CommandObjectStats.cpp | 9 ++++++---
lldb/source/Interpreter/OptionArgParser.cpp | 8 ++++----
4 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/lldb/include/lldb/Interpreter/OptionArgParser.h b/lldb/include/lldb/Interpreter/OptionArgParser.h
index 9bae7674ea2f1..af20cd258aff3 100644
--- a/lldb/include/lldb/Interpreter/OptionArgParser.h
+++ b/lldb/include/lldb/Interpreter/OptionArgParser.h
@@ -29,7 +29,8 @@ struct OptionArgParser {
static bool ToBoolean(llvm::StringRef s, bool fail_value, bool *success_ptr);
- static bool ToBoolean(llvm::StringRef option_name, llvm::StringRef option_arg, bool fail_value, Status &error);
+ static bool ToBoolean(llvm::StringRef option_name, llvm::StringRef option_arg,
+ bool fail_value, Status &error);
static char ToChar(llvm::StringRef s, char fail_value, bool *success_ptr);
diff --git a/lldb/include/lldb/Interpreter/Options.h b/lldb/include/lldb/Interpreter/Options.h
index a95111262c25f..9a6a17c2793fa 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -217,7 +217,7 @@ class Options {
void OptionsSetUnion(const OptionSet &set_a, const OptionSet &set_b,
OptionSet &union_set);
-
+
// Subclasses must reset their option values prior to starting a new option
// parse. Each subclass must override this function and revert all option
// settings to default values.
diff --git a/lldb/source/Commands/CommandObjectStats.cpp b/lldb/source/Commands/CommandObjectStats.cpp
index 929a79180070f..b093c6607d5d8 100644
--- a/lldb/source/Commands/CommandObjectStats.cpp
+++ b/lldb/source/Commands/CommandObjectStats.cpp
@@ -95,13 +95,16 @@ class CommandObjectStatsDump : public CommandObjectParsed {
m_stats_options.load_all_debug_info = true;
break;
case 'r':
- m_stats_options.include_targets = OptionArgParser::ToBoolean("--targets", option_arg, true /* doesn't matter */, error);
+ m_stats_options.include_targets = OptionArgParser::ToBoolean(
+ "--targets", option_arg, true /* doesn't matter */, error);
break;
case 'm':
- m_stats_options.include_modules = OptionArgParser::ToBoolean("--modules", option_arg, true /* doesn't matter */, error);
+ m_stats_options.include_modules = OptionArgParser::ToBoolean(
+ "--modules", option_arg, true /* doesn't matter */, error);
break;
case 't':
- m_stats_options.include_transcript = OptionArgParser::ToBoolean("--transcript", option_arg, true /* doesn't matter */, error);
+ m_stats_options.include_transcript = OptionArgParser::ToBoolean(
+ "--transcript", option_arg, true /* doesn't matter */, error);
break;
default:
llvm_unreachable("Unimplemented option");
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp b/lldb/source/Interpreter/OptionArgParser.cpp
index 963828d6bdda1..978032807ebd1 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -35,11 +35,11 @@ bool OptionArgParser::ToBoolean(llvm::StringRef ref, bool fail_value,
return fail_value;
}
-bool OptionArgParser::ToBoolean(llvm::StringRef option_name, llvm::StringRef option_arg, bool fail_value, Status &error)
-{
+bool OptionArgParser::ToBoolean(llvm::StringRef option_name,
+ llvm::StringRef option_arg, bool fail_value,
+ Status &error) {
bool parse_success;
- const bool arg_value =
- ToBoolean(option_arg, fail_value, &parse_success);
+ const bool arg_value = ToBoolean(option_arg, fail_value, &parse_success);
if (!parse_success)
error.SetErrorStringWithFormat(
"Invalid boolean value for option '%s': '%s'",
>From 140feac7a63b05e99b90767f321cd283b2c448a4 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Thu, 13 Jun 2024 17:14:22 -0700
Subject: [PATCH 08/15] Fix option descriptions
---
lldb/source/Commands/Options.td | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index b7e8fa22cdaa4..dffaf525ef052 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -1429,17 +1429,17 @@ let Command = "statistics dump" in {
Arg<"Boolean">,
Desc<"Dump statistics for the targets, including breakpoints, expression "
"evaluations, frame variables, etc. "
- "If both the '--targets' and the '--modules' options are specified, a "
- "list of module identifiers will be added to the 'targets' section.">;
+ "If both the '--targets' and the '--modules' options are 'true', a list "
+ "of module identifiers will be added to the 'targets' section.">;
def statistics_dump_modules: Option<"modules", "m">, Group<1>,
Arg<"Boolean">,
Desc<"Dump statistics for the modules, including time and size of various "
"aspects of the module and debug information, type system, path, etc. "
- "If both the '--targets' and the '--modules' options are specified, a "
- "list of module identifiers will be added to the 'targets' section.">;
+ "If both the '--targets' and the '--modules' options are 'true', a list "
+ "of module identifiers will be added to the 'targets' section.">;
def statistics_dump_transcript: Option<"transcript", "t">, Group<1>,
Arg<"Boolean">,
Desc<"If the setting interpreter.save-transcript is enabled and this "
- "option is specified, include a JSON array with all commands the user and/"
- "or scripts executed during a debug session.">;
+ "option is 'true', include a JSON array with all commands the user and/or "
+ "scripts executed during a debug session.">;
}
>From 1d9f8aa016133c644cd4979bb64289f9b42d2d31 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Thu, 13 Jun 2024 17:26:42 -0700
Subject: [PATCH 09/15] Add unittest for the new function
OptionArgParser::ToBoolean
---
.../Interpreter/TestOptionArgParser.cpp | 79 ++++++++-----------
1 file changed, 31 insertions(+), 48 deletions(-)
diff --git a/lldb/unittests/Interpreter/TestOptionArgParser.cpp b/lldb/unittests/Interpreter/TestOptionArgParser.cpp
index e173febcfa8fe..70ac5db7b2826 100644
--- a/lldb/unittests/Interpreter/TestOptionArgParser.cpp
+++ b/lldb/unittests/Interpreter/TestOptionArgParser.cpp
@@ -8,61 +8,44 @@
#include "gtest/gtest.h"
#include "lldb/Interpreter/OptionArgParser.h"
+#include "lldb/Utility/Status.h"
+#include "llvm/ADT/StringRef.h"
using namespace lldb_private;
-TEST(OptionArgParserTest, toBoolean) {
+void TestToBoolean(llvm::StringRef option_arg, bool fail_value, bool expected_value, bool expected_success) {
+ EXPECT_EQ(expected_value,
+ OptionArgParser::ToBoolean(option_arg, fail_value, nullptr));
+
bool success = false;
- EXPECT_TRUE(
- OptionArgParser::ToBoolean(llvm::StringRef("true"), false, nullptr));
- EXPECT_TRUE(
- OptionArgParser::ToBoolean(llvm::StringRef("on"), false, nullptr));
- EXPECT_TRUE(
- OptionArgParser::ToBoolean(llvm::StringRef("yes"), false, nullptr));
- EXPECT_TRUE(OptionArgParser::ToBoolean(llvm::StringRef("1"), false, nullptr));
-
- EXPECT_TRUE(
- OptionArgParser::ToBoolean(llvm::StringRef("true"), false, &success));
- EXPECT_TRUE(success);
- EXPECT_TRUE(
- OptionArgParser::ToBoolean(llvm::StringRef("on"), false, &success));
- EXPECT_TRUE(success);
- EXPECT_TRUE(
- OptionArgParser::ToBoolean(llvm::StringRef("yes"), false, &success));
- EXPECT_TRUE(success);
- EXPECT_TRUE(
- OptionArgParser::ToBoolean(llvm::StringRef("1"), false, &success));
- EXPECT_TRUE(success);
+ EXPECT_EQ(expected_value,
+ OptionArgParser::ToBoolean(option_arg, fail_value, &success));
+ EXPECT_EQ(expected_success, success);
- EXPECT_FALSE(
- OptionArgParser::ToBoolean(llvm::StringRef("false"), true, nullptr));
- EXPECT_FALSE(
- OptionArgParser::ToBoolean(llvm::StringRef("off"), true, nullptr));
- EXPECT_FALSE(
- OptionArgParser::ToBoolean(llvm::StringRef("no"), true, nullptr));
- EXPECT_FALSE(OptionArgParser::ToBoolean(llvm::StringRef("0"), true, nullptr));
+ Status status;
+ EXPECT_EQ(expected_value,
+ OptionArgParser::ToBoolean(llvm::StringRef("--test"), option_arg, fail_value, status));
+ EXPECT_EQ(expected_success, status.Success());
+}
- EXPECT_FALSE(
- OptionArgParser::ToBoolean(llvm::StringRef("false"), true, &success));
- EXPECT_TRUE(success);
- EXPECT_FALSE(
- OptionArgParser::ToBoolean(llvm::StringRef("off"), true, &success));
- EXPECT_TRUE(success);
- EXPECT_FALSE(
- OptionArgParser::ToBoolean(llvm::StringRef("no"), true, &success));
- EXPECT_TRUE(success);
- EXPECT_FALSE(
- OptionArgParser::ToBoolean(llvm::StringRef("0"), true, &success));
- EXPECT_TRUE(success);
+TEST(OptionArgParserTest, toBoolean) {
+ // "True"-ish values should be successfully parsed and return `true`.
+ TestToBoolean(llvm::StringRef("true"), false, true, true);
+ TestToBoolean(llvm::StringRef("on"), false, true, true);
+ TestToBoolean(llvm::StringRef("yes"), false, true, true);
+ TestToBoolean(llvm::StringRef("1"), false, true, true);
- EXPECT_FALSE(
- OptionArgParser::ToBoolean(llvm::StringRef("10"), false, &success));
- EXPECT_FALSE(success);
- EXPECT_TRUE(
- OptionArgParser::ToBoolean(llvm::StringRef("10"), true, &success));
- EXPECT_FALSE(success);
- EXPECT_TRUE(OptionArgParser::ToBoolean(llvm::StringRef(""), true, &success));
- EXPECT_FALSE(success);
+ // "False"-ish values should be successfully parsed and return `false`.
+ TestToBoolean(llvm::StringRef("false"), true, false, true);
+ TestToBoolean(llvm::StringRef("off"), true, false, true);
+ TestToBoolean(llvm::StringRef("no"), true, false, true);
+ TestToBoolean(llvm::StringRef("0"), true, false, true);
+
+ // Other values should fail the parse and return the given `fail_value`.
+ TestToBoolean(llvm::StringRef("10"), false, false, false);
+ TestToBoolean(llvm::StringRef("10"), true, true, false);
+ TestToBoolean(llvm::StringRef(""), false, false, false);
+ TestToBoolean(llvm::StringRef(""), true, true, false);
}
TEST(OptionArgParserTest, toChar) {
>From 8b6066b1d019f3ef987265afab71b19bd6f8127b Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Thu, 13 Jun 2024 17:30:07 -0700
Subject: [PATCH 10/15] Fix format
---
lldb/unittests/Interpreter/TestOptionArgParser.cpp | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/lldb/unittests/Interpreter/TestOptionArgParser.cpp b/lldb/unittests/Interpreter/TestOptionArgParser.cpp
index 70ac5db7b2826..8e5a100888e95 100644
--- a/lldb/unittests/Interpreter/TestOptionArgParser.cpp
+++ b/lldb/unittests/Interpreter/TestOptionArgParser.cpp
@@ -13,18 +13,20 @@
using namespace lldb_private;
-void TestToBoolean(llvm::StringRef option_arg, bool fail_value, bool expected_value, bool expected_success) {
+void TestToBoolean(llvm::StringRef option_arg, bool fail_value,
+ bool expected_value, bool expected_success) {
EXPECT_EQ(expected_value,
- OptionArgParser::ToBoolean(option_arg, fail_value, nullptr));
+ OptionArgParser::ToBoolean(option_arg, fail_value, nullptr));
bool success = false;
EXPECT_EQ(expected_value,
- OptionArgParser::ToBoolean(option_arg, fail_value, &success));
+ OptionArgParser::ToBoolean(option_arg, fail_value, &success));
EXPECT_EQ(expected_success, success);
Status status;
EXPECT_EQ(expected_value,
- OptionArgParser::ToBoolean(llvm::StringRef("--test"), option_arg, fail_value, status));
+ OptionArgParser::ToBoolean(llvm::StringRef("--test"), option_arg,
+ fail_value, status));
EXPECT_EQ(expected_success, status.Success());
}
>From d6073538548ebccb9e673967aacf34eaab95023b Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Thu, 13 Jun 2024 17:37:25 -0700
Subject: [PATCH 11/15] Minor fix in test
---
lldb/test/API/commands/statistics/basic/TestStats.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index f19ec89bfe777..10acca69b4ce2 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -724,7 +724,7 @@ def test_sections_existence(self):
"targets": False,
"targets.moduleIdentifiers": False,
"targets.breakpoints": False,
- "targets.expressionEvaluation": True,
+ "targets.expressionEvaluation": False,
"modules": True,
"transcript": False,
},
>From 6fca90c558948e35d86cdd678597caf2657e4aae Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 17 Jun 2024 08:52:37 -0700
Subject: [PATCH 12/15] Change StatisticsOptions to use accessors + optional
fields
---
lldb/include/lldb/Target/Statistics.h | 47 ++++-
lldb/source/API/SBStatisticsOptions.cpp | 23 +--
lldb/source/Commands/CommandObjectStats.cpp | 28 +--
lldb/source/Target/Statistics.cpp | 14 +-
.../commands/statistics/basic/TestStats.py | 169 ++++++++++++------
5 files changed, 185 insertions(+), 96 deletions(-)
diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index 268de2f1d9686..7e1533e7e00a0 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -131,11 +131,48 @@ struct ConstStringStats {
};
struct StatisticsOptions {
- bool summary_only = false;
- bool load_all_debug_info = false;
- bool include_targets = true;
- bool include_modules = true;
- bool include_transcript = true;
+public:
+ void SetSummaryOnly(bool value) { m_summary_only = value; }
+ bool GetSummaryOnly() const { return m_summary_only.value_or(false); }
+
+ void SetLoadAllDebugInfo(bool value) { m_load_all_debug_info = value; }
+ bool GetLoadAllDebugInfo() const {
+ return m_load_all_debug_info.value_or(false);
+ }
+
+ void SetIncludeTargets(bool value) { m_include_targets = value; }
+ bool GetIncludeTargets() const {
+ if (m_include_targets.has_value())
+ return m_include_targets.value();
+ // `m_include_targets` has no value set, so return a value base on
+ // `m_summary_only`
+ return !GetSummaryOnly();
+ }
+
+ void SetIncludeModules(bool value) { m_include_modules = value; }
+ bool GetIncludeModules() const {
+ if (m_include_modules.has_value())
+ return m_include_modules.value();
+ // `m_include_modules` has no value set, so return a value base on
+ // `m_summary_only`
+ return !GetSummaryOnly();
+ }
+
+ void SetIncludeTranscript(bool value) { m_include_transcript = value; }
+ bool GetIncludeTranscript() const {
+ if (m_include_transcript.has_value())
+ return m_include_transcript.value();
+ // `m_include_transcript` has no value set, so return a value base on
+ // `m_summary_only`
+ return !GetSummaryOnly();
+ }
+
+private:
+ std::optional<bool> m_summary_only;
+ std::optional<bool> m_load_all_debug_info;
+ std::optional<bool> m_include_transcript;
+ std::optional<bool> m_include_targets;
+ std::optional<bool> m_include_modules;
};
/// A class that represents statistics for a since lldb_private::Target.
diff --git a/lldb/source/API/SBStatisticsOptions.cpp b/lldb/source/API/SBStatisticsOptions.cpp
index 7cf8664955ae7..71d4048573b56 100644
--- a/lldb/source/API/SBStatisticsOptions.cpp
+++ b/lldb/source/API/SBStatisticsOptions.cpp
@@ -18,7 +18,6 @@ using namespace lldb_private;
SBStatisticsOptions::SBStatisticsOptions()
: m_opaque_up(new StatisticsOptions()) {
LLDB_INSTRUMENT_VA(this);
- m_opaque_up->summary_only = false;
}
SBStatisticsOptions::SBStatisticsOptions(const SBStatisticsOptions &rhs) {
@@ -39,41 +38,43 @@ SBStatisticsOptions::operator=(const SBStatisticsOptions &rhs) {
}
void SBStatisticsOptions::SetSummaryOnly(bool b) {
- m_opaque_up->summary_only = b;
+ m_opaque_up->SetSummaryOnly(b);
}
-bool SBStatisticsOptions::GetSummaryOnly() { return m_opaque_up->summary_only; }
+bool SBStatisticsOptions::GetSummaryOnly() {
+ return m_opaque_up->GetSummaryOnly();
+}
void SBStatisticsOptions::SetIncludeTargets(bool b) {
- m_opaque_up->include_targets = b;
+ m_opaque_up->SetIncludeTargets(b);
}
bool SBStatisticsOptions::GetIncludeTargets() const {
- return m_opaque_up->include_targets;
+ return m_opaque_up->GetIncludeTargets();
}
void SBStatisticsOptions::SetIncludeModules(bool b) {
- m_opaque_up->include_modules = b;
+ m_opaque_up->SetIncludeModules(b);
}
bool SBStatisticsOptions::GetIncludeModules() const {
- return m_opaque_up->include_modules;
+ return m_opaque_up->GetIncludeModules();
}
void SBStatisticsOptions::SetIncludeTranscript(bool b) {
- m_opaque_up->include_transcript = b;
+ m_opaque_up->SetIncludeTranscript(b);
}
bool SBStatisticsOptions::GetIncludeTranscript() const {
- return m_opaque_up->include_transcript;
+ return m_opaque_up->GetIncludeTranscript();
}
void SBStatisticsOptions::SetReportAllAvailableDebugInfo(bool b) {
- m_opaque_up->load_all_debug_info = b;
+ m_opaque_up->SetLoadAllDebugInfo(b);
}
bool SBStatisticsOptions::GetReportAllAvailableDebugInfo() {
- return m_opaque_up->load_all_debug_info;
+ return m_opaque_up->GetLoadAllDebugInfo();
}
const lldb_private::StatisticsOptions &SBStatisticsOptions::ref() const {
diff --git a/lldb/source/Commands/CommandObjectStats.cpp b/lldb/source/Commands/CommandObjectStats.cpp
index b093c6607d5d8..5541f46dfc55c 100644
--- a/lldb/source/Commands/CommandObjectStats.cpp
+++ b/lldb/source/Commands/CommandObjectStats.cpp
@@ -77,34 +77,22 @@ class CommandObjectStatsDump : public CommandObjectParsed {
m_all_targets = true;
break;
case 's':
- m_stats_options.summary_only = true;
- // In the "summary" mode, the following sections should be omitted by
- // default unless their corresponding options are explicitly given.
- // If such options were processed before 's', `m_seen_options` should
- // contain them, and we will skip setting them to `false` here. If such
- // options are not yet processed, we will set them to `false` here, and
- // they will be overwritten when the options are processed.
- if (m_seen_options.find((int)'r') == m_seen_options.end())
- m_stats_options.include_targets = false;
- if (m_seen_options.find((int)'m') == m_seen_options.end())
- m_stats_options.include_modules = false;
- if (m_seen_options.find((int)'t') == m_seen_options.end())
- m_stats_options.include_transcript = false;
+ m_stats_options.SetSummaryOnly(true);
break;
case 'f':
- m_stats_options.load_all_debug_info = true;
+ m_stats_options.SetLoadAllDebugInfo(true);
break;
case 'r':
- m_stats_options.include_targets = OptionArgParser::ToBoolean(
- "--targets", option_arg, true /* doesn't matter */, error);
+ m_stats_options.SetIncludeTargets(OptionArgParser::ToBoolean(
+ "--targets", option_arg, true /* doesn't matter */, error));
break;
case 'm':
- m_stats_options.include_modules = OptionArgParser::ToBoolean(
- "--modules", option_arg, true /* doesn't matter */, error);
+ m_stats_options.SetIncludeModules(OptionArgParser::ToBoolean(
+ "--modules", option_arg, true /* doesn't matter */, error));
break;
case 't':
- m_stats_options.include_transcript = OptionArgParser::ToBoolean(
- "--transcript", option_arg, true /* doesn't matter */, error);
+ m_stats_options.SetIncludeTranscript(OptionArgParser::ToBoolean(
+ "--transcript", option_arg, true /* doesn't matter */, error));
break;
default:
llvm_unreachable("Unimplemented option");
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 716f80a7fd6bb..583d1524881fc 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -107,8 +107,8 @@ TargetStats::ToJSON(Target &target,
const lldb_private::StatisticsOptions &options) {
json::Object target_metrics_json;
ProcessSP process_sp = target.GetProcessSP();
- const bool summary_only = options.summary_only;
- const bool include_modules = options.include_modules;
+ const bool summary_only = options.GetSummaryOnly();
+ const bool include_modules = options.GetIncludeModules();
if (!summary_only) {
CollectStats(target);
@@ -226,11 +226,11 @@ llvm::json::Value DebuggerStats::ReportStatistics(
Debugger &debugger, Target *target,
const lldb_private::StatisticsOptions &options) {
- const bool summary_only = options.summary_only;
- const bool load_all_debug_info = options.load_all_debug_info;
- const bool include_targets = options.include_targets;
- const bool include_modules = options.include_modules;
- const bool include_transcript = options.include_transcript;
+ const bool summary_only = options.GetSummaryOnly();
+ const bool load_all_debug_info = options.GetLoadAllDebugInfo();
+ const bool include_targets = options.GetIncludeTargets();
+ const bool include_modules = options.GetIncludeModules();
+ const bool include_transcript = options.GetIncludeTranscript();
json::Array json_targets;
json::Array json_modules;
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index 10acca69b4ce2..66c04a6d8bc77 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -658,30 +658,44 @@ def test_transcript_happy_path(self):
# The second "statistics dump" in the transcript should have no output
self.assertNotIn("output", transcript[2])
- def test_sections_existence(self):
- """
- Test "statistics dump" and the existence of sections when different
- options are given.
- """
- self.build()
- exe = self.getBuildArtifact("a.out")
- target = self.createTestTarget(file_path=exe)
-
- # Create some transcript so that it can be tested.
- self.runCmd("settings set interpreter.save-transcript true")
- self.runCmd("version")
- self.runCmd("b main")
- # But disable transcript so that it won't change during verification
- self.runCmd("settings set interpreter.save-transcript false")
+ def verify_stats(self, stats, expectation, options):
+ for field_name in expectation:
+ idx = field_name.find(".")
+ if idx == -1:
+ # `field` is a top-level field
+ exists = field_name in stats
+ should_exist = expectation[field_name]
+ should_exist_string = "" if should_exist else "not "
+ self.assertEqual(
+ exists,
+ should_exist,
+ f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'",
+ )
+ else:
+ # `field` is a string of "<top-level field>.<second-level field>"
+ top_level_field_name = field_name[0:idx]
+ second_level_field_name = field_name[idx + 1 :]
+ for top_level_field in (
+ stats[top_level_field_name] if top_level_field_name in stats else {}
+ ):
+ exists = second_level_field_name in top_level_field
+ should_exist = expectation[field_name]
+ should_exist_string = "" if should_exist else "not "
+ self.assertEqual(
+ exists,
+ should_exist,
+ f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'",
+ )
- # Expectation
+ def get_expectations_for_sections_existence(self):
should_always_exist_or_not = {
"totalDebugInfoEnabled": True,
"memory": True,
}
test_cases = [
{ # Everything mode
- "options": "",
+ "command_options": "",
+ "api_options": {},
"expect": {
"commands": True,
"targets": True,
@@ -693,7 +707,10 @@ def test_sections_existence(self):
},
},
{ # Summary mode
- "options": " --summary",
+ "command_options": " --summary",
+ "api_options": {
+ "SetSummaryOnly": True,
+ },
"expect": {
"commands": False,
"targets": False,
@@ -705,7 +722,11 @@ def test_sections_existence(self):
},
},
{ # Summary mode with targets
- "options": " --summary --targets=true",
+ "command_options": " --summary --targets=true",
+ "api_options": {
+ "SetSummaryOnly": True,
+ "SetIncludeTargets": True,
+ },
"expect": {
"commands": False,
"targets": True,
@@ -718,7 +739,11 @@ def test_sections_existence(self):
},
},
{ # Summary mode with modules
- "options": " --summary --modules=true",
+ "command_options": " --summary --modules=true",
+ "api_options": {
+ "SetSummaryOnly": True,
+ "SetIncludeModules": True,
+ },
"expect": {
"commands": False,
"targets": False,
@@ -730,7 +755,11 @@ def test_sections_existence(self):
},
},
{ # Everything mode but without modules and transcript
- "options": " --modules=false --transcript=false",
+ "command_options": " --modules=false --transcript=false",
+ "api_options": {
+ "SetIncludeModules": False,
+ "SetIncludeTranscript": False,
+ },
"expect": {
"commands": True,
"targets": True,
@@ -742,7 +771,10 @@ def test_sections_existence(self):
},
},
{ # Everything mode but without modules
- "options": " --modules=false",
+ "command_options": " --modules=false",
+ "api_options": {
+ "SetIncludeModules": False,
+ },
"expect": {
"commands": True,
"targets": True,
@@ -754,42 +786,73 @@ def test_sections_existence(self):
},
},
]
+ return (should_always_exist_or_not, test_cases)
+
+ def test_sections_existence_through_command(self):
+ """
+ Test "statistics dump" and the existence of sections when different
+ options are given through the command line (CLI or HandleCommand).
+ """
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ target = self.createTestTarget(file_path=exe)
+
+ # Create some transcript so that it can be tested.
+ self.runCmd("settings set interpreter.save-transcript true")
+ self.runCmd("version")
+ self.runCmd("b main")
+ # But disable transcript so that it won't change during verification
+ self.runCmd("settings set interpreter.save-transcript false")
+
+ # Expectation
+ should_always_exist_or_not, test_cases = (
+ self.get_expectations_for_sections_existence()
+ )
# Verification
for test_case in test_cases:
- options = test_case["options"]
- debug_stats = self.get_stats(options)
+ options = test_case["command_options"]
+ # Get statistics dump result
+ stats = self.get_stats(options)
# Verify that each field should exist (or not)
expectation = {**should_always_exist_or_not, **test_case["expect"]}
- for field_name in expectation:
- idx = field_name.find(".")
- if idx == -1:
- # `field` is a top-level field
- exists = field_name in debug_stats
- should_exist = expectation[field_name]
- should_exist_string = "" if should_exist else "not "
- self.assertEqual(
- exists,
- should_exist,
- f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'",
- )
- else:
- # `field` is a string of "<top-level field>.<second-level field>"
- top_level_field_name = field_name[0:idx]
- second_level_field_name = field_name[idx + 1 :]
- for top_level_field in (
- debug_stats[top_level_field_name]
- if top_level_field_name in debug_stats
- else []
- ):
- exists = second_level_field_name in top_level_field
- should_exist = expectation[field_name]
- should_exist_string = "" if should_exist else "not "
- self.assertEqual(
- exists,
- should_exist,
- f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'",
- )
+ self.verify_stats(stats, expectation, options)
+
+ def test_sections_existence_through_api(self):
+ """
+ Test "statistics dump" and the existence of sections when different
+ options are given through the public API.
+ """
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ target = self.createTestTarget(file_path=exe)
+
+ # Create some transcript so that it can be tested.
+ self.runCmd("settings set interpreter.save-transcript true")
+ self.runCmd("version")
+ self.runCmd("b main")
+ # But disable transcript so that it won't change during verification
+ self.runCmd("settings set interpreter.save-transcript false")
+
+ # Expectation
+ should_always_exist_or_not, test_cases = (
+ self.get_expectations_for_sections_existence()
+ )
+
+ # Verification
+ for test_case in test_cases:
+ # Create options
+ options = test_case["api_options"]
+ sb_options = lldb.SBStatisticsOptions()
+ for method_name, param_value in options.items():
+ getattr(sb_options, method_name)(param_value)
+ # Get statistics dump result
+ stream = lldb.SBStream()
+ target.GetStatistics(sb_options).GetAsJSON(stream)
+ stats = json.loads(stream.GetData())
+ # Verify that each field should exist (or not)
+ expectation = {**should_always_exist_or_not, **test_case["expect"]}
+ self.verify_stats(stats, expectation, options)
def test_order_of_options_do_not_matter(self):
"""
>From dfdf528d74a84670f05c2fc822dcff16be8716bd Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 17 Jun 2024 09:08:34 -0700
Subject: [PATCH 13/15] Minor fixes
---
lldb/include/lldb/Target/Statistics.h | 2 +-
lldb/source/Interpreter/Options.cpp | 1 -
.../API/commands/statistics/basic/TestStats.py | 14 +++++++-------
lldb/unittests/Interpreter/TestOptionArgParser.cpp | 1 -
4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index 7e1533e7e00a0..5c6fee6b8a7b1 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -170,9 +170,9 @@ struct StatisticsOptions {
private:
std::optional<bool> m_summary_only;
std::optional<bool> m_load_all_debug_info;
- std::optional<bool> m_include_transcript;
std::optional<bool> m_include_targets;
std::optional<bool> m_include_modules;
+ std::optional<bool> m_include_transcript;
};
/// A class that represents statistics for a since lldb_private::Target.
diff --git a/lldb/source/Interpreter/Options.cpp b/lldb/source/Interpreter/Options.cpp
index 65d95013fc903..4e7d074ace1b8 100644
--- a/lldb/source/Interpreter/Options.cpp
+++ b/lldb/source/Interpreter/Options.cpp
@@ -18,7 +18,6 @@
#include "lldb/Interpreter/CommandInterpreter.h"
#include "lldb/Interpreter/CommandObject.h"
#include "lldb/Interpreter/CommandReturnObject.h"
-#include "lldb/Interpreter/OptionArgParser.h"
#include "lldb/Target/Target.h"
#include "lldb/Utility/StreamString.h"
#include "llvm/ADT/STLExtras.h"
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index 66c04a6d8bc77..44dec7199fad7 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -662,7 +662,7 @@ def verify_stats(self, stats, expectation, options):
for field_name in expectation:
idx = field_name.find(".")
if idx == -1:
- # `field` is a top-level field
+ # `field_name` is a top-level field
exists = field_name in stats
should_exist = expectation[field_name]
should_exist_string = "" if should_exist else "not "
@@ -672,7 +672,7 @@ def verify_stats(self, stats, expectation, options):
f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'",
)
else:
- # `field` is a string of "<top-level field>.<second-level field>"
+ # `field_name` is a string of "<top-level field>.<second-level field>"
top_level_field_name = field_name[0:idx]
second_level_field_name = field_name[idx + 1 :]
for top_level_field in (
@@ -687,7 +687,7 @@ def verify_stats(self, stats, expectation, options):
f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'",
)
- def get_expectations_for_sections_existence(self):
+ def get_test_cases_for_sections_existence(self):
should_always_exist_or_not = {
"totalDebugInfoEnabled": True,
"memory": True,
@@ -801,12 +801,12 @@ def test_sections_existence_through_command(self):
self.runCmd("settings set interpreter.save-transcript true")
self.runCmd("version")
self.runCmd("b main")
- # But disable transcript so that it won't change during verification
+ # Then disable transcript so that it won't change during verification
self.runCmd("settings set interpreter.save-transcript false")
# Expectation
should_always_exist_or_not, test_cases = (
- self.get_expectations_for_sections_existence()
+ self.get_test_cases_for_sections_existence()
)
# Verification
@@ -836,7 +836,7 @@ def test_sections_existence_through_api(self):
# Expectation
should_always_exist_or_not, test_cases = (
- self.get_expectations_for_sections_existence()
+ self.get_test_cases_for_sections_existence()
)
# Verification
@@ -866,7 +866,7 @@ def test_order_of_options_do_not_matter(self):
self.runCmd("settings set interpreter.save-transcript true")
self.runCmd("version")
self.runCmd("b main")
- # But disable transcript so that it won't change during verification
+ # Then disable transcript so that it won't change during verification
self.runCmd("settings set interpreter.save-transcript false")
# The order of the following options shouldn't matter
diff --git a/lldb/unittests/Interpreter/TestOptionArgParser.cpp b/lldb/unittests/Interpreter/TestOptionArgParser.cpp
index 8e5a100888e95..c17a443421ad5 100644
--- a/lldb/unittests/Interpreter/TestOptionArgParser.cpp
+++ b/lldb/unittests/Interpreter/TestOptionArgParser.cpp
@@ -9,7 +9,6 @@
#include "gtest/gtest.h"
#include "lldb/Interpreter/OptionArgParser.h"
#include "lldb/Utility/Status.h"
-#include "llvm/ADT/StringRef.h"
using namespace lldb_private;
>From 7b17e773efff1e538f898736cfd591175120cfa1 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 17 Jun 2024 13:09:37 -0700
Subject: [PATCH 14/15] Fix format
---
.../API/commands/statistics/basic/TestStats.py | 14 ++++++++------
lldb/unittests/Interpreter/TestOptionArgParser.cpp | 2 +-
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index 44dec7199fad7..136c44e17c645 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -805,9 +805,10 @@ def test_sections_existence_through_command(self):
self.runCmd("settings set interpreter.save-transcript false")
# Expectation
- should_always_exist_or_not, test_cases = (
- self.get_test_cases_for_sections_existence()
- )
+ (
+ should_always_exist_or_not,
+ test_cases,
+ ) = self.get_test_cases_for_sections_existence()
# Verification
for test_case in test_cases:
@@ -835,9 +836,10 @@ def test_sections_existence_through_api(self):
self.runCmd("settings set interpreter.save-transcript false")
# Expectation
- should_always_exist_or_not, test_cases = (
- self.get_test_cases_for_sections_existence()
- )
+ (
+ should_always_exist_or_not,
+ test_cases,
+ ) = self.get_test_cases_for_sections_existence()
# Verification
for test_case in test_cases:
diff --git a/lldb/unittests/Interpreter/TestOptionArgParser.cpp b/lldb/unittests/Interpreter/TestOptionArgParser.cpp
index c17a443421ad5..ca6a0c30f828b 100644
--- a/lldb/unittests/Interpreter/TestOptionArgParser.cpp
+++ b/lldb/unittests/Interpreter/TestOptionArgParser.cpp
@@ -6,9 +6,9 @@
//
//===----------------------------------------------------------------------===//
-#include "gtest/gtest.h"
#include "lldb/Interpreter/OptionArgParser.h"
#include "lldb/Utility/Status.h"
+#include "gtest/gtest.h"
using namespace lldb_private;
>From 64afbe153da892912c76e0e9afcddb1a5d2d80b8 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 17 Jun 2024 13:15:44 -0700
Subject: [PATCH 15/15] Minor fixes
---
lldb/include/lldb/Target/Statistics.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index 5c6fee6b8a7b1..122eb3ddd711f 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -144,8 +144,8 @@ struct StatisticsOptions {
bool GetIncludeTargets() const {
if (m_include_targets.has_value())
return m_include_targets.value();
- // `m_include_targets` has no value set, so return a value base on
- // `m_summary_only`
+ // `m_include_targets` has no value set, so return a value based on
+ // `m_summary_only`.
return !GetSummaryOnly();
}
@@ -153,8 +153,8 @@ struct StatisticsOptions {
bool GetIncludeModules() const {
if (m_include_modules.has_value())
return m_include_modules.value();
- // `m_include_modules` has no value set, so return a value base on
- // `m_summary_only`
+ // `m_include_modules` has no value set, so return a value based on
+ // `m_summary_only`.
return !GetSummaryOnly();
}
@@ -162,8 +162,8 @@ struct StatisticsOptions {
bool GetIncludeTranscript() const {
if (m_include_transcript.has_value())
return m_include_transcript.value();
- // `m_include_transcript` has no value set, so return a value base on
- // `m_summary_only`
+ // `m_include_transcript` has no value set, so return a value based on
+ // `m_summary_only`.
return !GetSummaryOnly();
}
More information about the lldb-commits
mailing list