This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: ui_out format strings for fields and styles (Re: [PATCH] Style "pwd" output)
On 6/5/19 9:27 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> Prototype time!
>
> Nice.
>
> Pedro> I reserved "%pS" and "%pN" for styling, but haven't really prototyped that.
> Pedro> Is there a global current style stack we can push a style to/from?
>
> No, there's only a way to set the current style. However, it's not
> really intended for callers to set the style and then just leave it set
> -- all the existing calls should set the style, then later set it back
> to the default style.
>
> It seems to me that this could be enforced by our printf. Or, if we
> really do want stacking, the stack could just be local to this function.
>
> I wouldn't mind some kind of brackets being used as the characters here,
> like %p[ ... %p]. It's too bad that we still have to pass some pointer
> value for the closer.
Yeah. I prototyped %pS/%pN now, and having to pass a value for the closer
is ... yeah, a bit ugly.
Also, since gcc expects a pointer for %p, and we want to pass an
enum for the style, I added a small pointer-wrapper hack -- see the
ptr function. Yay C++.
I've not been paying much attention to the styling patches, so I can't
off hand tell which places would benefit the most from this. So I just
grepped for _styled and replaced a couple spots. Likely there are better
examples.
Anyway, see below.
The format strings do get a bit ... wild:
"%pS%s%pN %pS%s%pN\n"
With your suggestion, it'd look like:
"%p[%s%p] %p[%s%p]\n"
I'm thinking that %pS/%pN %p[/%pN] are more appropriate when
the styled string is constant, like:
"this %p[text here%p] should be styled\n"
For the seemingly common case of printing a string variable
with a style, I'm thinking that a specific formatter would
be better. I'll post a follow up patch for that.
>From 8f1d8637839fc953d089f4816a12531fbf35202b Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 5 Jun 2019 20:44:54 +0100
Subject: [PATCH 1/2] Support %pS / %pN for styling
---
gdb/breakpoint.c | 6 +++---
gdb/cli-out.c | 50 ++++++++++++++++++++++++++------------------------
gdb/cli-out.h | 5 +++--
gdb/mi/mi-out.c | 3 ++-
gdb/mi/mi-out.h | 5 +++--
gdb/symtab.c | 26 ++++++++++++++------------
gdb/ui-out.c | 23 +++++++++++++----------
gdb/ui-out.h | 25 ++++++++++++++++++++++---
gdb/utils.c | 10 ++++++++++
gdb/utils.h | 6 ++++++
10 files changed, 102 insertions(+), 57 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index c4270064275..0c3c7df92fb 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6693,9 +6693,9 @@ describe_other_breakpoints (struct gdbarch *gdbarch,
(others > 1) ? ","
: ((others == 1) ? " and" : ""));
}
- printf_filtered (_("also set at pc "));
- fputs_styled (paddress (gdbarch, pc), address_style.style (), gdb_stdout);
- printf_filtered (".\n");
+ current_uiout->message (_("also set at pc %pS%s%pN.\n"),
+ ptr (ui_out_style_kind::ADDRESS),
+ paddress (gdbarch, pc), nullptr);
}
}
diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index 55c8d2b3b1b..71ed5c3cf33 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -118,6 +118,27 @@ cli_ui_out::do_field_skip (int fldno, int width, ui_align alignment,
ui_out_style_kind::DEFAULT);
}
+static ui_file_style
+style_from_style_kind (ui_out_style_kind kind)
+{
+ switch (kind)
+ {
+ case ui_out_style_kind::DEFAULT:
+ /* Nothing. */
+ return {};
+ case ui_out_style_kind::FILE:
+ return file_name_style.style ();
+ case ui_out_style_kind::FUNCTION:
+ return function_name_style.style ();
+ case ui_out_style_kind::VARIABLE:
+ return variable_name_style.style ();
+ case ui_out_style_kind::ADDRESS:
+ return address_style.style ();
+ default:
+ gdb_assert_not_reached ("missing case");
+ }
+}
+
/* other specific cli_field_* end up here so alignment and field
separators are both handled by cli_field_string */
@@ -160,28 +181,7 @@ cli_ui_out::do_field_string (int fldno, int width, ui_align align,
if (string)
{
- ui_file_style fstyle;
- switch (style)
- {
- case ui_out_style_kind::DEFAULT:
- /* Nothing. */
- break;
- case ui_out_style_kind::FILE:
- /* Nothing. */
- fstyle = file_name_style.style ();
- break;
- case ui_out_style_kind::FUNCTION:
- fstyle = function_name_style.style ();
- break;
- case ui_out_style_kind::VARIABLE:
- fstyle = variable_name_style.style ();
- break;
- case ui_out_style_kind::ADDRESS:
- fstyle = address_style.style ();
- break;
- default:
- gdb_assert_not_reached ("missing case");
- }
+ ui_file_style fstyle = style_from_style_kind (style);
fputs_styled (string, fstyle, m_streams.back ());
}
@@ -227,12 +227,14 @@ cli_ui_out::do_text (const char *string)
}
void
-cli_ui_out::do_message (const char *format, va_list args)
+cli_ui_out::do_message (ui_out_style_kind style,
+ const char *format, va_list args)
{
if (m_suppress_output)
return;
- vfprintf_unfiltered (m_streams.back (), format, args);
+ ui_file_style fstyle = style_from_style_kind (style);
+ vfprintf_styled (m_streams.back (), fstyle, format, args);
}
void
diff --git a/gdb/cli-out.h b/gdb/cli-out.h
index eeb555fbbec..4ed7b396a57 100644
--- a/gdb/cli-out.h
+++ b/gdb/cli-out.h
@@ -59,8 +59,9 @@ protected:
override ATTRIBUTE_PRINTF (6,0);
virtual void do_spaces (int numspaces) override;
virtual void do_text (const char *string) override;
- virtual void do_message (const char *format, va_list args) override
- ATTRIBUTE_PRINTF (2,0);
+ virtual void do_message (ui_out_style_kind style,
+ const char *format, va_list args) override
+ ATTRIBUTE_PRINTF (3,0);
virtual void do_wrap_hint (const char *identstring) override;
virtual void do_flush () override;
virtual void do_redirect (struct ui_file *outstream) override;
diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index d8bee0f3927..bb52360e540 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -159,7 +159,8 @@ mi_ui_out::do_text (const char *string)
}
void
-mi_ui_out::do_message (const char *format, va_list args)
+mi_ui_out::do_message (ui_out_style_kind style,
+ const char *format, va_list args)
{
}
diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h
index 82f77592da8..914a44194d4 100644
--- a/gdb/mi/mi-out.h
+++ b/gdb/mi/mi-out.h
@@ -64,8 +64,9 @@ protected:
override ATTRIBUTE_PRINTF (6,0);
virtual void do_spaces (int numspaces) override;
virtual void do_text (const char *string) override;
- virtual void do_message (const char *format, va_list args) override
- ATTRIBUTE_PRINTF (2,0);
+ virtual void do_message (ui_out_style_kind style,
+ const char *format, va_list args) override
+ ATTRIBUTE_PRINTF (3,0);
virtual void do_wrap_hint (const char *identstring) override;
virtual void do_flush () override;
virtual void do_redirect (struct ui_file *outstream) override;
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 130d5cd48ff..5696d6fa890 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4603,9 +4603,10 @@ print_symbol_info (enum search_domain kind,
if (filename_cmp (last, s_filename) != 0)
{
- fputs_filtered ("\nFile ", gdb_stdout);
- fputs_styled (s_filename, file_name_style.style (), gdb_stdout);
- fputs_filtered (":\n", gdb_stdout);
+ current_uiout->message ("\nFile %pS%s%pN:\n",
+ ptr (ui_out_style_kind::FILE),
+ s_filename,
+ nullptr);
}
if (SYMBOL_LINE (sym) != 0)
@@ -4651,15 +4652,16 @@ print_msymbol_info (struct bound_minimal_symbol msymbol)
else
tmp = hex_string_custom (BMSYMBOL_VALUE_ADDRESS (msymbol),
16);
- fputs_styled (tmp, address_style.style (), gdb_stdout);
- fputs_filtered (" ", gdb_stdout);
- if (msymbol.minsym->text_p ())
- fputs_styled (MSYMBOL_PRINT_NAME (msymbol.minsym),
- function_name_style.style (),
- gdb_stdout);
- else
- fputs_filtered (MSYMBOL_PRINT_NAME (msymbol.minsym), gdb_stdout);
- fputs_filtered ("\n", gdb_stdout);
+
+ current_uiout->message (_("%pS%s%pN %pS%s%pN\n"),
+ ptr (ui_out_style_kind::ADDRESS),
+ tmp,
+ nullptr,
+ (msymbol.minsym->text_p ()
+ ? ptr (ui_out_style_kind::FUNCTION)
+ : ptr (ui_out_style_kind::DEFAULT)),
+ MSYMBOL_PRINT_NAME (msymbol.minsym),
+ nullptr);
}
/* This is the guts of the commands "info functions", "info types", and
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 9c987a6cfa6..6e1c87ab841 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -548,12 +548,12 @@ ui_out::text (const char *string)
}
void
-ui_out::call_do_message (const char *format, ...)
+ui_out::call_do_message (ui_out_style_kind style, const char *format, ...)
{
va_list args;
va_start (args, format);
- do_message (format, args);
+ do_message (style, format, args);
va_end (args);
}
@@ -562,6 +562,8 @@ ui_out::message (const char *format, ...)
{
format_pieces fpieces (&format, true);
+ ui_out_style_kind style = ui_out_style_kind::DEFAULT;
+
va_list args;
va_start (args, format);
@@ -572,7 +574,7 @@ ui_out::message (const char *format, ...)
switch (piece.argclass)
{
case string_arg:
- call_do_message (current_substring, va_arg (args, const char *));
+ call_do_message (style, current_substring, va_arg (args, const char *));
break;
case wide_string_arg:
/* FIXME */
@@ -582,16 +584,16 @@ ui_out::message (const char *format, ...)
break;
case long_long_arg:
#ifdef PRINTF_HAS_LONG_LONG
- call_do_message (current_substring, va_arg (args, long long));
+ call_do_message (style, current_substring, va_arg (args, long long));
break;
#else
error (_("long long not supported in ui_out::message"));
#endif
case int_arg:
- call_do_message (current_substring, va_arg (args, int));
+ call_do_message (style, current_substring, va_arg (args, int));
break;
case long_arg:
- call_do_message (current_substring, va_arg (args, long));
+ call_do_message (style, current_substring, va_arg (args, long));
break;
/* Handle floating-point values. */
case double_arg:
@@ -611,13 +613,14 @@ ui_out::message (const char *format, ...)
}
break;
case 'S':
- /* Push style on stack? */
+ style = *va_arg (args, ui_out_style_kind *);
break;
case 'N':
- /* Pop style from stack? */
+ va_arg (args, void *);
+ style = ui_out_style_kind::DEFAULT;
break;
default:
- call_do_message (current_substring, va_arg (args, void *));
+ call_do_message (style, current_substring, va_arg (args, void *));
break;
}
break;
@@ -629,7 +632,7 @@ ui_out::message (const char *format, ...)
pass a dummy argument because some platforms have
modified GCC to include -Wformat-security by default,
which will warn here if there is no argument. */
- call_do_message (current_substring, 0);
+ call_do_message (style, current_substring, 0);
break;
default:
internal_error (__FILE__, __LINE__,
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index b20d9508330..18af856313d 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -103,6 +103,24 @@ private:
int m_val;
};
+/* Wrap a ui_out_style_kind in a pointer to a temporary. */
+
+/* XXX: Make a template? */
+struct ptrS
+{
+ ptrS (ui_out_style_kind t) : m_t (t) {}
+
+ const ui_out_style_kind *ptr () const { return &m_t; }
+
+ ui_out_style_kind m_t;
+};
+
+static inline const ui_out_style_kind *
+ptr (ptrS &&t)
+{
+ return t.ptr ();
+}
+
class ui_out
{
public:
@@ -188,8 +206,9 @@ class ui_out
ATTRIBUTE_PRINTF (6,0) = 0;
virtual void do_spaces (int numspaces) = 0;
virtual void do_text (const char *string) = 0;
- virtual void do_message (const char *format, va_list args)
- ATTRIBUTE_PRINTF (2,0) = 0;
+ virtual void do_message (ui_out_style_kind style,
+ const char *format, va_list args)
+ ATTRIBUTE_PRINTF (3,0) = 0;
virtual void do_wrap_hint (const char *identstring) = 0;
virtual void do_flush () = 0;
virtual void do_redirect (struct ui_file *outstream) = 0;
@@ -201,7 +220,7 @@ class ui_out
{ return false; }
private:
- void call_do_message (const char *format, ...);
+ void call_do_message (ui_out_style_kind style, const char *format, ...);
ui_out_flags m_flags;
diff --git a/gdb/utils.c b/gdb/utils.c
index f55661287eb..6ba4e41faa2 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2147,6 +2147,16 @@ fprintf_styled (struct ui_file *stream, const ui_file_style &style,
set_output_style (stream, ui_file_style ());
}
+/* See utils.h. */
+
+void
+vfprintf_styled (struct ui_file *stream, const ui_file_style &style,
+ const char *format, va_list args)
+{
+ set_output_style (stream, style);
+ vfprintf_filtered (stream, format, args);
+ set_output_style (stream, ui_file_style ());
+}
void
printf_filtered (const char *format, ...)
diff --git a/gdb/utils.h b/gdb/utils.h
index 58b4a283447..bb73d4a8e72 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -429,6 +429,12 @@ extern void fprintf_styled (struct ui_file *stream,
...)
ATTRIBUTE_PRINTF (3, 4);
+extern void vfprintf_styled (struct ui_file *stream,
+ const ui_file_style &style,
+ const char *fmt,
+ va_list args)
+ ATTRIBUTE_PRINTF (3, 0);
+
/* Like fputs_filtered, but styles the output according to STYLE, when
appropriate. */
--
2.14.5