[Lldb-commits] [lldb] [lldb] Fix term-width setting (PR #82736)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

<details>
<summary>Changes</summary>

I noticed that the term-width setting would always report its default value (80) despite the driver correctly setting the value with SBDebugger::SetTerminalWidth.

  (lldb) settings show term-width
  term-width (int) = 80

The issue is that the setting was defined as a SInt64 instead of a UInt64 while the getter returned an unsigned value. There's no reason the terminal width should be a signed value. My best guess it that it was using SInt64 because UInt64 didn't support min and max values. I fixed that and correct the type and now lldb reports the correct terminal width:

  (lldb) settings show term-width
  term-width (unsigned) = 189

---
Full diff: https://github.com/llvm/llvm-project/pull/82736.diff


5 Files Affected:

- (modified) lldb/include/lldb/Interpreter/OptionValueUInt64.h (+24-2) 
- (modified) lldb/source/Core/CoreProperties.td (+1-1) 
- (modified) lldb/source/Core/Debugger.cpp (+2-2) 
- (modified) lldb/test/API/commands/settings/TestSettings.py (+11-4) 
- (modified) lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py (+2-1) 


``````````diff
diff --git a/lldb/include/lldb/Interpreter/OptionValueUInt64.h b/lldb/include/lldb/Interpreter/OptionValueUInt64.h
index 30c27bf73d99c6..b20fc880869c65 100644
--- a/lldb/include/lldb/Interpreter/OptionValueUInt64.h
+++ b/lldb/include/lldb/Interpreter/OptionValueUInt64.h
@@ -64,13 +64,35 @@ class OptionValueUInt64 : public Cloneable<OptionValueUInt64, OptionValue> {
 
   uint64_t GetDefaultValue() const { return m_default_value; }
 
-  void SetCurrentValue(uint64_t value) { m_current_value = value; }
+  bool SetCurrentValue(uint64_t value) {
+    if (value >= m_min_value && value <= m_max_value) {
+      m_current_value = value;
+      return true;
+    }
+    return false;
+  }
+
+  bool SetDefaultValue(uint64_t value) {
+    if (value >= m_min_value && value <= m_max_value) {
+      m_default_value = value;
+      return true;
+    }
+    return false;
+  }
+
+  void SetMinimumValue(int64_t v) { m_min_value = v; }
+
+  uint64_t GetMinimumValue() const { return m_min_value; }
+
+  void SetMaximumValue(int64_t v) { m_max_value = v; }
 
-  void SetDefaultValue(uint64_t value) { m_default_value = value; }
+  int64_t GetMaximumValue() const { return m_max_value; }
 
 protected:
   uint64_t m_current_value = 0;
   uint64_t m_default_value = 0;
+  uint64_t m_min_value = INT64_MIN;
+  uint64_t m_max_value = INT64_MAX;
 };
 
 } // namespace lldb_private
diff --git a/lldb/source/Core/CoreProperties.td b/lldb/source/Core/CoreProperties.td
index 4cfff805688c56..a6cb951187a040 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -132,7 +132,7 @@ let Definition = "debugger" in {
     Global,
     DefaultStringValue<"${ansi.normal}">,
     Desc<"When displaying the line marker in a color-enabled terminal, use the ANSI terminal code specified in this format immediately after the line to be marked.">;
-  def TerminalWidth: Property<"term-width", "SInt64">,
+  def TerminalWidth: Property<"term-width", "UInt64">,
     Global,
     DefaultUnsignedValue<80>,
     Desc<"The maximum number of columns to use for displaying text.">;
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 97311b4716ac2f..bb81110ae35a5d 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -886,8 +886,8 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, void *baton)
   }
   assert(m_dummy_target_sp.get() && "Couldn't construct dummy target?");
 
-  OptionValueSInt64 *term_width =
-      m_collection_sp->GetPropertyAtIndexAsOptionValueSInt64(
+  OptionValueUInt64 *term_width =
+      m_collection_sp->GetPropertyAtIndexAsOptionValueUInt64(
           ePropertyTerminalWidth);
   term_width->SetMinimumValue(10);
   term_width->SetMaximumValue(1024);
diff --git a/lldb/test/API/commands/settings/TestSettings.py b/lldb/test/API/commands/settings/TestSettings.py
index a2d845493d1df9..104a9f09788c37 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -2,7 +2,6 @@
 Test lldb settings command.
 """
 
-
 import json
 import os
 import re
@@ -151,14 +150,22 @@ def test_set_term_width(self):
         self.expect(
             "settings show term-width",
             SETTING_MSG("term-width"),
-            startstr="term-width (int) = 70",
+            startstr="term-width (unsigned) = 70",
         )
 
         # The overall display should also reflect the new setting.
         self.expect(
             "settings show",
             SETTING_MSG("term-width"),
-            substrs=["term-width (int) = 70"],
+            substrs=["term-width (unsigned) = 70"],
+        )
+
+        self.dbg.SetTerminalWidth(60)
+
+        self.expect(
+            "settings show",
+            SETTING_MSG("term-width"),
+            substrs=["term-width (unsigned) = 60"],
         )
 
     # rdar://problem/10712130
@@ -593,7 +600,7 @@ def test_settings_with_trailing_whitespace(self):
         self.expect(
             "settings show term-width",
             SETTING_MSG("term-width"),
-            startstr="term-width (int) = 60",
+            startstr="term-width (unsigned) = 60",
         )
         self.runCmd("settings clear term-width", check=False)
         # string
diff --git a/lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py b/lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
index 357999b6f56193..ee35dbd23b3dbe 100644
--- a/lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
+++ b/lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
@@ -24,7 +24,8 @@ def do_test(self, term_width, pattern_list):
         )
         self.expect("set set term-width " + str(term_width))
         self.expect(
-            "set show term-width", substrs=["term-width (int) = " + str(term_width)]
+            "set show term-width",
+            substrs=["term-width (unsigned) = " + str(term_width)],
         )
 
         self.child.send("file " + self.getBuildArtifact("a.out") + "\n")

``````````

</details>


https://github.com/llvm/llvm-project/pull/82736