[PATCH] Avoid complaint warning on mingw

[PATCH] Avoid complaint warning on mingw

[Tom Tromey]

The mingw build currently issues a warning:

./../../src/gdb/utils.h:378:56: warning: ignoring attributes on template argument 'void(const char*, va_list)' {aka 'void(const char*, char*)'} [-Wignored-attributes]

This patch fixes the problem as suggested by Simon:

    https://sourceware.org/pipermail/gdb-patches/2024-April/207908.html

...that is, by changing the warning interceptor to a class with a
single 'warn' method.
---
 gdb/complaints.c          |  8 ++++----
 gdb/complaints.h          |  6 +++---
 gdb/dwarf2/cooked-index.c |  2 +-
 gdb/utils.c               |  2 +-
 gdb/utils.h               | 28 ++++++++++++++++------------
 5 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/gdb/complaints.c b/gdb/complaints.c
index e375c734884..d3c72df6d41 100644
--- a/gdb/complaints.c
+++ b/gdb/complaints.c
@@ -58,7 +58,7 @@ complaint_internal (const char *fmt, ...)
 
   warning_hook_handler handler = get_warning_hook_handler ();
   if (handler != nullptr)
-    handler (fmt, args);
+    handler->warn (fmt, args);
   else
     {
       gdb_puts (_("During symbol reading: "), gdb_stderr);
@@ -85,7 +85,7 @@ thread_local complaint_interceptor *complaint_interceptor::g_complaint_intercept
 
 complaint_interceptor::complaint_interceptor ()
   : m_saved_complaint_interceptor (&g_complaint_interceptor, this),
-    m_saved_warning_hook (issue_complaint)
+    m_saved_warning_hook (this)
 {
 }
 
@@ -96,7 +96,7 @@ wrap_warning_hook (warning_hook_handler hook, ...)
 {
   va_list args;
   va_start (args, hook);
-  hook ("%s", args);
+  hook->warn ("%s", args);
   va_end (args);
 }
 
@@ -121,7 +121,7 @@ re_emit_complaints (const complaint_collection &complaints)
 /* See complaints.h.  */
 
 void
-complaint_interceptor::issue_complaint (const char *fmt, va_list args)
+complaint_interceptor::warn (const char *fmt, va_list args)
 {
 #if CXX_STD_THREAD
   std::lock_guard<std::mutex> guard (complaint_mutex);
diff --git a/gdb/complaints.h b/gdb/complaints.h
index 8ac4c5034ee..995c35e0ab0 100644
--- a/gdb/complaints.h
+++ b/gdb/complaints.h
@@ -70,7 +70,7 @@ typedef std::unordered_set<std::string> complaint_collection;
    the main thread).  Messages can be re-emitted on the main thread
    using re_emit_complaints, see below.  */
 
-class complaint_interceptor
+class complaint_interceptor final : public warning_hook_handler_type
 {
 public:
 
@@ -94,8 +94,8 @@ class complaint_interceptor
 
   /* A helper function that is used by the 'complaint' implementation
      to issue a complaint.  */
-  static void issue_complaint (const char *, va_list)
-    ATTRIBUTE_PRINTF (1, 0);
+  void warn (const char *, va_list) override
+    ATTRIBUTE_PRINTF (2, 0);
 
   /* This object.  Used by the static callback function.  */
   static thread_local complaint_interceptor *g_complaint_interceptor;
diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 66822b575ca..cb0d1f3cd47 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -589,7 +589,7 @@ cooked_index_worker::write_to_cache (const cooked_index *idx,
 	 See PR symtab/30837.  This arranges to capture all such
 	 warnings.  This is safe because we know the deferred_warnings
 	 object isn't in use by any other thread at this point.  */
-      scoped_restore_warning_hook defer (*warn);
+      scoped_restore_warning_hook defer (warn);
       m_cache_store.store ();
     }
 }
diff --git a/gdb/utils.c b/gdb/utils.c
index ded03c74099..6a77f6be713 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -164,7 +164,7 @@ void
 vwarning (const char *string, va_list args)
 {
   if (warning_hook != nullptr)
-    warning_hook (string, args);
+    warning_hook->warn (string, args);
   else
     {
       std::optional<target_terminal::scoped_restore_terminal_state> term_state;
diff --git a/gdb/utils.h b/gdb/utils.h
index 875a2583179..65882363e3c 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -374,8 +374,19 @@ assign_return_if_changed (T &lval, const T &val)
   return true;
 }
 
-/* A function that can be used to intercept warnings.  */
-typedef gdb::function_view<void (const char *, va_list)> warning_hook_handler;
+/* A class that can be used to intercept warnings.  A class is used
+   here, rather than a gdb::function_view because it proved difficult
+   to use a function view in conjunction with ATTRIBUTE_PRINTF in a
+   way that would satisfy all compilers on all systems.  And, even
+   though gdb only ever uses deferred_warnings here, a virtual
+   function is used to help Insight.  */
+struct warning_hook_handler_type
+{
+  virtual void warn (const char *format, va_list args) ATTRIBUTE_PRINTF (2, 0)
+    = 0;
+};
+
+typedef warning_hook_handler_type *warning_hook_handler;
 
 /* Set the thread-local warning hook, and restore the old value when
    finished.  */
@@ -418,7 +429,7 @@ extern warning_hook_handler get_warning_hook_handler ();
    instance of this class with the 'warn' function, and all warnings can be
    emitted with a single call to 'emit'.  */
 
-struct deferred_warnings
+struct deferred_warnings final : public warning_hook_handler_type
 {
   deferred_warnings ()
     : m_can_style (gdb_stderr->can_emit_style_escape ())
@@ -444,18 +455,11 @@ struct deferred_warnings
      hook; see scoped_restore_warning_hook.  Note that no locking is
      done, so users have to be careful to only install this into a
      single thread at a time.  */
-  void operator() (const char *format, va_list args)
+  void warn (const char *format, va_list args) override
+    ATTRIBUTE_PRINTF (2, 0)
   {
     string_file msg (m_can_style);
-    /* Clang warns if we add ATTRIBUTE_PRINTF to this method (because
-       the function-view wrapper doesn't also have the attribute), but
-       then warns again if we remove it, because this vprintf call
-       does not use a literal format string.  So, suppress the
-       warnings here.  */
-    DIAGNOSTIC_PUSH
-    DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
     msg.vprintf (format, args);
-    DIAGNOSTIC_POP
     m_warnings.emplace_back (std::move (msg));
   }
 
-- 
2.43.0

Re: [PATCH] Avoid complaint warning on mingw

[Simon Marchi]

On 4/9/24 11:46 AM, Tom Tromey wrote:
> @@ -418,7 +429,7 @@ extern warning_hook_handler get_warning_hook_handler ();
>     instance of this class with the 'warn' function, and all warnings can be
>     emitted with a single call to 'emit'.  */
>  
> -struct deferred_warnings
> +struct deferred_warnings final : public warning_hook_handler_type
>  {
>    deferred_warnings ()
>      : m_can_style (gdb_stderr->can_emit_style_escape ())
> @@ -444,18 +455,11 @@ struct deferred_warnings
>       hook; see scoped_restore_warning_hook.  Note that no locking is
>       done, so users have to be careful to only install this into a
>       single thread at a time.  */
> -  void operator() (const char *format, va_list args)
> +  void warn (const char *format, va_list args) override
> +    ATTRIBUTE_PRINTF (2, 0)

The `warn (const char *format, ...)` method should probably defer to the
`warn (const char *format, va_list args)` method, they appear to do the
exact same thing.

Otherwise, LGTM:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon

Re: [PATCH] Avoid complaint warning on mingw

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> The `warn (const char *format, ...)` method should probably defer to the
Simon> `warn (const char *format, va_list args)` method, they appear to do the
Simon> exact same thing.

I made this change and I'm going to check it in.

Now I've found another clang build problem related to another patch of
mine, I'll send a fix shortly.

Tom