From patchwork Wed Jun 5 20:39:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 33031 Received: (qmail 34442 invoked by alias); 5 Jun 2019 20:39:44 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 34431 invoked by uid 89); 5 Jun 2019 20:39:44 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_STOCKGEN, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=2069, STYLE, 598 X-HELO: mail-wr1-f67.google.com Received: from mail-wr1-f67.google.com (HELO mail-wr1-f67.google.com) (209.85.221.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 05 Jun 2019 20:39:42 +0000 Received: by mail-wr1-f67.google.com with SMTP id d18so123716wrs.5 for ; Wed, 05 Jun 2019 13:39:41 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:4eeb:42ff:feef:f164? ([2001:8a0:f913:f700:4eeb:42ff:feef:f164]) by smtp.gmail.com with ESMTPSA id z135sm5651865wmc.45.2019.06.05.13.39.38 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Wed, 05 Jun 2019 13:39:39 -0700 (PDT) Subject: Re: ui_out format strings for fields and styles (Re: [PATCH] Style "pwd" output) To: Tom Tromey References: <20190605020116.1550-1-tom@tromey.com> <1ee4bd6b-4cdf-f3a9-74af-0843bf123a8b@redhat.com> <87lfygi1x0.fsf@tromey.com> <32872d6a-15d6-9718-59ae-957694e114c9@redhat.com> <87imtjhj6b.fsf@tromey.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <625cd0ba-058d-d4bf-8ba3-8676f335b0f3@redhat.com> Date: Wed, 5 Jun 2019 21:39:38 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <87imtjhj6b.fsf@tromey.com> On 6/5/19 9:27 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves 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 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. */