ui_out format strings for fields and styles (Re: [PATCH] Style "pwd" output)
Commit Message
On 7/1/19 1:55 PM, Pedro Alves wrote:
> There are two other things that I wanted to experiment/try first.
>
> - See if we could get rid of the need for the ".ptr ()" calls.
>
> - See about implementing support for some ui field type other than
> integers, to make sure that we're not missing something.
Here's a patch for thing #2. It adds a string_field type.
It applies on top of the other patch that gets rid of .ptr(), but
it's super easy to make string_field a ctor, that was not the
point here.
I looked for a few things here and there to convert, one thing
was interesting, the field_fmt() calls. If we wanted to add some
fmt_field type for that, one which saves a copy of the va_list
as a field, to avoid formatting into a temporary string, then
the "get rid of .ptr()" patch conflicts with that desire, as that
relies on a default argument to int_field/string_field etc, which
doesn't work when you also want a "..." parameter, like:
static inline string_field_s *
fmt_field (const char *name, const char *str,
fmt_s &&tmp = {}, ...)
{
tmp.vargs = va_copy ....;
...
}
I'm not sure whether we could portably save the varargs like
above, though. So that might be moot.
The patch replaces the format_fmt calls with string_printf + string_field.
From ebcc5fb83fb0c6e98db2501c5868ba8cee6dd1f8 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 7 Jun 2019 22:45:09 +0100
Subject: [PATCH] string_field
---
gdb/breakpoint.c | 14 ++++++--------
gdb/infrun.c | 19 +++++++------------
gdb/ui-out.c | 18 ++++++++++++++++--
gdb/ui-out.h | 37 ++++++++++++++++++++++++++++++++++++-
4 files changed, 65 insertions(+), 23 deletions(-)
Comments
On 7/1/19 2:17 PM, Pedro Alves wrote:
> On 7/1/19 1:55 PM, Pedro Alves wrote:
>
>> There are two other things that I wanted to experiment/try first.
>>
>> - See if we could get rid of the need for the ".ptr ()" calls.
>>
>> - See about implementing support for some ui field type other than
>> integers, to make sure that we're not missing something.
> Here's a patch for thing #2. It adds a string_field type.
I pushed the 4 patches I shown today to the users/palves/format_strings-experiment
branch now. (left users/palves/format_strings untouched.)
Thanks,
Pedro Alves
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> I'm not sure whether we could portably save the varargs like
Pedro> above, though. So that might be moot.
I'm not sure it matters a whole lot.
I looked through all the field_fmt calls, and many of them can be
replaced with some other call.
The few that remain can be rewritten in terms of string_printf if need
be.
Pedro> - uiout->text ("[Inferior ");
Pedro> - uiout->text (plongest (inf->num));
Pedro> - uiout->text (" (");
Pedro> - uiout->text (pidstr.c_str ());
Pedro> - uiout->text (") exited with code ");
Pedro> - uiout->field_fmt ("exit-code", "0%o", (unsigned int) exitstatus);
Pedro> - uiout->text ("]\n");
Pedro> + std::string exit_code_str
Pedro> + = string_printf ("0%o", (unsigned int) exitstatus);
Pedro> + uiout->message ("[Inferior %s (%s) exited with code %pF]\n",
Pedro> + plongest (inf->num), pidstr.c_str (),
Pedro> + string_field ("exit-code", exit_code_str.c_str ()));
This is so much better.
Tom
Tom> I looked through all the field_fmt calls, and many of them can be
Tom> replaced with some other call.
I wrote some patches to do this where it is possible.
Tom
On 7/1/19 7:49 PM, Tom Tromey wrote:
> Tom> I looked through all the field_fmt calls, and many of them can be
> Tom> replaced with some other call.
>
> I wrote some patches to do this where it is possible.
Awesome. I wrote a patch to implement %p[ / %p], and am now writing
a patch to document the formatters.
Thanks,
Pedro Alves
On 7/1/19 6:38 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> I'm not sure whether we could portably save the varargs like
> Pedro> above, though. So that might be moot.
>
> I'm not sure it matters a whole lot.
>
> I looked through all the field_fmt calls, and many of them can be
> replaced with some other call.
>
> The few that remain can be rewritten in terms of string_printf if need
> be.
>
> Pedro> - uiout->text ("[Inferior ");
> Pedro> - uiout->text (plongest (inf->num));
> Pedro> - uiout->text (" (");
> Pedro> - uiout->text (pidstr.c_str ());
> Pedro> - uiout->text (") exited with code ");
> Pedro> - uiout->field_fmt ("exit-code", "0%o", (unsigned int) exitstatus);
> Pedro> - uiout->text ("]\n");
> Pedro> + std::string exit_code_str
> Pedro> + = string_printf ("0%o", (unsigned int) exitstatus);
> Pedro> + uiout->message ("[Inferior %s (%s) exited with code %pF]\n",
> Pedro> + plongest (inf->num), pidstr.c_str (),
> Pedro> + string_field ("exit-code", exit_code_str.c_str ()));
>
> This is so much better.
I merged this one to users/palves/format_strings too.
Thanks,
Pedro Alves
@@ -6193,10 +6193,9 @@ print_one_breakpoint_location (struct breakpoint *b,
&& breakpoint_condition_evaluation_mode ()
== condition_evaluation_target)
{
- uiout->text (" (");
- uiout->field_string ("evaluated-by",
- bp_condition_evaluator (b));
- uiout->text (" evals)");
+ uiout->message (" (%pF evals)",
+ string_field ("evaluated-by",
+ bp_condition_evaluator (b)));
}
uiout->text ("\n");
}
@@ -12752,10 +12751,9 @@ tracepoint_print_one_detail (const struct breakpoint *self,
{
gdb_assert (self->type == bp_static_tracepoint);
- uiout->text ("\tmarker id is ");
- uiout->field_string ("static-tracepoint-marker-string-id",
- tp->static_trace_marker_id);
- uiout->text ("\n");
+ uiout->message ("\tmarker id is %pF\n",
+ string_field ("static-tracepoint-marker-string-id",
+ tp->static_trace_marker_id));
}
}
@@ -7641,24 +7641,19 @@ print_exited_reason (struct ui_out *uiout, int exitstatus)
{
if (uiout->is_mi_like_p ())
uiout->field_string ("reason", async_reason_lookup (EXEC_ASYNC_EXITED));
- uiout->text ("[Inferior ");
- uiout->text (plongest (inf->num));
- uiout->text (" (");
- uiout->text (pidstr.c_str ());
- uiout->text (") exited with code ");
- uiout->field_fmt ("exit-code", "0%o", (unsigned int) exitstatus);
- uiout->text ("]\n");
+ std::string exit_code_str
+ = string_printf ("0%o", (unsigned int) exitstatus);
+ uiout->message ("[Inferior %s (%s) exited with code %pF]\n",
+ plongest (inf->num), pidstr.c_str (),
+ string_field ("exit-code", exit_code_str.c_str ()));
}
else
{
if (uiout->is_mi_like_p ())
uiout->field_string
("reason", async_reason_lookup (EXEC_ASYNC_EXITED_NORMALLY));
- uiout->text ("[Inferior ");
- uiout->text (plongest (inf->num));
- uiout->text (" (");
- uiout->text (pidstr.c_str ());
- uiout->text (") exited normally]\n");
+ uiout->message ("[Inferior %s (%s) exited normally]\n",
+ plongest (inf->num), pidstr.c_str ());
}
}
@@ -607,8 +607,22 @@ ui_out::vmessage (const char *format, va_list args)
{
case 'F':
{
- int_field_s *field = va_arg (args, int_field_s *);
- field_int (field->name, field->val);
+ base_field_s *bf = va_arg (args, base_field_s *);
+ switch (bf->kind)
+ {
+ case field_kind::INT:
+ {
+ auto *f = (int_field_s *) bf;
+ field_int (f->name, f->val);
+ }
+ break;
+ case field_kind::STRING:
+ {
+ auto *f = (string_field_s *) bf;
+ field_string (f->name, f->str);
+ }
+ break;
+ }
}
break;
case 's':
@@ -68,11 +68,24 @@ enum ui_out_type
ui_out_type_list
};
+enum class field_kind
+ {
+ INT,
+ STRING,
+ };
+
/* An int field, to be passed to %pF in format strings. */
-struct int_field_s
+struct base_field_s
{
const char *name;
+ field_kind kind;
+};
+
+/* An int field, to be passed to %pF in format strings. */
+
+struct int_field_s : base_field_s
+{
int val;
};
@@ -85,10 +98,32 @@ int_field (const char *name, int val,
int_field_s &&tmp = {})
{
tmp.name = name;
+ tmp.kind = field_kind::INT;
tmp.val = val;
return &tmp;
}
+/* A string field, to be passed to %pF in format strings. */
+
+struct string_field_s : base_field_s
+{
+ const char *str;
+};
+
+/* Construct a temporary string_field_s on the caller's stack and
+ return a pointer to the constructed object. We use this because
+ it's not possible to pass a reference via va_args. */
+
+static inline string_field_s *
+string_field (const char *name, const char *str,
+ string_field_s &&tmp = {})
+{
+ tmp.name = name;
+ tmp.kind = field_kind::STRING;
+ tmp.str = str;
+ return &tmp;
+}
+
/* A styled string. */
struct styled_string_s