[Lldb-commits] [lldb] 70f41a8 - [lldb] Add/change options in `statistics dump` to control what sections are dumped (#95075)

via lldb-commits lldb-commits at lists.llvm.org
Tue Jun 18 17:21:24 PDT 2024


Author: royitaqi
Date: 2024-06-18T17:21:20-07:00
New Revision: 70f41a8c305478cb16bcda9f9967af96ab1e3a20

URL: https://github.com/llvm/llvm-project/commit/70f41a8c305478cb16bcda9f9967af96ab1e3a20
DIFF: https://github.com/llvm/llvm-project/commit/70f41a8c305478cb16bcda9f9967af96ab1e3a20.diff

LOG: [lldb] Add/change options in `statistics dump` to control what sections are dumped (#95075)

# Added/changed options

The following options are **added** to the `statistics dump` command:
* `--targets=bool`: Boolean. Dumps the `targets` section.
* `--modules=bool`: Boolean. Dumps the `modules` section.
When both options are given, the field `moduleIdentifiers` will be
dumped for each target in the `targets` section.

The following options are **changed**:
* `--transcript=bool`: Changed to a boolean. Dumps the `transcript`
section.

# Behavior of `statistics dump` with various options

The behavior is **backward compatible**:
- When no options are provided, `statistics dump` dumps all sections.
- When `--summary` is provided, only dumps the summary info.

**New** behavior:
- `--targets=bool`, `--modules=bool`, `--transcript=bool` overrides the
above "default".

For **example**:
- `statistics dump --modules=false` dumps summary + targets +
transcript. No modules.
- `statistics dump --summary --targets=true --transcript=true` dumps
summary + targets (in summary mode) + transcript.


# Added options into public API

In `SBStatisticsOptions`, add:
* `Set/GetIncludeTargets`
* `Set/GetIncludeModules`
* `Set/GetIncludeTranscript`

**Alternative considered**: Thought about adding
`Set/GetIncludeSections(string sections_spec)`, which receives a
comma-separated list of section names to be included ("targets",
"modules", "transcript"). The **benefit** of this approach is that the
API is more future-proof when it comes to possible adding/changing of
section names. **However**, I feel the section names are likely to
remain unchanged for a while - it's not like we plan to make big changes
to the output of `statistics dump` any time soon. The **downsides** of
this approach are: 1\ the readability of the API is worse (requires
reading doc to understand what string can be accepted), 2\ string input
are more prone to human error (e.g. typo "target" instead of expected
"targets").


# Tests

```
bin/llvm-lit -sv ../external/llvm-project/lldb/test/API/commands/statistics/basic/TestStats.py
```

```
./tools/lldb/unittests/Interpreter/InterpreterTests
```

New test cases have been added to verify:
* Different sections are dumped/not dumped when different
`StatisticsOptions` are given through command line (CLI or
`HandleCommand`; see `test_sections_existence_through_command`) or API
(see `test_sections_existence_through_api`).
* The order in which the options are given in command line does not
matter (see `test_order_of_options_do_not_matter`).

---------

Co-authored-by: Roy Shi <royshi at meta.com>

Added: 
    

Modified: 
    lldb/include/lldb/API/SBStatisticsOptions.h
    lldb/include/lldb/Interpreter/OptionArgParser.h
    lldb/include/lldb/Target/Statistics.h
    lldb/source/API/SBStatisticsOptions.cpp
    lldb/source/Commands/CommandObjectStats.cpp
    lldb/source/Commands/Options.td
    lldb/source/Interpreter/OptionArgParser.cpp
    lldb/source/Target/Statistics.cpp
    lldb/test/API/commands/statistics/basic/TestStats.py
    lldb/unittests/Interpreter/TestOptionArgParser.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/API/SBStatisticsOptions.h b/lldb/include/lldb/API/SBStatisticsOptions.h
index a0055135e36c2..74bea03eff9c9 100644
--- a/lldb/include/lldb/API/SBStatisticsOptions.h
+++ b/lldb/include/lldb/API/SBStatisticsOptions.h
@@ -22,9 +22,46 @@ class LLDB_API SBStatisticsOptions {
 
   const SBStatisticsOptions &operator=(const lldb::SBStatisticsOptions &rhs);
 
+  /// If true, dump only high-level summary statistics. Exclude details like
+  /// targets, modules, breakpoints, etc. This turns off `IncludeTargets`,
+  /// `IncludeModules` and `IncludeTranscript` by default.
+  ///
+  /// Defaults to false.
   void SetSummaryOnly(bool b);
   bool GetSummaryOnly();
 
+  /// If true, dump statistics for the targets, including breakpoints,
+  /// expression evaluations, frame variables, etc.
+  ///
+  /// Defaults to true, unless the `SummaryOnly` mode is enabled, in which case
+  /// this is turned off unless specified.
+  ///
+  /// If both `IncludeTargets` and `IncludeModules` are true, a list of module
+  /// identifiers will be added to the "targets" section.
+  void SetIncludeTargets(bool b);
+  bool GetIncludeTargets() const;
+
+  /// If true, dump statistics for the modules, including time and size of
+  /// various aspects of the module and debug information, type system, path,
+  /// etc.
+  ///
+  /// Defaults to true, unless the `SummaryOnly` mode is enabled, in which case
+  /// this is turned off unless specified.
+  ///
+  /// If both `IncludeTargets` and `IncludeModules` are true, a list of module
+  /// identifiers will be added to the "targets" section.
+  void SetIncludeModules(bool b);
+  bool GetIncludeModules() const;
+
+  /// If true and the setting `interpreter.save-transcript` is enabled, include
+  /// a JSON array with all commands the user and/or scripts executed during a
+  /// debug session.
+  ///
+  /// Defaults to true, unless the `SummaryOnly` mode is enabled, in which case
+  /// this is turned off unless specified.
+  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/include/lldb/Interpreter/OptionArgParser.h b/lldb/include/lldb/Interpreter/OptionArgParser.h
index 66b16112fce96..76a48fca69208 100644
--- a/lldb/include/lldb/Interpreter/OptionArgParser.h
+++ b/lldb/include/lldb/Interpreter/OptionArgParser.h
@@ -29,6 +29,9 @@ struct OptionArgParser {
 
   static bool ToBoolean(llvm::StringRef s, bool fail_value, bool *success_ptr);
 
+  static llvm::Expected<bool> ToBoolean(llvm::StringRef option_name,
+                                        llvm::StringRef option_arg);
+
   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/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index c04d529290fff..122eb3ddd711f 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -131,9 +131,48 @@ struct ConstStringStats {
 };
 
 struct StatisticsOptions {
-  bool summary_only = false;
-  bool load_all_debug_info = false;
-  bool include_transcript = false;
+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 based 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 based 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 based 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_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/API/SBStatisticsOptions.cpp b/lldb/source/API/SBStatisticsOptions.cpp
index 7e826c4c93ebc..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,17 +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->SetIncludeTargets(b);
+}
+
+bool SBStatisticsOptions::GetIncludeTargets() const {
+  return m_opaque_up->GetIncludeTargets();
+}
+
+void SBStatisticsOptions::SetIncludeModules(bool b) {
+  m_opaque_up->SetIncludeModules(b);
+}
+
+bool SBStatisticsOptions::GetIncludeModules() const {
+  return m_opaque_up->GetIncludeModules();
+}
+
+void SBStatisticsOptions::SetIncludeTranscript(bool b) {
+  m_opaque_up->SetIncludeTranscript(b);
+}
+
+bool SBStatisticsOptions::GetIncludeTranscript() const {
+  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 1935b0fdfadfb..53855e7d03165 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;
@@ -76,13 +77,31 @@ class CommandObjectStatsDump : public CommandObjectParsed {
         m_all_targets = true;
         break;
       case 's':
-        m_stats_options.summary_only = true;
+        m_stats_options.SetSummaryOnly(true);
         break;
       case 'f':
-        m_stats_options.load_all_debug_info = true;
+        m_stats_options.SetLoadAllDebugInfo(true);
+        break;
+      case 'r':
+        if (llvm::Expected<bool> bool_or_error =
+                OptionArgParser::ToBoolean("--targets", option_arg))
+          m_stats_options.SetIncludeTargets(*bool_or_error);
+        else
+          error = bool_or_error.takeError();
+        break;
+      case 'm':
+        if (llvm::Expected<bool> bool_or_error =
+                OptionArgParser::ToBoolean("--modules", option_arg))
+          m_stats_options.SetIncludeModules(*bool_or_error);
+        else
+          error = bool_or_error.takeError();
         break;
       case 't':
-        m_stats_options.include_transcript = true;
+        if (llvm::Expected<bool> bool_or_error =
+                OptionArgParser::ToBoolean("--transcript", option_arg))
+          m_stats_options.SetIncludeTranscript(*bool_or_error);
+        else
+          error = bool_or_error.takeError();
         break;
       default:
         llvm_unreachable("Unimplemented option");

diff  --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index cee5a81d3796b..ba256e5ab917a 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -1425,8 +1425,28 @@ let Command = "statistics dump" in {
     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<1>,
+    Arg<"Boolean">,
+    Desc<"Dump statistics for the targets, including breakpoints, expression "
+    "evaluations, frame variables, etc. "
+    "Defaults to true, unless the '--summary' mode is enabled, in which case "
+    "this is turned off unless specified. "
+    "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. "
+    "Defaults to true, unless the '--summary' mode is enabled, in which case "
+    "this is turned off unless specified. "
+    "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. "
+    "Defaults to true, unless the '--summary' mode is enabled, in which case "
+    "this is turned off unless specified.">;
+
 }

diff  --git a/lldb/source/Interpreter/OptionArgParser.cpp b/lldb/source/Interpreter/OptionArgParser.cpp
index 9a8275128ede9..105d4846da148 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -35,6 +35,20 @@ bool OptionArgParser::ToBoolean(llvm::StringRef ref, bool fail_value,
   return fail_value;
 }
 
+llvm::Expected<bool> OptionArgParser::ToBoolean(llvm::StringRef option_name,
+                                                llvm::StringRef option_arg) {
+  bool parse_success;
+  const bool option_value =
+      ToBoolean(option_arg, false /* doesn't matter */, &parse_success);
+  if (parse_success)
+    return option_value;
+  else
+    return llvm::createStringError(
+        "Invalid boolean value for option '%s': '%s'",
+        option_name.str().c_str(),
+        option_arg.empty() ? "<null>" : option_arg.str().c_str());
+}
+
 char OptionArgParser::ToChar(llvm::StringRef s, char fail_value,
                              bool *success_ptr) {
   if (success_ptr)

diff  --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 2a5300012511a..583d1524881fc 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -107,7 +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 summary_only = options.GetSummaryOnly();
+  const bool include_modules = options.GetIncludeModules();
   if (!summary_only) {
     CollectStats(target);
 
@@ -117,8 +118,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_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 =
@@ -224,9 +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_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;
@@ -314,7 +318,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
     if (module_stat.debug_info_had_incomplete_types)
       ++num_modules_with_incomplete_types;
 
-    if (!summary_only) {
+    if (include_modules) {
       module_stat.identifier = (intptr_t)module;
       module_stat.path = module->GetFileSpec().GetPath();
       if (ConstString object_name = module->GetObjectName()) {
@@ -347,13 +351,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_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,10 +368,13 @@ 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_modules) {
+    global_stats.try_emplace("modules", std::move(json_modules));
+  }
+
   if (include_transcript) {
     // When transcript is available, add it to the to-be-returned statistics.
     //

diff  --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index ebe6166eb45e5..136c44e17c645 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)
@@ -657,16 +658,239 @@ 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 verify_stats(self, stats, expectation, options):
+        for field_name in expectation:
+            idx = field_name.find(".")
+            if idx == -1:
+                # `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 "
+                self.assertEqual(
+                    exists,
+                    should_exist,
+                    f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'",
+                )
+            else:
+                # `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 (
+                    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}'",
+                    )
+
+    def get_test_cases_for_sections_existence(self):
+        should_always_exist_or_not = {
+            "totalDebugInfoEnabled": True,
+            "memory": True,
+        }
+        test_cases = [
+            {  # Everything mode
+                "command_options": "",
+                "api_options": {},
+                "expect": {
+                    "commands": True,
+                    "targets": True,
+                    "targets.moduleIdentifiers": True,
+                    "targets.breakpoints": True,
+                    "targets.expressionEvaluation": True,
+                    "modules": True,
+                    "transcript": True,
+                },
+            },
+            {  # Summary mode
+                "command_options": " --summary",
+                "api_options": {
+                    "SetSummaryOnly": True,
+                },
+                "expect": {
+                    "commands": False,
+                    "targets": False,
+                    "targets.moduleIdentifiers": False,
+                    "targets.breakpoints": False,
+                    "targets.expressionEvaluation": False,
+                    "modules": False,
+                    "transcript": False,
+                },
+            },
+            {  # Summary mode with targets
+                "command_options": " --summary --targets=true",
+                "api_options": {
+                    "SetSummaryOnly": True,
+                    "SetIncludeTargets": True,
+                },
+                "expect": {
+                    "commands": False,
+                    "targets": True,
+                    "targets.moduleIdentifiers": False,
+                    "targets.breakpoints": False,
+                    "targets.expressionEvaluation": False,
+                    "targets.totalSharedLibraryEventHitCount": True,
+                    "modules": False,
+                    "transcript": False,
+                },
+            },
+            {  # Summary mode with modules
+                "command_options": " --summary --modules=true",
+                "api_options": {
+                    "SetSummaryOnly": True,
+                    "SetIncludeModules": True,
+                },
+                "expect": {
+                    "commands": False,
+                    "targets": False,
+                    "targets.moduleIdentifiers": False,
+                    "targets.breakpoints": False,
+                    "targets.expressionEvaluation": False,
+                    "modules": True,
+                    "transcript": False,
+                },
+            },
+            {  # Everything mode but without modules and transcript
+                "command_options": " --modules=false --transcript=false",
+                "api_options": {
+                    "SetIncludeModules": False,
+                    "SetIncludeTranscript": False,
+                },
+                "expect": {
+                    "commands": True,
+                    "targets": True,
+                    "targets.moduleIdentifiers": False,
+                    "targets.breakpoints": True,
+                    "targets.expressionEvaluation": True,
+                    "modules": False,
+                    "transcript": False,
+                },
+            },
+            {  # Everything mode but without modules
+                "command_options": " --modules=false",
+                "api_options": {
+                    "SetIncludeModules": False,
+                },
+                "expect": {
+                    "commands": True,
+                    "targets": True,
+                    "targets.moduleIdentifiers": False,
+                    "targets.breakpoints": True,
+                    "targets.expressionEvaluation": True,
+                    "modules": False,
+                    "transcript": True,
+                },
+            },
+        ]
+        return (should_always_exist_or_not, test_cases)
+
+    def test_sections_existence_through_command(self):
         """
-        Test "statistics dump" and the transcript information.
+        Test "statistics dump" and the existence of sections when 
diff erent
+        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")
+        # 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_test_cases_for_sections_existence()
+
+        # Verification
+        for test_case in test_cases:
+            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"]}
+            self.verify_stats(stats, expectation, options)
+
+    def test_sections_existence_through_api(self):
+        """
+        Test "statistics dump" and the existence of sections when 
diff erent
+        options are given through the public API.
+        """
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        target = self.createTestTarget(file_path=exe)
 
-        # Verify the output of a first "statistics dump"
-        debug_stats = self.get_stats()  # Not with "--transcript"
-        self.assertNotIn("transcript", debug_stats)
+        # 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_test_cases_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):
+        """
+        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")
+        # 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
+        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",
+            )

diff  --git a/lldb/unittests/Interpreter/TestOptionArgParser.cpp b/lldb/unittests/Interpreter/TestOptionArgParser.cpp
index e173febcfa8fe..25335e5396fbd 100644
--- a/lldb/unittests/Interpreter/TestOptionArgParser.cpp
+++ b/lldb/unittests/Interpreter/TestOptionArgParser.cpp
@@ -8,6 +8,7 @@
 
 #include "gtest/gtest.h"
 #include "lldb/Interpreter/OptionArgParser.h"
+#include "llvm/Support/Error.h"
 
 using namespace lldb_private;
 
@@ -65,6 +66,45 @@ TEST(OptionArgParserTest, toBoolean) {
   EXPECT_FALSE(success);
 }
 
+void TestToBooleanWithExpectedBool(llvm::StringRef option_arg,
+                                   bool expected_parse_success,
+                                   bool expected_result) {
+  llvm::Expected<bool> bool_or_error =
+      OptionArgParser::ToBoolean(llvm::StringRef("test_option"), option_arg);
+  EXPECT_EQ(expected_parse_success, (bool)bool_or_error);
+  if (expected_parse_success)
+    EXPECT_EQ(expected_result, *bool_or_error);
+  else {
+    std::string error = llvm::toString(bool_or_error.takeError());
+    EXPECT_NE(std::string::npos, error.find("test_option"));
+  }
+}
+
+TEST(OptionArgParserTest, toBooleanWithExpectedBool) {
+  TestToBooleanWithExpectedBool(llvm::StringRef("true"), true, true);
+  TestToBooleanWithExpectedBool(llvm::StringRef("on"), true, true);
+  TestToBooleanWithExpectedBool(llvm::StringRef("yes"), true, true);
+  TestToBooleanWithExpectedBool(llvm::StringRef("1"), true, true);
+
+  TestToBooleanWithExpectedBool(llvm::StringRef("True"), true, true);
+  TestToBooleanWithExpectedBool(llvm::StringRef("On"), true, true);
+  TestToBooleanWithExpectedBool(llvm::StringRef("Yes"), true, true);
+
+  TestToBooleanWithExpectedBool(llvm::StringRef("false"), true, false);
+  TestToBooleanWithExpectedBool(llvm::StringRef("off"), true, false);
+  TestToBooleanWithExpectedBool(llvm::StringRef("no"), true, false);
+  TestToBooleanWithExpectedBool(llvm::StringRef("0"), true, false);
+
+  TestToBooleanWithExpectedBool(llvm::StringRef("False"), true, false);
+  TestToBooleanWithExpectedBool(llvm::StringRef("Off"), true, false);
+  TestToBooleanWithExpectedBool(llvm::StringRef("No"), true, false);
+
+  TestToBooleanWithExpectedBool(llvm::StringRef("10"), false,
+                                false /* doesn't matter */);
+  TestToBooleanWithExpectedBool(llvm::StringRef(""), false,
+                                false /* doesn't matter */);
+}
+
 TEST(OptionArgParserTest, toChar) {
   bool success = false;
 


        


More information about the lldb-commits mailing list