[debugger] Improve function formatting.
Makes function naming a little bit more predictable. The normal name of
the function no longer includes the parameters. Previously we always
appended "()" regardless of whether there were parameters or not, which
was misleading.
If we need "full function name with parameter types" we can add a new
helper function to the Function class in the future.
Changes the default function name printing to show an elided parameter
list in the default case (previously it picked up "()" from the function
name as described above). It only shows "()" if there are really no
arguments. Flags are added to "frame" and "backtrace" that expand these
lists for cases where functions are overloaded and the parameters are
important.
Adds syntax highlighting for function names in locations, notable stack
traces. This makes the final function name bold and all template
parameters dim. This helps a lot reading the output from complicated
template functions.
Adds some unit testing ability to the OutputBuffer so we can test code
that does formatting. This is used by the new function formatting tests.
TEST=unit, manual
Change-Id: Ib09512ee5a00f30c4c49c1ccdff89ef4c5bceda8
diff --git a/bin/zxdb/console/command_utils.cc b/bin/zxdb/console/command_utils.cc
index dc64c0c..351c9eb 100644
--- a/bin/zxdb/console/command_utils.cc
+++ b/bin/zxdb/console/command_utils.cc
@@ -20,9 +20,11 @@
#include "garnet/bin/zxdb/console/console_context.h"
#include "garnet/bin/zxdb/console/output_buffer.h"
#include "garnet/bin/zxdb/console/string_util.h"
+#include "garnet/bin/zxdb/expr/identifier.h"
#include "garnet/bin/zxdb/symbols/function.h"
#include "garnet/bin/zxdb/symbols/location.h"
#include "garnet/bin/zxdb/symbols/symbol_utils.h"
+#include "garnet/bin/zxdb/symbols/variable.h"
#include "lib/fxl/logging.h"
#include "lib/fxl/strings/string_printf.h"
#include "lib/fxl/strings/trim.h"
@@ -132,7 +134,8 @@
return Err(ErrType::kInput, "Expecting number after \"0x\".");
for (size_t i = hex_after_prefix; i < trimmed.size(); i++) {
if (!isxdigit(trimmed[i]))
- return Err(ErrType::kInput, "Invalid hex number: + \"" + trimmed + "\".");
+ return Err(ErrType::kInput,
+ "Invalid hex number: + \"" + trimmed + "\".");
}
*out = strtoull(trimmed.c_str(), nullptr, 16);
} else {
@@ -268,9 +271,8 @@
context->IdForTarget(settings.scope_target));
case BreakpointSettings::Scope::kThread:
return fxl::StringPrintf(
- "pr %d t %d",
- context->IdForTarget(
- settings.scope_thread->GetProcess()->GetTarget()),
+ "pr %d t %d", context->IdForTarget(
+ settings.scope_thread->GetProcess()->GetTarget()),
context->IdForThread(settings.scope_thread));
}
FXL_NOTREACHED();
@@ -424,38 +426,107 @@
return std::string();
}
-std::string DescribeLocation(const Location& loc, bool always_show_address) {
- if (!loc.is_valid())
- return "<invalid address>";
- if (!loc.has_symbols())
- return fxl::StringPrintf("0x%" PRIx64, loc.address());
+OutputBuffer FormatIdentifier(const std::string& str, bool bold_last) {
+ const auto & [ err, identifier ] = Identifier::FromString(str);
+ if (err.has_error()) {
+ // Not parseable as an identnfier, just write the string.
+ return OutputBuffer(str);
+ }
- std::string result;
- if (always_show_address)
- result = fxl::StringPrintf("0x%" PRIx64 ", ", loc.address());
+ OutputBuffer result;
+
+ const auto& comps = identifier.components();
+ for (size_t i = 0; i < comps.size(); i++) {
+ const auto& comp = comps[i];
+ if (comp.has_separator())
+ result.Append("::");
+
+ // Name.
+ if (bold_last && i == comps.size() - 1)
+ result.Append(Syntax::kHeading, comp.name().value());
+ else
+ result.Append(Syntax::kNormal, comp.name().value());
+
+ // Template.
+ if (comp.has_template()) {
+ std::string t_string;
+ t_string += comp.template_begin().value();
+
+ for (size_t t_i = 0; t_i < comp.template_contents().size(); t_i++) {
+ if (t_i > 0)
+ t_string += ", ";
+ t_string += comp.template_contents()[t_i];
+ }
+
+ t_string += comp.template_end().value();
+ result.Append(Syntax::kComment, std::move(t_string));
+ }
+ }
+
+ return result;
+}
+
+OutputBuffer FormatFunctionName(const Function* function, bool show_params) {
+ OutputBuffer result = FormatIdentifier(function->GetFullName(), true);
+
+ const auto& params = function->parameters();
+ std::string params_str;
+ if (show_params) {
+ params_str.push_back('(');
+ for (size_t i = 0; i < params.size(); i++) {
+ if (i > 0)
+ params_str += ", ";
+ if (const Variable* var = params[i].Get()->AsVariable())
+ params_str += var->type().Get()->GetFullName();
+ }
+ params_str.push_back(')');
+ } else {
+ if (params.empty())
+ params_str += "()";
+ else
+ params_str += "(…)";
+ }
+
+ result.Append(Syntax::kComment, std::move(params_str));
+ return result;
+}
+
+OutputBuffer FormatLocation(const Location& loc, bool always_show_address,
+ bool always_show_types) {
+ if (!loc.is_valid())
+ return OutputBuffer("<invalid address>");
+ if (!loc.has_symbols())
+ return OutputBuffer(fxl::StringPrintf("0x%" PRIx64, loc.address()));
+
+ OutputBuffer result;
+ if (always_show_address) {
+ result = OutputBuffer(Syntax::kComment,
+ fxl::StringPrintf("0x%" PRIx64 ", ", loc.address()));
+ }
const Function* func = loc.symbol().Get()->AsFunction();
if (func) {
- const std::string& func_name = func->GetFullName();
- if (!func_name.empty()) {
- result += func_name;
+ OutputBuffer func_output = FormatFunctionName(func, always_show_types);
+ if (!func_output.empty()) {
+ result.Append(std::move(func_output));
if (loc.file_line().is_valid()) {
// Separator between function and file/line.
- result += " " + GetBullet() + " ";
+ result.Append(" " + GetBullet() + " ");
} else {
// Check if the address is inside a function and show the offset.
AddressRange function_range = func->GetFullRange(loc.symbol_context());
if (function_range.InRange(loc.address())) {
// Inside a function but no file/line known. Show the offset.
- result += fxl::StringPrintf(" + 0x%" PRIx64 " (no line info)",
- loc.address() - function_range.begin());
+ result.Append(fxl::StringPrintf(" + 0x%" PRIx64,
+ loc.address() - function_range.begin()));
+ result.Append(Syntax::kComment, " (no line info)");
}
}
}
}
if (loc.file_line().is_valid())
- result += DescribeFileLine(loc.file_line());
+ result.Append(DescribeFileLine(loc.file_line()));
return result;
}
@@ -481,7 +552,8 @@
return result;
}
-Err SetElementsToAdd(const std::vector<std::string>& args, AssignType* assign_type,
+Err SetElementsToAdd(const std::vector<std::string>& args,
+ AssignType* assign_type,
std::vector<std::string>* elements_to_set) {
if (args.size() < 2u)
return Err("Expected at least two arguments.");
diff --git a/bin/zxdb/console/command_utils.h b/bin/zxdb/console/command_utils.h
index e0a3188..0caf737 100644
--- a/bin/zxdb/console/command_utils.h
+++ b/bin/zxdb/console/command_utils.h
@@ -22,6 +22,7 @@
class ConsoleContext;
class Err;
class Frame;
+class Function;
struct InputLocation;
class Location;
class Thread;
@@ -101,7 +102,24 @@
const Breakpoint* breakpoint);
std::string DescribeInputLocation(const InputLocation& location);
-std::string DescribeLocation(const Location& loc, bool always_show_address);
+
+// Formats the given string as an identifier, with any template annotations
+// dimmed. If bold_last is set, the last identifier component will be bolded.
+OutputBuffer FormatIdentifier(const std::string& str, bool bold_last);
+
+// Formats the function name with syntax highlighting. If show_params is true,
+// the types of the function parameters will be output. Otherwise the
+// function name will end with "()" if there are no parameters, or "(...)" if
+// there are some. The goal is to be as short as possible without being
+// misleading (showing "()" when there are parameters may be misleading, and
+// no parens at all don't look like a function).
+OutputBuffer FormatFunctionName(const Function* function, bool show_params);
+
+// Formats the location. Normally if a function name is present the code
+// address will be omitted, but always_show_address will override this.
+OutputBuffer FormatLocation(const Location& loc,
+ bool always_show_address,
+ bool show_params);
// If show_path is set, the path to the file will be included, otherwise only
// the last file component will be printed.
diff --git a/bin/zxdb/console/command_utils_unittest.cc b/bin/zxdb/console/command_utils_unittest.cc
index 24cc37e..d35cdf9 100644
--- a/bin/zxdb/console/command_utils_unittest.cc
+++ b/bin/zxdb/console/command_utils_unittest.cc
@@ -11,8 +11,12 @@
#include "garnet/bin/zxdb/common/err.h"
#include "garnet/bin/zxdb/console/command.h"
#include "garnet/bin/zxdb/console/output_buffer.h"
+#include "garnet/bin/zxdb/symbols/base_type.h"
#include "garnet/bin/zxdb/symbols/function.h"
#include "garnet/bin/zxdb/symbols/location.h"
+#include "garnet/bin/zxdb/symbols/namespace.h"
+#include "garnet/bin/zxdb/symbols/type_test_support.h"
+#include "garnet/bin/zxdb/symbols/variable_test_support.h"
#include "gtest/gtest.h"
#include "lib/fxl/strings/string_printf.h"
@@ -178,43 +182,113 @@
EXPECT_TRUE(ParseHostPort("google.com:99999999", &host, &port).has_error());
}
-TEST(CommandUtils, DescribeLocation) {
+TEST(CommandUtils, FormatIdentifier) {
+ // Obviously unparseable identifiers should just come through as strings.
+ std::string gibberish("!@(#*ASDGasdh<@");
+ EXPECT_EQ(gibberish, FormatIdentifier(gibberish, true).AsString());
+
+ // Regular name.
+ auto output = FormatIdentifier("ThisIsAName", false);
+ EXPECT_EQ("kNormal \"ThisIsAName\"", output.GetDebugString());
+
+ // Regular name with bolding.
+ output = FormatIdentifier("ThisIsAName", true);
+ EXPECT_EQ("kHeading \"ThisIsAName\"", output.GetDebugString());
+
+ // Hierarchical name.
+ output = FormatIdentifier("::Foo<int, char*>::Bar<Baz>", true);
+ EXPECT_EQ(
+ "kNormal \"::Foo\", "
+ "kComment \"<int, char*>\", "
+ "kNormal \"::\", "
+ "kHeading \"Bar\", "
+ "kComment \"<Baz>\"",
+ output.GetDebugString());
+}
+
+TEST(CommandUtils, FormatFunctionName) {
+ auto function = fxl::MakeRefCounted<Function>();
+ function->set_assigned_name("Function");
+
+ // Function with no parameters.
+ EXPECT_EQ("Function()", FormatFunctionName(function.get(), false).AsString());
+ EXPECT_EQ("Function()", FormatFunctionName(function.get(), true).AsString());
+
+ // Add two parameters.
+ auto int32_type = MakeInt32Type();
+ auto param_value = MakeVariableForTest("value", int32_type, 0x100, 0x200,
+ std::vector<uint8_t>());
+ auto param_other = MakeVariableForTest("other_param", int32_type, 0x100,
+ 0x200, std::vector<uint8_t>());
+ function->set_parameters({LazySymbol(param_value), LazySymbol(param_other)});
+
+ EXPECT_EQ("Function(…)",
+ FormatFunctionName(function.get(), false).AsString());
+ EXPECT_EQ("Function(int32_t, int32_t)",
+ FormatFunctionName(function.get(), true).AsString());
+
+ // Put in a namespace and add some templates. This needs a new function
+ // because the name will be cached above.
+ function = fxl::MakeRefCounted<Function>();
+ function->set_assigned_name("Function<int>");
+ function->set_parameters({LazySymbol(param_value), LazySymbol(param_other)});
+
+ auto ns = fxl::MakeRefCounted<Namespace>();
+ ns->set_assigned_name("ns");
+ function->set_parent(LazySymbol(ns));
+
+ EXPECT_EQ(
+ "kNormal \"ns::\", "
+ "kHeading \"Function\", "
+ "kComment \"<int>(…)\"",
+ FormatFunctionName(function.get(), false).GetDebugString());
+
+ function->set_parent(LazySymbol());
+}
+
+TEST(CommandUtils, FormatLocation) {
SymbolContext symbol_context = SymbolContext::ForRelativeAddresses();
- EXPECT_EQ("<invalid address>", DescribeLocation(Location(), true));
+ EXPECT_EQ("<invalid address>",
+ FormatLocation(Location(), true, false).AsString());
// Address-only location.
EXPECT_EQ(
"0x12345",
- DescribeLocation(Location(Location::State::kAddress, 0x12345), false));
+ FormatLocation(Location(Location::State::kAddress, 0x12345), false, false)
+ .AsString());
// Function-only location.
fxl::RefPtr<Function> function(fxl::MakeRefCounted<Function>());
function->set_assigned_name("Func");
function->set_code_ranges({{0x1200, 0x1300}});
EXPECT_EQ("Func() + 0x34 (no line info)",
- DescribeLocation(Location(0x1234, FileLine(), 0, symbol_context,
- LazySymbol(function)),
- false));
+ FormatLocation(Location(0x1234, FileLine(), 0, symbol_context,
+ LazySymbol(function)),
+ false, false)
+ .AsString());
// Same as above but location is before the function address (probably
// something is corrupt). It should omit the offset.
EXPECT_EQ("Func()",
- DescribeLocation(Location(0x1100, FileLine(), 0, symbol_context,
- LazySymbol(function)),
- false));
+ FormatLocation(Location(0x1100, FileLine(), 0, symbol_context,
+ LazySymbol(function)),
+ false, false)
+ .AsString());
// File/line-only location.
EXPECT_EQ("foo.cc:21",
- DescribeLocation(Location(0x1234, FileLine("/path/foo.cc", 21), 0,
- symbol_context),
- false));
+ FormatLocation(Location(0x1234, FileLine("/path/foo.cc", 21), 0,
+ symbol_context),
+ false, false)
+ .AsString());
// Full location.
Location loc(0x1234, FileLine("/path/foo.cc", 21), 0, symbol_context,
LazySymbol(function));
- EXPECT_EQ("0x1234, Func() • foo.cc:21", DescribeLocation(loc, true));
- EXPECT_EQ("Func() • foo.cc:21", DescribeLocation(loc, false));
+ EXPECT_EQ("0x1234, Func() • foo.cc:21",
+ FormatLocation(loc, true, false).AsString());
+ EXPECT_EQ("Func() • foo.cc:21", FormatLocation(loc, false, false).AsString());
}
TEST(CommandUtils, DescribeFileLine) {
@@ -261,8 +335,8 @@
// Multiple assign.
- err = SetElementsToAdd({"option_name", "value", "value2"}, &assign_type,
- &out);
+ err =
+ SetElementsToAdd({"option_name", "value", "value2"}, &assign_type, &out);
EXPECT_FALSE(err.has_error()) << err.msg();
EXPECT_EQ(assign_type, AssignType::kAssign);
ASSERT_EQ(out.size(), 2u);
diff --git a/bin/zxdb/console/console_context.cc b/bin/zxdb/console/console_context.cc
index 88b92df..a25d12f 100644
--- a/bin/zxdb/console/console_context.cc
+++ b/bin/zxdb/console/console_context.cc
@@ -511,7 +511,7 @@
FXL_DCHECK(!frames.empty());
const Location& location = frames[0]->GetLocation();
out.Append("at ");
- out.Append(DescribeLocation(location, false));
+ out.Append(FormatLocation(location, false, false));
if (location.has_symbols()) {
out.Append("\n");
} else {
diff --git a/bin/zxdb/console/format_frame.cc b/bin/zxdb/console/format_frame.cc
index 43af967..51d351b 100644
--- a/bin/zxdb/console/format_frame.cc
+++ b/bin/zxdb/console/format_frame.cc
@@ -24,7 +24,8 @@
namespace {
-void ListCompletedFrames(Thread* thread, bool long_format) {
+void ListCompletedFrames(Thread* thread, bool include_params,
+ bool long_format) {
Console* console = Console::get();
int active_frame_id = console->context().GetActiveFrameIdForThread(thread);
@@ -60,10 +61,11 @@
// Supply "-1" for the frame index to suppress printing (we already
// did it above).
if (long_format) {
- FormatFrameLong(frames[i], helper.get(), FormatValueOptions(), -1);
+ FormatFrameLong(frames[i], include_params, helper.get(),
+ FormatValueOptions(), -1);
} else {
OutputBuffer out;
- FormatFrame(frames[i], &out, -1);
+ FormatFrame(frames[i], include_params, &out, -1);
helper->Append(std::move(out));
}
@@ -77,26 +79,28 @@
} // namespace
-void OutputFrameList(Thread* thread, bool long_format) {
+void OutputFrameList(Thread* thread, bool include_params, bool long_format) {
// Always request an up-to-date frame list from the agent. Various things
// could have changed and the user is manually requesting a new list, so
// don't rely on the cached copy even if Thread::HasAllFrames() is true.
- thread->SyncFrames([ thread = thread->GetWeakPtr(), long_format ]() {
- Console* console = Console::get();
- if (thread)
- ListCompletedFrames(thread.get(), long_format);
- else
- console->Output("Thread exited, no frames.\n");
- });
+ thread->SyncFrames(
+ [ thread = thread->GetWeakPtr(), include_params, long_format ]() {
+ Console* console = Console::get();
+ if (thread)
+ ListCompletedFrames(thread.get(), include_params, long_format);
+ else
+ console->Output("Thread exited, no frames.\n");
+ });
}
-void FormatFrame(const Frame* frame, OutputBuffer* out, int id) {
+void FormatFrame(const Frame* frame, bool include_params, OutputBuffer* out,
+ int id) {
if (id >= 0)
out->Append(fxl::StringPrintf("Frame %d ", id));
- out->Append(DescribeLocation(frame->GetLocation(), false));
+ out->Append(FormatLocation(frame->GetLocation(), false, include_params));
}
-void FormatFrameLong(const Frame* frame, FormatValue* out,
+void FormatFrameLong(const Frame* frame, bool include_params, FormatValue* out,
const FormatValueOptions& options, int id) {
if (id >= 0)
out->Append(OutputBuffer::WithContents(fxl::StringPrintf("Frame %d ", id)));
@@ -105,7 +109,7 @@
// address will be shown twice.
const Location& location = frame->GetLocation();
if (location.has_symbols())
- out->Append(DescribeLocation(location, false));
+ out->Append(FormatLocation(location, false, include_params));
// Long format includes the IP address.
// TODO(brettw) handle asynchronously available BP.
diff --git a/bin/zxdb/console/format_frame.h b/bin/zxdb/console/format_frame.h
index 45a75a8..64748b8 100644
--- a/bin/zxdb/console/format_frame.h
+++ b/bin/zxdb/console/format_frame.h
@@ -13,21 +13,24 @@
class Thread;
// Outputs the list of frames to the console. This will complete asynchronously
-// if the frames are not currently available.
-void OutputFrameList(Thread* thread, bool long_format);
+// if the frames are not currently available. Printing of function parameter
+// types is controlled by include_params.
+void OutputFrameList(Thread* thread, bool include_params, bool long_format);
// Formats one frame using the short format to the output buffer. The frame ID
// will be printed if supplied. If the ID is -1, it will be omitted.
//
+// Printing of function parameter types is controlled by include_params.
+//
// This does not append a newline at the end of the output.
-void FormatFrame(const Frame* frame, OutputBuffer* out, int id = -1);
+void FormatFrame(const Frame* frame, bool include_params, OutputBuffer* out, int id = -1);
// Formats one frame using the long format. Since the long format includes
// function parameters which are computed asynchronously, this takes the
// asynchronous FormatValue as the output.
//
// This does not append a newline at the end of the output.
-void FormatFrameLong(const Frame* frame, FormatValue* out,
+void FormatFrameLong(const Frame* frame, bool include_params, FormatValue* out,
const FormatValueOptions& options, int id = -1);
} // namespace zxdb
diff --git a/bin/zxdb/console/format_frame_unittest.cc b/bin/zxdb/console/format_frame_unittest.cc
index a6f7dc5..0684440 100644
--- a/bin/zxdb/console/format_frame_unittest.cc
+++ b/bin/zxdb/console/format_frame_unittest.cc
@@ -24,7 +24,7 @@
auto helper = fxl::MakeRefCounted<FormatValue>(
std::make_unique<MockFormatValueProcessContext>());
- FormatFrameLong(frame, helper.get(), FormatValueOptions());
+ FormatFrameLong(frame, false, helper.get(), FormatValueOptions());
std::string out_string;
bool complete = false;
@@ -59,7 +59,7 @@
OutputBuffer out;
// Short format just prints the address.
- FormatFrame(&frame, &out);
+ FormatFrame(&frame, false, &out);
EXPECT_EQ("0x12345678", out.AsString());
// Long version should do the same (not duplicate it).
@@ -68,7 +68,7 @@
// With index.
out = OutputBuffer();
- FormatFrame(&frame, &out, 3);
+ FormatFrame(&frame, false, &out, 3);
EXPECT_EQ("Frame 3 0x12345678", out.AsString());
}
diff --git a/bin/zxdb/console/format_value_unittest.cc b/bin/zxdb/console/format_value_unittest.cc
index 3402db3..d63a1fe 100644
--- a/bin/zxdb/console/format_value_unittest.cc
+++ b/bin/zxdb/console/format_value_unittest.cc
@@ -655,12 +655,9 @@
SyncFormatValue(ExprValue(func_ptr_type, {5, 0, 0, 0, 0, 0, 0, 0}),
opts));
- // Found symbol (matching kAddress) should be printed. The "()" looks weird
- // here but this is what our function name outputs. GDB includes the () and
- // includes the parameter types which can disambiguate overloaded functions,
- // but also adds noise.
+ // Found symbol (matching kAddress) should be printed.
ExprValue good_ptr(func_ptr_type, {0x34, 0x12, 0, 0, 0, 0, 0, 0});
- EXPECT_EQ("&MyFunc()", SyncFormatValue(good_ptr, opts));
+ EXPECT_EQ("&MyFunc", SyncFormatValue(good_ptr, opts));
// Force output as hex even when the function is matched.
FormatValueOptions hex_opts;
@@ -684,7 +681,7 @@
// looks like a class member, but that's OK, wherever the address points to
// is what we print.
ExprValue good_member_func_ptr(member_func, {0x34, 0x12, 0, 0, 0, 0, 0, 0});
- EXPECT_EQ("(void (MyClass::*)()) &MyFunc()",
+ EXPECT_EQ("(void (MyClass::*)()) &MyFunc",
SyncFormatValue(good_member_func_ptr, type_opts));
}
diff --git a/bin/zxdb/console/input_location_parser.cc b/bin/zxdb/console/input_location_parser.cc
index 19ca76b..80e778e 100644
--- a/bin/zxdb/console/input_location_parser.cc
+++ b/bin/zxdb/console/input_location_parser.cc
@@ -148,7 +148,7 @@
if (locations[i].file_line().is_valid())
err_str += DescribeFileLine(locations[i].file_line(), true);
else
- err_str += DescribeLocation(locations[i], true);
+ err_str += FormatLocation(locations[i], true, false).AsString();
err_str += "\n";
}
if (locations.size() > kMaxSuggestions) {
diff --git a/bin/zxdb/console/nouns.cc b/bin/zxdb/console/nouns.cc
index c95b9a7..376c15d 100644
--- a/bin/zxdb/console/nouns.cc
+++ b/bin/zxdb/console/nouns.cc
@@ -34,7 +34,8 @@
namespace {
-constexpr int kVerboseSwitch = 1;
+constexpr int kForceTypes = 1;
+constexpr int kVerboseSwitch = 2;
// Frames ----------------------------------------------------------------------
@@ -57,7 +58,12 @@
Options
- --verbose | -v
+ -t
+ --types
+ Include all type information for function parameters.
+
+ -v
+ --verbose
Show more information in the frame list. This is valid when listing
frames only.
@@ -90,7 +96,8 @@
if (cmd.GetNounIndex(Noun::kFrame) == Command::kNoIndex) {
// Just "frame", this lists available frames.
- OutputFrameList(cmd.thread(), cmd.HasSwitch(kVerboseSwitch));
+ OutputFrameList(cmd.thread(), cmd.HasSwitch(kForceTypes),
+ cmd.HasSwitch(kVerboseSwitch));
return true;
}
@@ -106,7 +113,8 @@
// Schedule asynchronous output of the full frame description.
auto helper = fxl::MakeRefCounted<FormatValue>(
std::make_unique<FormatValueProcessContextImpl>(cmd.target()));
- FormatFrameLong(cmd.frame(), helper.get(), FormatValueOptions(),
+ FormatFrameLong(cmd.frame(), cmd.HasSwitch(kForceTypes), helper.get(),
+ FormatValueOptions(),
context->GetActiveFrameIdForThread(cmd.thread()));
helper->Complete(
[helper](OutputBuffer out) { Console::get()->Output(std::move(out)); });
@@ -601,6 +609,7 @@
const std::vector<SwitchRecord>& GetNounSwitches() {
static std::vector<SwitchRecord> switches;
if (switches.empty()) {
+ switches.emplace_back(kForceTypes, false, "types", 't');
switches.emplace_back(kVerboseSwitch, false, "verbose", 'v');
}
return switches;
diff --git a/bin/zxdb/console/output_buffer.cc b/bin/zxdb/console/output_buffer.cc
index 85ee812..7c9f227 100644
--- a/bin/zxdb/console/output_buffer.cc
+++ b/bin/zxdb/console/output_buffer.cc
@@ -172,6 +172,66 @@
} // namespace
+const char* SyntaxToString(Syntax syntax) {
+ switch (syntax) {
+ case Syntax::kNormal: return "kNormal";
+ case Syntax::kComment: return "kComment";
+ case Syntax::kHeading: return "kHeading";
+ case Syntax::kError: return "kError";
+ case Syntax::kWarning: return "kWarning";
+ case Syntax::kSpecial: return "kSpecial";
+ case Syntax::kReversed: return "kReversed";
+ case Syntax::kVariable: return "kVariable";
+ }
+ return nullptr;
+}
+
+const char* TextBackgroundColorToString(TextBackgroundColor color) {
+ switch (color) {
+ case TextBackgroundColor::kDefault: return "kDefault";
+ case TextBackgroundColor::kBlack: return "kBlack";
+ case TextBackgroundColor::kBlue: return "kBlue";
+ case TextBackgroundColor::kCyan: return "kCyan";
+ case TextBackgroundColor::kGray: return "kGray";
+ case TextBackgroundColor::kGreen: return "kGreen";
+ case TextBackgroundColor::kMagenta: return "kMagenta";
+ case TextBackgroundColor::kRed: return "kRed";
+ case TextBackgroundColor::kYellow: return "kYellow";
+ case TextBackgroundColor::kWhite: return "kWhite";
+ case TextBackgroundColor::kLightBlue: return "kLightBlue";
+ case TextBackgroundColor::kLightCyan: return "kLightCyan";
+ case TextBackgroundColor::kLightGray: return "kLightGray";
+ case TextBackgroundColor::kLightGreen: return "kLightGreen";
+ case TextBackgroundColor::kLightMagenta: return "kLightMagenta";
+ case TextBackgroundColor::kLightRed: return "kLightRed";
+ case TextBackgroundColor::kLightYellow: return "kLightYellow";
+ }
+ return nullptr;
+}
+
+const char* TextForegroundColorToString(TextForegroundColor color) {
+ switch (color) {
+ case TextForegroundColor::kDefault: return "kDefault";
+ case TextForegroundColor::kBlack: return "kBlack";
+ case TextForegroundColor::kBlue: return "kBlue";
+ case TextForegroundColor::kCyan: return "kCyan";
+ case TextForegroundColor::kGray: return "kGray";
+ case TextForegroundColor::kGreen: return "kGreen";
+ case TextForegroundColor::kMagenta: return "kMagenta";
+ case TextForegroundColor::kRed: return "kRed";
+ case TextForegroundColor::kYellow: return "kYellow";
+ case TextForegroundColor::kWhite: return "kWhite";
+ case TextForegroundColor::kLightBlue: return "kLightBlue";
+ case TextForegroundColor::kLightCyan: return "kLightCyan";
+ case TextForegroundColor::kLightGray: return "kLightGray";
+ case TextForegroundColor::kLightGreen: return "kLightGreen";
+ case TextForegroundColor::kLightMagenta: return "kLightMagenta";
+ case TextForegroundColor::kLightRed: return "kLightRed";
+ case TextForegroundColor::kLightYellow: return "kLightYellow";
+ }
+ return nullptr;
+}
+
OutputBuffer::Span::Span(Syntax s, std::string t) : syntax(s), text(std::move(t)) {}
OutputBuffer::Span::Span(TextForegroundColor fg, std::string t) : foreground(fg), text(std::move(t)) {}
@@ -300,4 +360,43 @@
spans_.clear();
}
+std::string OutputBuffer::GetDebugString() const {
+ // Normalize so the output is the same even if it was built with different
+ // sequences of spans.
+ std::vector<Span> normalized;
+ for (const auto& cur : spans_) {
+ if (normalized.empty()) {
+ normalized.push_back(cur);
+ } else {
+ Span& prev = normalized.back();
+ if (prev.syntax == cur.syntax &&
+ prev.background == cur.background &&
+ prev.foreground == cur.foreground)
+ prev.text.append(cur.text); // Merge: continuation of same format.
+ else
+ normalized.push_back(cur); // New format.
+ }
+ }
+
+ std::string result;
+ for (size_t i = 0; i < normalized.size(); i++) {
+ if (i > 0)
+ result += ", ";
+
+ result += SyntaxToString(normalized[i].syntax);
+ if (normalized[i].background != TextBackgroundColor::kDefault ||
+ normalized[i].foreground != TextForegroundColor::kDefault) {
+ result += " ";
+ result += TextBackgroundColorToString(normalized[i].background);
+ result += " ";
+ result += TextForegroundColorToString(normalized[i].foreground);
+ }
+
+ result += " \"";
+ result += normalized[i].text;
+ result.push_back('"');
+ }
+ return result;
+}
+
} // namespace zxdb
diff --git a/bin/zxdb/console/output_buffer.h b/bin/zxdb/console/output_buffer.h
index c5b50d5..413cec9 100644
--- a/bin/zxdb/console/output_buffer.h
+++ b/bin/zxdb/console/output_buffer.h
@@ -24,6 +24,8 @@
kVariable // Use for variable names.
};
+const char* SyntaxToString(Syntax);
+
// The following color enums are to be used when Syntax is not enough, which
// is meant to semantic meaning. Colors are to be used by specific output that
// use more fine-grained control over color output, like the register output
@@ -53,6 +55,8 @@
kLightYellow,
};
+const char* TextBackgroundColorToString(TextBackgroundColor);
+
enum class TextForegroundColor {
kDefault,
// Basic 16 colors
@@ -75,6 +79,8 @@
kLightYellow,
};
+const char* TextForegroundColorToString(TextBackgroundColor);
+
// This class collects output from commands so it can be put on the screen in
// one chunk. It's not just a string because we want to add helper functions
// and may want to add things like coloring in the future.
@@ -120,6 +126,17 @@
bool empty() const { return spans_.empty(); }
+ // Formats this buffer's formatting into a text form for testing. It will be
+ // normalized (so adjacent spans of the same format will be combined).
+ //
+ // The format is
+ // <format> "<text>", <format> "<text>", ...
+ // The format is the syntax enum name, and if either foreground or background
+ // are non-default, background and foreground, they will both follow (always
+ // together), separated by spaces. So:
+ // kComment "foo", kNormal kGreen kGray "bar"
+ std::string GetDebugString() const;
+
private:
struct Span {
Span(Syntax s, std::string t);
diff --git a/bin/zxdb/console/verbs_symbol.cc b/bin/zxdb/console/verbs_symbol.cc
index 5a03b0a..1091705 100644
--- a/bin/zxdb/console/verbs_symbol.cc
+++ b/bin/zxdb/console/verbs_symbol.cc
@@ -521,7 +521,7 @@
cmd.target()->GetProcess()->GetSymbols()->ResolveInputLocation(
InputLocation(address));
FXL_DCHECK(locations.size() == 1u);
- Console::get()->Output(DescribeLocation(locations[0], true));
+ Console::get()->Output(FormatLocation(locations[0], true, true));
return Err();
}
diff --git a/bin/zxdb/console/verbs_thread.cc b/bin/zxdb/console/verbs_thread.cc
index d3ab77f..ead47db 100644
--- a/bin/zxdb/console/verbs_thread.cc
+++ b/bin/zxdb/console/verbs_thread.cc
@@ -126,6 +126,12 @@
To see less information, use "frame" or just "f".
+Arguments
+
+ -t
+ --types
+ Include all type information for function parameters.
+
Examples
t 2 bt
@@ -139,7 +145,8 @@
if (!cmd.thread())
return Err("There is no thread to have frames.");
- OutputFrameList(cmd.thread(), true);
+ bool show_params = cmd.HasSwitch(kForceTypes);
+ OutputFrameList(cmd.thread(), show_params, true);
return Err();
}
@@ -1004,17 +1011,20 @@
void AppendThreadVerbs(std::map<Verb, VerbRecord>* verbs) {
// Shared options for value printing.
+ SwitchRecord force_types(kForceTypes, false, "types", 't');
const std::vector<SwitchRecord> format_switches{
- SwitchRecord(kForceTypes, false, "types", 't'),
+ force_types,
SwitchRecord(kForceNumberChar, false, "", 'c'),
SwitchRecord(kForceNumberSigned, false, "", 'd'),
SwitchRecord(kForceNumberUnsigned, false, "", 'u'),
SwitchRecord(kForceNumberHex, false, "", 'x'),
SwitchRecord(kMaxArraySize, true, "max-array")};
- (*verbs)[Verb::kBacktrace] =
- VerbRecord(&DoBacktrace, {"backtrace", "bt"}, kBacktraceShortHelp,
- kBacktraceHelp, CommandGroup::kQuery);
+ VerbRecord backtrace(&DoBacktrace, {"backtrace", "bt"}, kBacktraceShortHelp,
+ kBacktraceHelp, CommandGroup::kQuery);
+ backtrace.switches = {force_types};
+ (*verbs)[Verb::kBacktrace] = std::move(backtrace);
+
(*verbs)[Verb::kContinue] =
VerbRecord(&DoContinue, {"continue", "c"}, kContinueShortHelp,
kContinueHelp, CommandGroup::kStep, SourceAffinity::kSource);
diff --git a/bin/zxdb/symbols/function.cc b/bin/zxdb/symbols/function.cc
index 1bc8917..80cd2d6 100644
--- a/bin/zxdb/symbols/function.cc
+++ b/bin/zxdb/symbols/function.cc
@@ -14,13 +14,7 @@
const Function* Function::AsFunction() const { return this; }
std::string Function::ComputeFullName() const {
- // TODO(brettw) add parameter types, at least for C++, since this is part of
- // the function type signature.
- //
- // This doesn't show the return types because C++ can't overload based on
- // return types so they're not ambiguous (and add noise). Neither GDB nor
- // LLDB shows the function return types normally.
- return GetSymbolScopePrefix(this) + GetAssignedName() + "()";
+ return GetSymbolScopePrefix(this) + GetAssignedName();
}
} // namespace zxdb
diff --git a/bin/zxdb/symbols/function.h b/bin/zxdb/symbols/function.h
index 1487a65..802c18f 100644
--- a/bin/zxdb/symbols/function.h
+++ b/bin/zxdb/symbols/function.h
@@ -30,6 +30,12 @@
// attributes from the specification automatically so this Function object
// will have full context. Be aware that this won't necessarily match the
// DIE that generated the object.
+//
+// NAMING: The "full name" (as returned by Symbol::GetFullName()) of a function
+// is the qualified name without any return types or parameters. Some callers
+// may want parameters, we can add a helper function in the future if necessary
+// (for display we would often want to syntax highlight these differently so
+// this is often betetr done at a a different layer).
class Function final : public CodeBlock {
public:
// Construct with fxl::MakeRefCounted().
diff --git a/bin/zxdb/symbols/symbol_utils_unittest.cc b/bin/zxdb/symbols/symbol_utils_unittest.cc
index 7015e3d..d618dae 100644
--- a/bin/zxdb/symbols/symbol_utils_unittest.cc
+++ b/bin/zxdb/symbols/symbol_utils_unittest.cc
@@ -49,19 +49,18 @@
ns->set_assigned_name("ns");
// Function definition inside namespace.
- // TODO(brettw) these will need to be updated when function parameter types
- // are added to the name.
fxl::RefPtr<Function> fn = fxl::MakeRefCounted<Function>();
fn->set_parent(LazySymbol(ns));
fn->set_assigned_name("Function");
EXPECT_EQ("ns::", GetSymbolScopePrefix(fn.get()));
- EXPECT_EQ("ns::Function()", fn->GetFullName());
+ EXPECT_EQ("ns::Function", fn->GetFullName());
// Lexical scope inside the function. This should not appear in names.
+ // TODO(brettw) these nested function scopes should include paramter names.
fxl::RefPtr<CodeBlock> block =
fxl::MakeRefCounted<CodeBlock>(Symbol::kTagLexicalBlock);
block->set_parent(LazySymbol(fn));
- EXPECT_EQ("ns::Function()::", GetSymbolScopePrefix(block.get()));
+ EXPECT_EQ("ns::Function::", GetSymbolScopePrefix(block.get()));
EXPECT_EQ("", block->GetFullName());
// Type defined inside the function is qualified by the function name. This
@@ -70,8 +69,8 @@
fxl::MakeRefCounted<Collection>(Symbol::kTagStructureType);
sc->set_parent(LazySymbol(block));
sc->set_assigned_name("Struct");
- EXPECT_EQ("ns::Function()::", GetSymbolScopePrefix(sc.get()));
- EXPECT_EQ("ns::Function()::Struct", sc->GetFullName());
+ EXPECT_EQ("ns::Function::", GetSymbolScopePrefix(sc.get()));
+ EXPECT_EQ("ns::Function::Struct", sc->GetFullName());
// Variable defined inside the function. Currently these are qualified with
// the function name like the types above. But this may need to change
@@ -83,8 +82,8 @@
fxl::MakeRefCounted<Variable>(Symbol::kTagVariable);
var->set_parent(LazySymbol(block));
var->set_assigned_name("var");
- EXPECT_EQ("ns::Function()::", GetSymbolScopePrefix(var.get()));
- EXPECT_EQ("ns::Function()::var", var->GetFullName());
+ EXPECT_EQ("ns::Function::", GetSymbolScopePrefix(var.get()));
+ EXPECT_EQ("ns::Function::var", var->GetFullName());
}
} // namespace zxdb
