On 7/1/19 1:55 PM, Pedro Alves wrote:
> On 7/1/19 1:23 PM, Pedro Alves wrote:
>>> What's left to do on that branch in order to merge it?
>> Well, you used printf_filtered the new patches, but that doesn't work
>> with the new format specifiers, yet. :-)
>
> 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.
So those "ptr ()" calls/members are there because int_field/styled_string
are ctors, and, we can't pass references via va_list. We can get rid of those,
if we replace the ctors with functions that allocate a temporary on the
callers stack, like:
static inline styled_string_s *
styled_string (const ui_file_style &style, const char *str,
styled_string_s &&tmp = {})
{
tmp.style = style;
tmp.str = str;
return &tmp;
}
Actually, what I originally thought to try, was to make it
as short as possible to write that "styled_string" expression.
I even experimented with renaming that styled_string function
above to "ss", resulting in:
printf_filtered (": file %ps, line %d.",
ss (file_name_style.style (), filename),
b->loc->line_number);
but I reverted it, thinking that I was maybe overdoing it.
We'll still need to use .ptr() with:
fprintf_filtered (stream, " %pS<repeats %u times>%pS",
metadata_style.style ().ptr (), reps, nullptr);
(whatever the formatter spelling we end up with)
unless we do make style() return a pointer.
Here's a patch implementing the idea above. I wrote this a couple
weeks ago, and at the time I felt more strongly about it. Is this
worth it?
From fd5bce2ea4716552ce9c3f9fdac7628cbf5a5cd2 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 7 Jun 2019 22:42:57 +0100
Subject: [PATCH] down with .ptr()
---
gdb/breakpoint.c | 14 ++++++------
gdb/macrocmd.c | 2 +-
gdb/printcmd.c | 2 +-
gdb/symfile.c | 8 +++----
gdb/symtab.c | 6 +++---
gdb/ui-out.c | 8 +++----
gdb/ui-out.h | 65 ++++++++++++++++++++++++++++++--------------------------
7 files changed, 54 insertions(+), 51 deletions(-)
@@ -4930,7 +4930,7 @@ watchpoint_check (bpstat bs)
uiout->message ("\nWatchpoint %pF deleted because the program has "
"left the block in\n"
"which its expression is valid.\n",
- int_field ("wpnum", b->number).ptr ());
+ int_field ("wpnum", b->number));
}
/* Make sure the watchpoint's commands aren't executed. */
@@ -6246,7 +6246,7 @@ print_one_breakpoint_location (struct breakpoint *b,
{
annotate_field (8);
uiout->message ("\tignore next %pF hits\n",
- int_field ("ignore", b->ignore_count).ptr ());
+ int_field ("ignore", b->ignore_count));
}
/* Note that an enable count of 1 corresponds to "enable once"
@@ -6695,7 +6695,7 @@ describe_other_breakpoints (struct gdbarch *gdbarch,
}
current_uiout->message (_("also set at pc %ps.\n"),
styled_string (address_style.style (),
- paddress (gdbarch, pc)).ptr ());
+ paddress (gdbarch, pc)));
}
}
@@ -12115,7 +12115,7 @@ say_where (struct breakpoint *b)
printf_filtered (" at %ps",
styled_string (address_style.style (),
paddress (b->loc->gdbarch,
- b->loc->address)).ptr ());
+ b->loc->address)));
if (b->loc->symtab != NULL)
{
/* If there is a single location, we can print the location
@@ -12126,7 +12126,7 @@ say_where (struct breakpoint *b)
= symtab_to_filename_for_display (b->loc->symtab);
printf_filtered (": file %ps, line %d.",
styled_string (file_name_style.style (),
- filename).ptr (),
+ filename),
b->loc->line_number);
}
else
@@ -12433,10 +12433,10 @@ bkpt_print_it (bpstat bs)
}
if (bp_temp)
uiout->message ("Temporary breakpoint %pF, ",
- int_field ("bkptno", b->number).ptr ());
+ int_field ("bkptno", b->number));
else
uiout->message ("Breakpoint %pF, ",
- int_field ("bkptno", b->number).ptr ());
+ int_field ("bkptno", b->number));
return PRINT_SRC_AND_LOC;
}
@@ -122,7 +122,7 @@ show_pp_source_pos (struct ui_file *stream,
std::string fullname = macro_source_fullname (file);
fprintf_filtered (stream, "%ps:%d\n",
styled_string (file_name_style.style (),
- fullname.c_str ()).ptr (),
+ fullname.c_str ()),
line);
while (file->included_by)
@@ -2190,7 +2190,7 @@ print_variable_and_value (const char *name, struct symbol *var,
name = SYMBOL_PRINT_NAME (var);
fprintf_filtered (stream, "%s%ps = ", n_spaces (2 * indent),
- styled_string (variable_name_style.style (), name).ptr ());
+ styled_string (variable_name_style.style (), name));
try
{
@@ -1117,7 +1117,7 @@ symbol_file_add_with_addrs (bfd *abfd, const char *name,
else
current_uiout->message (_("Reading symbols from %ps...\n"),
styled_string (file_name_style.style (),
- name).ptr ());
+ name));
}
syms_from_objfile (objfile, addrs, add_flags);
@@ -1130,8 +1130,7 @@ symbol_file_add_with_addrs (bfd *abfd, const char *name,
{
if (should_print)
current_uiout->message (_("Expanding full symbols from %ps...\n"),
- styled_string (file_name_style.style (),
- name).ptr ());
+ styled_string (file_name_style.style (), name));
if (objfile->sf)
objfile->sf->qf->expand_all_symtabs (objfile);
@@ -1144,8 +1143,7 @@ symbol_file_add_with_addrs (bfd *abfd, const char *name,
if (should_print && !objfile_has_symbols (objfile)
&& objfile->separate_debug_objfile == nullptr)
current_uiout->message (_("(No debugging symbols found in %ps)\n"),
- styled_string (file_name_style.style (),
- name).ptr ());
+ styled_string (file_name_style.style (), name));
if (should_print)
{
@@ -4605,7 +4605,7 @@ print_symbol_info (enum search_domain kind,
{
current_uiout->message
(_("\nFile %ps:\n"),
- styled_string (file_name_style.style (), s_filename).ptr ());
+ styled_string (file_name_style.style (), s_filename));
}
if (SYMBOL_LINE (sym) != 0)
@@ -4658,8 +4658,8 @@ print_msymbol_info (struct bound_minimal_symbol msymbol)
current_uiout->message
(_("%ps %ps\n"),
- styled_string (address_style.style (), tmp).ptr (),
- styled_string (sym_style, MSYMBOL_PRINT_NAME (msymbol.minsym)).ptr ());
+ styled_string (address_style.style (), tmp),
+ styled_string (sym_style, MSYMBOL_PRINT_NAME (msymbol.minsym)));
}
/* This is the guts of the commands "info functions", "info types", and
@@ -607,14 +607,14 @@ ui_out::vmessage (const char *format, va_list args)
{
case 'F':
{
- int_field *field = va_arg (args, int_field *);
- field_int (field->name (), field->val ());
+ int_field_s *field = va_arg (args, int_field_s *);
+ field_int (field->name, field->val);
}
break;
case 's':
{
- styled_string *ss = va_arg (args, styled_string *);
- call_do_message (ss->style (), "%s", ss->str ());
+ styled_string_s *ss = va_arg (args, styled_string_s *);
+ call_do_message (ss->style, "%s", ss->str);
}
break;
case 'S':
@@ -68,45 +68,50 @@ enum ui_out_type
ui_out_type_list
};
-struct int_field
+/* An int field, to be passed to %pF in format strings. */
+
+struct int_field_s
{
- int_field (const char *name, int val)
- : m_name (name),
- m_val (val)
- {
- }
+ const char *name;
+ int val;
+};
- /* We need this because we can't pass a reference via
- va_args. */
- const int_field *ptr () const { return this; }
+/* Construct a temporary int_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. */
- const char *name () const {return m_name; }
- int val () const {return m_val; }
+static inline int_field_s *
+int_field (const char *name, int val,
+ int_field_s &&tmp = {})
+{
+ tmp.name = name;
+ tmp.val = val;
+ return &tmp;
+}
-private:
- const char *m_name;
- int m_val;
-};
+/* A styled string. */
-struct styled_string
+struct styled_string_s
{
- styled_string (const ui_file_style &style, const char *str)
- : m_style (style),
- m_str (str)
- {
- }
+ /* The style. */
+ ui_file_style style;
- /* We need this because we can't pass a reference via
- va_args. */
- const styled_string *ptr () const { return this; }
+ /* The string. */
+ const char *str;
+};
- const ui_file_style &style () const { return m_style; }
- const char *str () const {return m_str; }
+/* Construct a temporary styled_string_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. */
-private:
- ui_file_style m_style;
- const char *m_str;
-};
+static inline styled_string_s *
+styled_string (const ui_file_style &style, const char *str,
+ styled_string_s &&tmp = {})
+{
+ tmp.style = style;
+ tmp.str = str;
+ return &tmp;
+}
class ui_out
{