[lldb] Add flag to "settings show" to include default values #153233
Conversation
kastiglione
requested review from
DavidSpickett,
labath,
medismailben,
Michael137 and
jimingham
|
Draft mode because the PR initially has no tests. Before writing tests, I wanted to get feedback on the implementation specifics. |
|
|
||
| if (dump_mask & eDumpOptionDefaultValue && | ||
| m_current_value != m_default_value && m_default_value.IsValid()) { | ||
| DefaultValueFormat label{strm}; |
There was a problem hiding this comment.
I know some folks prefer braced initializers but it's not common in LLVM or LLDB except for initializer lists, so let's stick to () for consistency.
| DefaultValueFormat label{strm}; | |
| DefaultValueFormat label(strm); |
There was a problem hiding this comment.
I'll change it but this makes me wonder: isn't it more modern and generally a preferred syntax? are there problems that result in the lldb codebase sticking to parenthetical initialization?
There was a problem hiding this comment.
It's mostly about the Golden Rule:
If you are extending, enhancing, or bug fixing already implemented code, use the style that is already being used so that the source is uniform and easy to follow.
Cynically, one could interpret that as nothing can change, but hasn't been the case. We have made a bunch of code style changes when they offered sufficient value to "outweigh" the value brought by consistency.
For braced initializes specifically, it turns out that we actually already have an entry in the style guide: https://llvm.org/docs/CodingStandards.html#do-not-use-braced-initializer-lists-to-call-a-constructor
There was a problem hiding this comment.
good to know it's documented, but too bad there's no phabricator link when it was added in 2014 so no way of learning what its reasons were.
There was a problem hiding this comment.
I know some folks prefer braced initializers but it's not common in LLVM or LLDB except for initializer lists
Flang is a fanatic for braced init (https://flang.llvm.org/docs/C%2B%2Bstyle.html#c-language) but it is very much an outlier in llvm-project.
| // DeepCopy to it. Inherit from Cloneable to avoid doing this manually. | ||
| virtual lldb::OptionValueSP Clone() const = 0; | ||
|
|
||
| struct DefaultValueFormat { |
| return lhs.GetCore() == rhs.GetCore(); | ||
| } | ||
|
|
||
| bool lldb_private::operator!=(const ArchSpec &lhs, const ArchSpec &rhs) { |
There was a problem hiding this comment.
I thought this would be compiler generated if the opposite one was implemented. Is that not the case ?
There was a problem hiding this comment.
it seems not until c++20 https://en.cppreference.com/w/cpp/language/default_comparisons.html
There was a problem hiding this comment.
my search skills produced the wrong link, ignore.
There was a problem hiding this comment.
I'm not sure what the compiler is supposed to do, but without declaring and implementing operator!=, the compiler emits:
OptionValueArch.cpp:36:25: error: invalid operands to binary expression ('ArchSpec' and 'ArchSpec')
36 | m_current_value != m_default_value && m_default_value.IsValid()) {
| ~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~
There was a problem hiding this comment.
The link is wrong, but the statement is correct. You need to define operators individually until c++20, and even then, the autogeneration only works when you define the "spaceship operator".
| ExecutionContext *execution_context) override { | ||
| const int short_option = m_getopt_table[option_idx].val; | ||
| switch (short_option) { | ||
| case 'v': |
There was a problem hiding this comment.
This is a tiny quibble, but unless we have other things in mind to include in a verbose listing, I'd suggest --show-default and -d rather than --verbose since that way we say exactly what we're doing. I can't think of anything else we'd include here, so --show-defaults seems better to me.
There was a problem hiding this comment.
And even if we do add extra stuff, seeing what the defaults are on their own seems like a useful enough action I expect people would want to dial it up separately.
There was a problem hiding this comment.
what do you think of --include-defaults since "show" might imply that a default will be printed for every setting, but that's not what the PR currently does. Or are you also suggesting that this should be changed to show a default, even for cases where for example a setting type doesn't have a default value (array, dict, etc).
There was a problem hiding this comment.
If we were to do separate displays for "defaults", "values" and "values with defaults" then show would be appropriate. But I think it's fine not to do that. As you said below, you can run w/o an .lldbinit in command line to figure that out, and in Xcode you could do the same thing by making an empty target-specific lldb init file. Presumably there's a way to do this in VSCode, but even if there weren't this is a special-purpose enough task having to do it with -x seems acceptable.
In that case, --include-defaults seems more accurate.
|
Do you think people are likely sometimes to want to see ONLY the default values? Showing the settings can get pretty verbose, so if you were trying to see "how lldb looks w/o any user intervention" not having to see two values and figure out when having only one means the default is that, and sometimes it's the second one might make that easier. |
@jimingham In that case I've used |
kastiglione
changed the title
lldb/source/Commands/Options.td
Outdated
| } | ||
|
|
||
| let Command = "settings show" in { | ||
| def setshow_include_defaults : Option<"include-defaults", "d">, |
There was a problem hiding this comment.
How about show-defaults? We have have 6 flags that start with show- and only one that starts with include- (process attach --include-existing).
There was a problem hiding this comment.
Jim originally suggested --show-defaults. I think "show" is shorter and cleaner, but I also think implies that the output will display a default value for every setting – however that's not what this change does. With that context, what are your thoughts?
There was a problem hiding this comment.
I don't think "show" and "include" are meaningfully different, even with that context, but I'm also not a native speaker. Go with whatever feels right to you 👍
There was a problem hiding this comment.
thought: since the subcommand is already show, does it make sense to have the flag be simply --defaults?
show --show-defaults
vs
show --defaults
There was a problem hiding this comment.
changed to settings show --defaults.
There was a problem hiding this comment.
Makes sense to me. I'm wondering whether the two reasons for omitting the default value couldn't lead to confusion. Like, is the default value omitted because the current value is the default or because it's type does not support default values (which I guess means that the default values is empty/default constructed?).
However, that's just something to consider, I'm not insisting on a particular resolution to that.
| std::string escaped; | ||
| EscapeBackticks(m_current_format, escaped); | ||
| strm << '"' << escaped << '"'; | ||
| strm.QuotedCString(EscapeBackticks(m_current_format).data()); |
There was a problem hiding this comment.
use c_str() here. .data() makes is look like you're calling StringRef::data, which would be a mistake.
(Personally, I'd say that the original version (I'm referring to <<, not to by-ref returns) is better).
There was a problem hiding this comment.
reverted to the << style
| return lhs.GetCore() == rhs.GetCore(); | ||
| } | ||
|
|
||
| bool lldb_private::operator!=(const ArchSpec &lhs, const ArchSpec &rhs) { |
There was a problem hiding this comment.
The link is wrong, but the statement is correct. You need to define operators individually until c++20, and even then, the autogeneration only works when you define the "spaceship operator".
| stream.PutCString(" (default: "); | ||
| } | ||
| ~DefaultValueFormat() { stream.PutChar(')'); } | ||
| Stream &stream; |
There was a problem hiding this comment.
Fancy, but then I'd suggest making this a proper class (making the stream private, and the object non-copyable).
@labath I agree with your point about confusion. One of my motivations was to not produce a bunch of noise in the output. However, checking my local lldb (Xcode 26), only 21 of the 234 settings are of a type which does not support default values. Now that I'm aware noise from those types isn't actually a problem, I will update this change to include |
|
@labath Now it's a question of formatting. Most of the types that don't have a default value are collections (arrays, dicts, list), and are printed out on multiple lines (even when there's only a single entry). For a multiline value, where should the before: after: |
|
I'd probably go with the first version. It doesn't leave a strange hanging default label, and it saves a line. It true that other settings show the default value after the actual one, but these settings are kind of special already (in that they print on multiple lines), so I think the consistency isn't that important. Instead of "none", could we say "default: empty" ? As a user, I'm not sure how I would interpret "none". Regardless of how it's implemented internally, "empty list/dict/whatever" is the value of the setting you get if you don't change it, is it not? |
|
I agree with Pavel that before is easier to read. Another way to indicate that the aggregate values are empty is to print the empty version of however we allow you to specify elements, so in your case: |
|
Updated. Of note:
|
|
@llvm/pr-subscribers-lldb Author: Dave Lee (kastiglione) ChangesAdds a A default value is not shown when the current value is the default. Note: some setting types do not print empty or invalid values. For these setting types, if the default value is empty or invalid, the same elision logic is applied to printing the default value. Patch is 30.93 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/153233.diff 23 Files Affected:
diff --git a/lldb/include/lldb/Interpreter/OptionValue.h b/lldb/include/lldb/Interpreter/OptionValue.h
index f293a3a33bfa0..9c992821251cb 100644
--- a/lldb/include/lldb/Interpreter/OptionValue.h
+++ b/lldb/include/lldb/Interpreter/OptionValue.h
@@ -17,6 +17,7 @@
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/FileSpecList.h"
#include "lldb/Utility/Status.h"
+#include "lldb/Utility/Stream.h"
#include "lldb/Utility/StringList.h"
#include "lldb/Utility/UUID.h"
#include "lldb/lldb-defines.h"
@@ -61,6 +62,7 @@ class OptionValue {
eDumpOptionDescription = (1u << 3),
eDumpOptionRaw = (1u << 4),
eDumpOptionCommand = (1u << 5),
+ eDumpOptionDefaultValue = (1u << 6),
eDumpGroupValue = (eDumpOptionName | eDumpOptionType | eDumpOptionValue),
eDumpGroupHelp =
(eDumpOptionName | eDumpOptionType | eDumpOptionDescription),
@@ -338,6 +340,20 @@ class OptionValue {
// DeepCopy to it. Inherit from Cloneable to avoid doing this manually.
virtual lldb::OptionValueSP Clone() const = 0;
+ class DefaultValueFormat {
+ public:
+ DefaultValueFormat(Stream &stream) : stream(stream) {
+ stream.PutCString(" (default: ");
+ }
+ ~DefaultValueFormat() { stream.PutChar(')'); }
+
+ DefaultValueFormat(const DefaultValueFormat &) = delete;
+ DefaultValueFormat &operator=(const DefaultValueFormat &) = delete;
+
+ private:
+ Stream &stream;
+ };
+
lldb::OptionValueWP m_parent_wp;
std::function<void()> m_callback;
bool m_value_was_set = false; // This can be used to see if a value has been
diff --git a/lldb/include/lldb/Interpreter/OptionValueEnumeration.h b/lldb/include/lldb/Interpreter/OptionValueEnumeration.h
index a3a13ca7b15eb..91ab454b2065e 100644
--- a/lldb/include/lldb/Interpreter/OptionValueEnumeration.h
+++ b/lldb/include/lldb/Interpreter/OptionValueEnumeration.h
@@ -72,6 +72,7 @@ class OptionValueEnumeration
protected:
void SetEnumerations(const OptionEnumValues &enumerators);
+ void DumpEnum(Stream &strm, enum_type value);
enum_type m_current_value;
enum_type m_default_value;
diff --git a/lldb/include/lldb/Utility/ArchSpec.h b/lldb/include/lldb/Utility/ArchSpec.h
index 7e9bc23a75acb..96bd5e3597b68 100644
--- a/lldb/include/lldb/Utility/ArchSpec.h
+++ b/lldb/include/lldb/Utility/ArchSpec.h
@@ -564,6 +564,7 @@ class ArchSpec {
/// \return true if \a lhs is less than \a rhs
bool operator<(const ArchSpec &lhs, const ArchSpec &rhs);
bool operator==(const ArchSpec &lhs, const ArchSpec &rhs);
+bool operator!=(const ArchSpec &lhs, const ArchSpec &rhs);
bool ParseMachCPUDashSubtypeTriple(llvm::StringRef triple_str, ArchSpec &arch);
diff --git a/lldb/source/Commands/CommandObjectSettings.cpp b/lldb/source/Commands/CommandObjectSettings.cpp
index 7bbb0dd567ab1..126f57c738115 100644
--- a/lldb/source/Commands/CommandObjectSettings.cpp
+++ b/lldb/source/Commands/CommandObjectSettings.cpp
@@ -237,28 +237,62 @@ insert-before or insert-after.");
};
// CommandObjectSettingsShow -- Show current values
+#define LLDB_OPTIONS_settings_show
+#include "CommandOptions.inc"
class CommandObjectSettingsShow : public CommandObjectParsed {
public:
CommandObjectSettingsShow(CommandInterpreter &interpreter)
: CommandObjectParsed(interpreter, "settings show",
"Show matching debugger settings and their current "
- "values. Defaults to showing all settings.",
- nullptr) {
+ "values. Defaults to showing all settings.") {
AddSimpleArgumentList(eArgTypeSettingVariableName, eArgRepeatOptional);
}
~CommandObjectSettingsShow() override = default;
+ Options *GetOptions() override { return &m_options; }
+
+ class CommandOptions : public Options {
+ public:
+ ~CommandOptions() override = default;
+
+ Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+ ExecutionContext *execution_context) override {
+ const int short_option = m_getopt_table[option_idx].val;
+ switch (short_option) {
+ case 'd':
+ m_include_defaults = true;
+ break;
+ default:
+ llvm_unreachable("Unimplemented option");
+ }
+ return {};
+ }
+
+ void OptionParsingStarting(ExecutionContext *execution_context) override {
+ m_include_defaults = false;
+ }
+
+ llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
+ return g_settings_show_options;
+ }
+
+ bool m_include_defaults = false;
+ };
+
protected:
void DoExecute(Args &args, CommandReturnObject &result) override {
result.SetStatus(eReturnStatusSuccessFinishResult);
+ uint32_t dump_mask = OptionValue::eDumpGroupValue;
+ if (m_options.m_include_defaults)
+ dump_mask |= OptionValue::eDumpOptionDefaultValue;
+
if (!args.empty()) {
for (const auto &arg : args) {
Status error(GetDebugger().DumpPropertyValue(
- &m_exe_ctx, result.GetOutputStream(), arg.ref(),
- OptionValue::eDumpGroupValue));
+ &m_exe_ctx, result.GetOutputStream(), arg.ref(), dump_mask));
if (error.Success()) {
result.GetOutputStream().EOL();
} else {
@@ -267,9 +301,12 @@ class CommandObjectSettingsShow : public CommandObjectParsed {
}
} else {
GetDebugger().DumpAllPropertyValues(&m_exe_ctx, result.GetOutputStream(),
- OptionValue::eDumpGroupValue);
+ dump_mask);
}
}
+
+private:
+ CommandOptions m_options;
};
// CommandObjectSettingsWrite -- Write settings to file
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 61acc40585976..17d72cdcad129 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -56,6 +56,11 @@ let Command = "settings clear" in {
Desc<"Clear all settings.">;
}
+let Command = "settings show" in {
+ def setshow_defaults : Option<"defaults", "d">,
+ Desc<"Include default values if defined.">;
+}
+
let Command = "breakpoint list" in {
// FIXME: We need to add an "internal" command, and then add this sort of
// thing to it. But I need to see it for now, and don't want to wait.
diff --git a/lldb/source/Interpreter/OptionValueArch.cpp b/lldb/source/Interpreter/OptionValueArch.cpp
index ea15ccaaefe2b..a960e39d35a62 100644
--- a/lldb/source/Interpreter/OptionValueArch.cpp
+++ b/lldb/source/Interpreter/OptionValueArch.cpp
@@ -11,6 +11,7 @@
#include "lldb/DataFormatters/FormatManager.h"
#include "lldb/Interpreter/CommandCompletions.h"
#include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Interpreter/OptionValue.h"
#include "lldb/Utility/Args.h"
#include "lldb/Utility/State.h"
@@ -30,6 +31,12 @@ void OptionValueArch::DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
if (arch_name)
strm.PutCString(arch_name);
}
+
+ if (dump_mask & eDumpOptionDefaultValue &&
+ m_current_value != m_default_value && m_default_value.IsValid()) {
+ DefaultValueFormat label(strm);
+ strm.PutCString(m_default_value.GetArchitectureName());
+ }
}
}
diff --git a/lldb/source/Interpreter/OptionValueArray.cpp b/lldb/source/Interpreter/OptionValueArray.cpp
index f6c14dee525e9..5600333c111a7 100644
--- a/lldb/source/Interpreter/OptionValueArray.cpp
+++ b/lldb/source/Interpreter/OptionValueArray.cpp
@@ -8,6 +8,7 @@
#include "lldb/Interpreter/OptionValueArray.h"
+#include "lldb/Interpreter/OptionValue.h"
#include "lldb/Utility/Args.h"
#include "lldb/Utility/Stream.h"
@@ -27,8 +28,15 @@ void OptionValueArray::DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
if (dump_mask & eDumpOptionValue) {
const bool one_line = dump_mask & eDumpOptionCommand;
const uint32_t size = m_values.size();
- if (dump_mask & eDumpOptionType)
- strm.Printf(" =%s", (m_values.size() > 0 && !one_line) ? "\n" : "");
+ if (dump_mask & (eDumpOptionType | eDumpOptionDefaultValue)) {
+ strm.PutCString(" =");
+ if (dump_mask & eDumpOptionDefaultValue && !m_values.empty()) {
+ DefaultValueFormat label(strm);
+ strm.PutCString("empty");
+ }
+ if (!m_values.empty() && !one_line)
+ strm.PutCString("\n");
+ }
if (!one_line)
strm.IndentMore();
for (uint32_t i = 0; i < size; ++i) {
diff --git a/lldb/source/Interpreter/OptionValueBoolean.cpp b/lldb/source/Interpreter/OptionValueBoolean.cpp
index d4fda762fea6b..023c243b3efc1 100644
--- a/lldb/source/Interpreter/OptionValueBoolean.cpp
+++ b/lldb/source/Interpreter/OptionValueBoolean.cpp
@@ -10,6 +10,7 @@
#include "lldb/Host/PosixApi.h"
#include "lldb/Interpreter/OptionArgParser.h"
+#include "lldb/Interpreter/OptionValue.h"
#include "lldb/Utility/Stream.h"
#include "lldb/Utility/StringList.h"
#include "llvm/ADT/STLExtras.h"
@@ -27,6 +28,11 @@ void OptionValueBoolean::DumpValue(const ExecutionContext *exe_ctx,
if (dump_mask & eDumpOptionType)
strm.PutCString(" = ");
strm.PutCString(m_current_value ? "true" : "false");
+ if (dump_mask & eDumpOptionDefaultValue &&
+ m_current_value != m_default_value) {
+ DefaultValueFormat label(strm);
+ strm.PutCString(m_default_value ? "true" : "false");
+ }
}
}
diff --git a/lldb/source/Interpreter/OptionValueChar.cpp b/lldb/source/Interpreter/OptionValueChar.cpp
index 2aadcff158388..595dcba49b61a 100644
--- a/lldb/source/Interpreter/OptionValueChar.cpp
+++ b/lldb/source/Interpreter/OptionValueChar.cpp
@@ -9,6 +9,7 @@
#include "lldb/Interpreter/OptionValueChar.h"
#include "lldb/Interpreter/OptionArgParser.h"
+#include "lldb/Interpreter/OptionValue.h"
#include "lldb/Utility/Stream.h"
#include "lldb/Utility/StringList.h"
#include "llvm/ADT/STLExtras.h"
@@ -16,6 +17,13 @@
using namespace lldb;
using namespace lldb_private;
+static void DumpChar(Stream &strm, char value) {
+ if (value != '\0')
+ strm.PutChar(value);
+ else
+ strm.PutCString("(null)");
+}
+
void OptionValueChar::DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
uint32_t dump_mask) {
if (dump_mask & eDumpOptionType)
@@ -24,10 +32,12 @@ void OptionValueChar::DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
if (dump_mask & eDumpOptionValue) {
if (dump_mask & eDumpOptionType)
strm.PutCString(" = ");
- if (m_current_value != '\0')
- strm.PutChar(m_current_value);
- else
- strm.PutCString("(null)");
+ DumpChar(strm, m_current_value);
+ if (dump_mask & eDumpOptionDefaultValue &&
+ m_current_value != m_default_value) {
+ DefaultValueFormat label(strm);
+ DumpChar(strm, m_default_value);
+ }
}
}
diff --git a/lldb/source/Interpreter/OptionValueDictionary.cpp b/lldb/source/Interpreter/OptionValueDictionary.cpp
index 19e21dd6f4c9a..0efc76bb7cd94 100644
--- a/lldb/source/Interpreter/OptionValueDictionary.cpp
+++ b/lldb/source/Interpreter/OptionValueDictionary.cpp
@@ -9,6 +9,7 @@
#include "lldb/Interpreter/OptionValueDictionary.h"
#include "lldb/DataFormatters/FormatManager.h"
+#include "lldb/Interpreter/OptionValue.h"
#include "lldb/Interpreter/OptionValueEnumeration.h"
#include "lldb/Interpreter/OptionValueString.h"
#include "lldb/Utility/Args.h"
@@ -30,8 +31,13 @@ void OptionValueDictionary::DumpValue(const ExecutionContext *exe_ctx,
}
if (dump_mask & eDumpOptionValue) {
const bool one_line = dump_mask & eDumpOptionCommand;
- if (dump_mask & eDumpOptionType)
+ if (dump_mask & (eDumpOptionType | eDumpOptionDefaultValue)) {
strm.PutCString(" =");
+ if (dump_mask & eDumpOptionDefaultValue && !m_values.empty()) {
+ DefaultValueFormat label(strm);
+ strm.PutCString("empty");
+ }
+ }
if (!one_line)
strm.IndentMore();
diff --git a/lldb/source/Interpreter/OptionValueEnumeration.cpp b/lldb/source/Interpreter/OptionValueEnumeration.cpp
index cf646233c80da..eb31bde498b7c 100644
--- a/lldb/source/Interpreter/OptionValueEnumeration.cpp
+++ b/lldb/source/Interpreter/OptionValueEnumeration.cpp
@@ -8,6 +8,7 @@
#include "lldb/Interpreter/OptionValueEnumeration.h"
+#include "lldb/Interpreter/OptionValue.h"
#include "lldb/Utility/StringList.h"
using namespace lldb;
@@ -19,6 +20,17 @@ OptionValueEnumeration::OptionValueEnumeration(
SetEnumerations(enumerators);
}
+void OptionValueEnumeration::DumpEnum(Stream &strm, enum_type value) {
+ const size_t count = m_enumerations.GetSize();
+ for (size_t i = 0; i < count; ++i)
+ if (m_enumerations.GetValueAtIndexUnchecked(i).value == value) {
+ strm.PutCString(m_enumerations.GetCStringAtIndex(i));
+ return;
+ }
+
+ strm.Printf("%" PRIu64, (uint64_t)value);
+}
+
void OptionValueEnumeration::DumpValue(const ExecutionContext *exe_ctx,
Stream &strm, uint32_t dump_mask) {
if (dump_mask & eDumpOptionType)
@@ -26,14 +38,12 @@ void OptionValueEnumeration::DumpValue(const ExecutionContext *exe_ctx,
if (dump_mask & eDumpOptionValue) {
if (dump_mask & eDumpOptionType)
strm.PutCString(" = ");
- const size_t count = m_enumerations.GetSize();
- for (size_t i = 0; i < count; ++i) {
- if (m_enumerations.GetValueAtIndexUnchecked(i).value == m_current_value) {
- strm.PutCString(m_enumerations.GetCStringAtIndex(i).GetStringRef());
- return;
- }
+ DumpEnum(strm, m_current_value);
+ if (dump_mask & eDumpOptionDefaultValue &&
+ m_current_value != m_default_value) {
+ DefaultValueFormat label(strm);
+ DumpEnum(strm, m_default_value);
}
- strm.Printf("%" PRIu64, (uint64_t)m_current_value);
}
}
diff --git a/lldb/source/Interpreter/OptionValueFileSpec.cpp b/lldb/source/Interpreter/OptionValueFileSpec.cpp
index 6fa6af4ace02a..8d4966dcda0bc 100644
--- a/lldb/source/Interpreter/OptionValueFileSpec.cpp
+++ b/lldb/source/Interpreter/OptionValueFileSpec.cpp
@@ -12,6 +12,7 @@
#include "lldb/Host/FileSystem.h"
#include "lldb/Interpreter/CommandCompletions.h"
#include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Interpreter/OptionValue.h"
#include "lldb/Utility/Args.h"
#include "lldb/Utility/State.h"
@@ -41,7 +42,12 @@ void OptionValueFileSpec::DumpValue(const ExecutionContext *exe_ctx,
strm.PutCString(" = ");
if (m_current_value) {
- strm << '"' << m_current_value.GetPath().c_str() << '"';
+ strm << '"' << m_current_value.GetPath() << '"';
+ }
+ if (dump_mask & eDumpOptionDefaultValue &&
+ m_current_value != m_default_value && m_default_value) {
+ DefaultValueFormat label(strm);
+ strm << '"' << m_default_value.GetPath() << '"';
}
}
}
diff --git a/lldb/source/Interpreter/OptionValueFileSpecList.cpp b/lldb/source/Interpreter/OptionValueFileSpecList.cpp
index f252dc4603cc1..f331c5d13f461 100644
--- a/lldb/source/Interpreter/OptionValueFileSpecList.cpp
+++ b/lldb/source/Interpreter/OptionValueFileSpecList.cpp
@@ -8,6 +8,7 @@
#include "lldb/Interpreter/OptionValueFileSpecList.h"
+#include "lldb/Interpreter/OptionValue.h"
#include "lldb/Utility/Args.h"
#include "lldb/Utility/Stream.h"
@@ -22,9 +23,15 @@ void OptionValueFileSpecList::DumpValue(const ExecutionContext *exe_ctx,
if (dump_mask & eDumpOptionValue) {
const bool one_line = dump_mask & eDumpOptionCommand;
const uint32_t size = m_current_value.GetSize();
- if (dump_mask & eDumpOptionType)
- strm.Printf(" =%s",
- (m_current_value.GetSize() > 0 && !one_line) ? "\n" : "");
+ if (dump_mask & (eDumpOptionType | eDumpOptionDefaultValue)) {
+ strm.Printf(" =");
+ if (dump_mask & eDumpOptionDefaultValue && !m_current_value.IsEmpty()) {
+ DefaultValueFormat label(strm);
+ strm.PutCString("empty");
+ }
+ if (!m_current_value.IsEmpty() && !one_line)
+ strm.PutCString("\n");
+ }
if (!one_line)
strm.IndentMore();
for (uint32_t i = 0; i < size; ++i) {
diff --git a/lldb/source/Interpreter/OptionValueFormat.cpp b/lldb/source/Interpreter/OptionValueFormat.cpp
index bc4e77923d10e..05990fb50c356 100644
--- a/lldb/source/Interpreter/OptionValueFormat.cpp
+++ b/lldb/source/Interpreter/OptionValueFormat.cpp
@@ -10,6 +10,7 @@
#include "lldb/DataFormatters/FormatManager.h"
#include "lldb/Interpreter/OptionArgParser.h"
+#include "lldb/Interpreter/OptionValue.h"
#include "lldb/Utility/Stream.h"
using namespace lldb;
@@ -23,6 +24,11 @@ void OptionValueFormat::DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
if (dump_mask & eDumpOptionType)
strm.PutCString(" = ");
strm.PutCString(FormatManager::GetFormatAsCString(m_current_value));
+ if (dump_mask & eDumpOptionDefaultValue &&
+ m_current_value != m_default_value) {
+ DefaultValueFormat label(strm);
+ strm.PutCString(FormatManager::GetFormatAsCString(m_default_value));
+ }
}
}
diff --git a/lldb/source/Interpreter/OptionValueFormatEntity.cpp b/lldb/source/Interpreter/OptionValueFormatEntity.cpp
index d8b830115005c..b31dd4e475878 100644
--- a/lldb/source/Interpreter/OptionValueFormatEntity.cpp
+++ b/lldb/source/Interpreter/OptionValueFormatEntity.cpp
@@ -10,6 +10,7 @@
#include "lldb/Core/Module.h"
#include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Interpreter/OptionValue.h"
#include "lldb/Utility/Stream.h"
#include "lldb/Utility/StringList.h"
using namespace lldb;
@@ -33,10 +34,9 @@ void OptionValueFormatEntity::Clear() {
m_value_was_set = false;
}
-static void EscapeBackticks(llvm::StringRef str, std::string &dst) {
- dst.clear();
+static std::string EscapeBackticks(llvm::StringRef str) {
+ std::string dst;
dst.reserve(str.size());
-
for (size_t i = 0, e = str.size(); i != e; ++i) {
char c = str[i];
if (c == '`') {
@@ -45,6 +45,7 @@ static void EscapeBackticks(llvm::StringRef str, std::string &dst) {
}
dst += c;
}
+ return dst;
}
void OptionValueFormatEntity::DumpValue(const ExecutionContext *exe_ctx,
@@ -54,17 +55,18 @@ void OptionValueFormatEntity::DumpValue(const ExecutionContext *exe_ctx,
if (dump_mask & eDumpOptionValue) {
if (dump_mask & eDumpOptionType)
strm.PutCString(" = ");
- std::string escaped;
- EscapeBackticks(m_current_format, escaped);
- strm << '"' << escaped << '"';
+ strm << '"' << EscapeBackticks(m_current_format) << '"';
+ if (dump_mask & eDumpOptionDefaultValue &&
+ m_current_format != m_default_format) {
+ DefaultValueFormat label(strm);
+ strm << '"' << EscapeBackticks(m_default_format) << '"';
+ }
}
}
llvm::json::Value
OptionValueFormatEntity::ToJSON(const ExecutionContext *exe_ctx) const {
- std::string escaped;
- EscapeBackticks(m_current_format, escaped);
- return escaped;
+ return EscapeBackticks(m_current_format);
}
Status OptionValueFormatEntity::SetValueFromString(llvm::StringRef value_str,
diff --git a/lldb/source/Interpreter/OptionValueLanguage.cpp b/lldb/source/Interpreter/OptionValueLanguage.cpp
index 0fdaacb02594e..bb28dc8debe6d 100644
--- a/lldb/source/Interpreter/OptionValueLanguage.cpp
+++ b/lldb/source/Interpreter/OptionValueLanguage.cpp
@@ -9,8 +9,9 @@
#include "lldb/Interpreter/OptionValueLanguage.h"
#include "lldb/DataFormatters/FormatManager.h"
-#include "lldb/Target/Language.h"
+#include "lldb/Interpreter/OptionValue.h"
#include "lldb/Symbol/TypeSystem.h"
+#include "lldb/Target/Language.h"
#include "lldb/Utility/Args.h"
#include "lldb/Utility/Stream.h"
@@ -26,6 +27,12 @@ void OptionValueLanguage::DumpValue(const ExecutionContext *exe_ctx,
strm.PutCString(" = ");
if (m_current_value != eLanguageTypeUnknown)
strm.PutCString(Language::GetNameForLanguageType(m_current_value));
+ if (dump_mask & eDumpOptionDefaultValue &&
+ m_current_value != m_default_value &&
+ m_default_value != eLanguageTypeUnknown) {
+ DefaultValueFormat label(strm);
+ strm.PutCString(Language::GetNameForLanguageType(m_default_value));
+ }
}
}
diff --git a/lldb/source/Interpreter/OptionValuePathMappings.cpp b/lldb/source/Interpreter/OptionValuePathMappings.cpp
index 95b8e64171455..abf4d429602e4 100644
--- a/lldb/source/Interpreter/OptionValuePathMappings.cpp
+++ b/lldb/source/Interpreter/OptionValuePathMappings.cpp
@@ -9,6 +9,7 @@
...
[truncated]
|
|
If there's no more feedback, I'll merge this soon. |
Fixes a few test failures on windows. See #153233
Fixes a few test failures on windows. See llvm/llvm-project#153233
|
The following buildbots are broken after this patch lldb-x86_64-win Please fix ASAP or revert to make them green. |
|
@slydiman I received emails yesterday and fixed the failures, but there's another failure that wasn't initially reported. I'll fix that now. |
Adds a
--defaults/-dflag tosettings show. This mode will optionally show a setting's default value. In other words, this does not always print a default value for every setting.A default value is not shown when the current value is the default.
Note: some setting types do not print empty or invalid values. For these setting types, if the default value is empty or invalid, the same elision logic is applied to printing the default value.