Skip to content

Commit afd4690

Browse files
authored
[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
1 parent 0d72fe9 commit afd4690

File tree

7 files changed

+52
-15
lines changed

7 files changed

+52
-15
lines changed

lldb/include/lldb/Interpreter/OptionValueSInt64.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ class OptionValueSInt64 : public Cloneable<OptionValueSInt64, OptionValue> {
8686
protected:
8787
int64_t m_current_value = 0;
8888
int64_t m_default_value = 0;
89-
int64_t m_min_value = INT64_MIN;
90-
int64_t m_max_value = INT64_MAX;
89+
int64_t m_min_value = std::numeric_limits<int64_t>::min();
90+
int64_t m_max_value = std::numeric_limits<int64_t>::max();
9191
};
9292

9393
} // namespace lldb_private

lldb/include/lldb/Interpreter/OptionValueUInt64.h

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,35 @@ class OptionValueUInt64 : public Cloneable<OptionValueUInt64, OptionValue> {
6464

6565
uint64_t GetDefaultValue() const { return m_default_value; }
6666

67-
void SetCurrentValue(uint64_t value) { m_current_value = value; }
67+
bool SetCurrentValue(uint64_t value) {
68+
if (value >= m_min_value && value <= m_max_value) {
69+
m_current_value = value;
70+
return true;
71+
}
72+
return false;
73+
}
74+
75+
bool SetDefaultValue(uint64_t value) {
76+
if (value >= m_min_value && value <= m_max_value) {
77+
m_default_value = value;
78+
return true;
79+
}
80+
return false;
81+
}
82+
83+
void SetMinimumValue(int64_t v) { m_min_value = v; }
84+
85+
uint64_t GetMinimumValue() const { return m_min_value; }
86+
87+
void SetMaximumValue(int64_t v) { m_max_value = v; }
6888

69-
void SetDefaultValue(uint64_t value) { m_default_value = value; }
89+
uint64_t GetMaximumValue() const { return m_max_value; }
7090

7191
protected:
7292
uint64_t m_current_value = 0;
7393
uint64_t m_default_value = 0;
94+
uint64_t m_min_value = std::numeric_limits<uint64_t>::min();
95+
uint64_t m_max_value = std::numeric_limits<uint64_t>::max();
7496
};
7597

7698
} // namespace lldb_private

lldb/source/Core/CoreProperties.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ let Definition = "debugger" in {
132132
Global,
133133
DefaultStringValue<"${ansi.normal}">,
134134
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.">;
135-
def TerminalWidth: Property<"term-width", "SInt64">,
135+
def TerminalWidth: Property<"term-width", "UInt64">,
136136
Global,
137137
DefaultUnsignedValue<80>,
138138
Desc<"The maximum number of columns to use for displaying text.">;

lldb/source/Core/Debugger.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -886,8 +886,8 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, void *baton)
886886
}
887887
assert(m_dummy_target_sp.get() && "Couldn't construct dummy target?");
888888

889-
OptionValueSInt64 *term_width =
890-
m_collection_sp->GetPropertyAtIndexAsOptionValueSInt64(
889+
OptionValueUInt64 *term_width =
890+
m_collection_sp->GetPropertyAtIndexAsOptionValueUInt64(
891891
ePropertyTerminalWidth);
892892
term_width->SetMinimumValue(10);
893893
term_width->SetMaximumValue(1024);

lldb/source/Interpreter/OptionValueUInt64.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,16 @@ Status OptionValueUInt64::SetValueFromString(llvm::StringRef value_ref,
4747
llvm::StringRef value_trimmed = value_ref.trim();
4848
uint64_t value;
4949
if (llvm::to_integer(value_trimmed, value)) {
50-
m_value_was_set = true;
51-
m_current_value = value;
52-
NotifyValueChanged();
50+
if (value >= m_min_value && value <= m_max_value) {
51+
m_value_was_set = true;
52+
m_current_value = value;
53+
NotifyValueChanged();
54+
} else {
55+
error.SetErrorStringWithFormat(
56+
"%" PRIu64 " is out of range, valid values must be between %" PRIu64
57+
" and %" PRIu64 ".",
58+
value, m_min_value, m_max_value);
59+
}
5360
} else {
5461
error.SetErrorStringWithFormat("invalid uint64_t string value: '%s'",
5562
value_ref.str().c_str());

lldb/test/API/commands/settings/TestSettings.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
Test lldb settings command.
33
"""
44

5-
65
import json
76
import os
87
import re
@@ -151,14 +150,22 @@ def test_set_term_width(self):
151150
self.expect(
152151
"settings show term-width",
153152
SETTING_MSG("term-width"),
154-
startstr="term-width (int) = 70",
153+
startstr="term-width (unsigned) = 70",
155154
)
156155

157156
# The overall display should also reflect the new setting.
158157
self.expect(
159158
"settings show",
160159
SETTING_MSG("term-width"),
161-
substrs=["term-width (int) = 70"],
160+
substrs=["term-width (unsigned) = 70"],
161+
)
162+
163+
self.dbg.SetTerminalWidth(60)
164+
165+
self.expect(
166+
"settings show",
167+
SETTING_MSG("term-width"),
168+
substrs=["term-width (unsigned) = 60"],
162169
)
163170

164171
# rdar://problem/10712130
@@ -593,7 +600,7 @@ def test_settings_with_trailing_whitespace(self):
593600
self.expect(
594601
"settings show term-width",
595602
SETTING_MSG("term-width"),
596-
startstr="term-width (int) = 60",
603+
startstr="term-width (unsigned) = 60",
597604
)
598605
self.runCmd("settings clear term-width", check=False)
599606
# string

lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ def do_test(self, term_width, pattern_list):
2424
)
2525
self.expect("set set term-width " + str(term_width))
2626
self.expect(
27-
"set show term-width", substrs=["term-width (int) = " + str(term_width)]
27+
"set show term-width",
28+
substrs=["term-width (unsigned) = " + str(term_width)],
2829
)
2930

3031
self.child.send("file " + self.getBuildArtifact("a.out") + "\n")

0 commit comments

Comments
 (0)