diff mbox series

[2/2] gdb: remove complaint_interceptor::g_complaint_interceptor

Message ID 20260521040112.1618748-2-simon.marchi@polymtl.ca
State New
Headers
Series [1/2] gdb: lock complaint_mutex in clear_complaints |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Simon Marchi May 21, 2026, 4 a.m. UTC 
  From: Simon Marchi <simon.marchi@polymtl.ca>

The thread_local g_complaint_interceptor pointer is unnecessary.  The
complaint_interceptor constructor registers itself as the warning hook
via m_saved_warning_hook (this), so when complaint_internal dispatches
through the warning hook, it lands in complaint_interceptor::warn with
'this' already pointing at the registered interceptor.  Inside warn,
g_complaint_interceptor and 'this' always refer to the same object.

Replace g_complaint_interceptor->m_complaints with m_complaints in
complaint_interceptor::warn and remove g_complaint_interceptor.

Change-Id: I75565a5f2c0e51363f36be0e3544210c10bb5491
---
 gdb/complaints.c | 9 ++-------
 gdb/complaints.h | 7 -------
 2 files changed, 2 insertions(+), 14 deletions(-)

Comments

Tom Tromey May 21, 2026, 2:34 p.m. UTC | #1 
>>>>> "Simon" == simon marchi <simon.marchi@polymtl.ca> writes:

Simon> From: Simon Marchi <simon.marchi@polymtl.ca>
Simon> The thread_local g_complaint_interceptor pointer is unnecessary.  The
Simon> complaint_interceptor constructor registers itself as the warning hook
Simon> via m_saved_warning_hook (this), so when complaint_internal dispatches
Simon> through the warning hook, it lands in complaint_interceptor::warn with
Simon> 'this' already pointing at the registered interceptor.  Inside warn,
Simon> g_complaint_interceptor and 'this' always refer to the same object.

Simon> Replace g_complaint_interceptor->m_complaints with m_complaints in
Simon> complaint_interceptor::warn and remove g_complaint_interceptor.

Ok.
Approved-By: Tom Tromey <tom@tromey.com>

I don't remember the code too well but I do wonder if we should just
have each set of complaints be completely independent and emit them all
at some spot guaranteed to be on the main thread.  Like, instead of
'complaint' function, only expose a method on some object.

OTOH I don't think we should spend any more time on complaints than is
strictly necessary.

Tom
Simon Marchi May 21, 2026, 5:51 p.m. UTC | #2 
On 5/21/26 10:34 AM, Tom Tromey wrote:
>>>>>> "Simon" == simon marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> From: Simon Marchi <simon.marchi@polymtl.ca>
> Simon> The thread_local g_complaint_interceptor pointer is unnecessary.  The
> Simon> complaint_interceptor constructor registers itself as the warning hook
> Simon> via m_saved_warning_hook (this), so when complaint_internal dispatches
> Simon> through the warning hook, it lands in complaint_interceptor::warn with
> Simon> 'this' already pointing at the registered interceptor.  Inside warn,
> Simon> g_complaint_interceptor and 'this' always refer to the same object.
> 
> Simon> Replace g_complaint_interceptor->m_complaints with m_complaints in
> Simon> complaint_interceptor::warn and remove g_complaint_interceptor.
> 
> Ok.
> Approved-By: Tom Tromey <tom@tromey.com>

Thanks, pushed both patches.

> I don't remember the code too well but I do wonder if we should just
> have each set of complaints be completely independent and emit them all
> at some spot guaranteed to be on the main thread.  Like, instead of
> 'complaint' function, only expose a method on some object.

I'm afraid I don't understand your suggestion.

> OTOH I don't think we should spend any more time on complaints than is
> strictly necessary.

Agreed.

Simon
diff mbox series

Patch

diff --git a/gdb/complaints.c b/gdb/complaints.c
index ab6e2049685c..e3d68a869c99 100644
--- a/gdb/complaints.c
+++ b/gdb/complaints.c
@@ -77,13 +77,8 @@  clear_complaints ()
 
 /* See complaints.h.  */
 
-thread_local complaint_interceptor *complaint_interceptor::g_complaint_interceptor;
-
-/* See complaints.h.  */
-
 complaint_interceptor::complaint_interceptor ()
-  : m_saved_complaint_interceptor (&g_complaint_interceptor, this),
-    m_saved_warning_hook (this)
+  : m_saved_warning_hook (this)
 {
 }
 
@@ -122,7 +117,7 @@  void
 complaint_interceptor::warn (const char *fmt, va_list args)
 {
   gdb::lock_guard<gdb::mutex> guard (complaint_mutex);
-  g_complaint_interceptor->m_complaints.insert (string_vprintf (fmt, args));
+  m_complaints.insert (string_vprintf (fmt, args));
 }
 
 static void
diff --git a/gdb/complaints.h b/gdb/complaints.h
index c607194e265e..8f5cf24c1c9b 100644
--- a/gdb/complaints.h
+++ b/gdb/complaints.h
@@ -20,7 +20,6 @@ 
 #ifndef GDB_COMPLAINTS_H
 #define GDB_COMPLAINTS_H
 
-#include "gdbsupport/scoped_restore.h"
 #include "gdbsupport/unordered_set.h"
 
 /* Helper for complaint.  */
@@ -89,17 +88,11 @@  class complaint_interceptor final : public warning_hook_handler_type
   /* The issued complaints.  */
   complaint_collection m_complaints;
 
-  /* The saved value of g_complaint_interceptor.  */
-  scoped_restore_tmpl<complaint_interceptor *> m_saved_complaint_interceptor;
-
   /* A helper function that is used by the 'complaint' implementation
      to issue a complaint.  */
   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;
-
   /* Object to initialise the warning hook.  */
   scoped_restore_warning_hook m_saved_warning_hook;
 };