Author: Jonas Devlieghere Date: 2024-02-22T21:48:49-08:00 New Revision: afd469023aad10786eaea3d444047a558ad8d5c1
URL: https://github.com/llvm/llvm-project/commit/afd469023aad10786eaea3d444047a558ad8d5c1 DIFF: https://github.com/llvm/llvm-project/commit/afd469023aad10786eaea3d444047a558ad8d5c1.diff LOG: [lldb] Fix term-width setting (#82736) 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 ``` rdar://123488999 Added: Modified: lldb/include/lldb/Interpreter/OptionValueSInt64.h lldb/include/lldb/Interpreter/OptionValueUInt64.h lldb/source/Core/CoreProperties.td lldb/source/Core/Debugger.cpp lldb/source/Interpreter/OptionValueUInt64.cpp lldb/test/API/commands/settings/TestSettings.py lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py Removed: ################################################################################ diff --git a/lldb/include/lldb/Interpreter/OptionValueSInt64.h b/lldb/include/lldb/Interpreter/OptionValueSInt64.h index 5efae627758aca..3cf41d38c0ef0c 100644 --- a/lldb/include/lldb/Interpreter/OptionValueSInt64.h +++ b/lldb/include/lldb/Interpreter/OptionValueSInt64.h @@ -86,8 +86,8 @@ class OptionValueSInt64 : public Cloneable<OptionValueSInt64, OptionValue> { protected: int64_t m_current_value = 0; int64_t m_default_value = 0; - int64_t m_min_value = INT64_MIN; - int64_t m_max_value = INT64_MAX; + int64_t m_min_value = std::numeric_limits<int64_t>::min(); + int64_t m_max_value = std::numeric_limits<int64_t>::max(); }; } // namespace lldb_private diff --git a/lldb/include/lldb/Interpreter/OptionValueUInt64.h b/lldb/include/lldb/Interpreter/OptionValueUInt64.h index 30c27bf73d99c6..07076075790c67 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; } + uint64_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 = std::numeric_limits<uint64_t>::min(); + uint64_t m_max_value = std::numeric_limits<uint64_t>::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/source/Interpreter/OptionValueUInt64.cpp b/lldb/source/Interpreter/OptionValueUInt64.cpp index 1999c63d11aff5..2e69c164e32ac3 100644 --- a/lldb/source/Interpreter/OptionValueUInt64.cpp +++ b/lldb/source/Interpreter/OptionValueUInt64.cpp @@ -47,9 +47,16 @@ Status OptionValueUInt64::SetValueFromString(llvm::StringRef value_ref, llvm::StringRef value_trimmed = value_ref.trim(); uint64_t value; if (llvm::to_integer(value_trimmed, value)) { - m_value_was_set = true; - m_current_value = value; - NotifyValueChanged(); + if (value >= m_min_value && value <= m_max_value) { + m_value_was_set = true; + m_current_value = value; + NotifyValueChanged(); + } else { + error.SetErrorStringWithFormat( + "%" PRIu64 " is out of range, valid values must be between %" PRIu64 + " and %" PRIu64 ".", + value, m_min_value, m_max_value); + } } else { error.SetErrorStringWithFormat("invalid uint64_t string value: '%s'", value_ref.str().c_str()); 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") _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
