diff mbox series

[03/24] gdb: make interps_notify work with references

Message ID	20231010204213.111285-4-simon.marchi@efficios.com
State	New
Headers	DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3E5003858C52 From: Simon Marchi <simon.marchi@efficios.com> To: gdb-patches@sourceware.org Cc: Simon Marchi <simon.marchi@polymtl.ca> Subject: [PATCH 03/24] gdb: make interps_notify work with references Date: Tue, 10 Oct 2023 16:39:58 -0400 Message-ID: <20231010204213.111285-4-simon.marchi@efficios.com> In-Reply-To: <20231010204213.111285-1-simon.marchi@efficios.com> References: <20231010204213.111285-1-simon.marchi@efficios.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: list Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org
Series	C++ification of struct so_list \| [00/24] C++ification of struct so_list [01/24] gdb: remove empty clear_solib functions [02/24] gdb: add program_space parameter to target_so_ops::clear_solib [03/24] gdb: make interps_notify work with references [04/24] gdb: replace some so_list parameters to use references [05/24] gdbsupport: use "reference" and "pointer" type aliases in intrusive_list [06/24] gdbsupport: make intrusive_list's disposer accept a reference [07/24] gdb: make get_cbfd_soname_build_id static [08/24] gdb: allocate so_list with new, deallocate with delete [09/24] gdb: rename lm_info_base to lm_info [10/24] gdb: remove target_so_ops::free_so [11/24] gdb: use gdb::checked_static_cast when casting lm_info [12/24] gdb: make solib-svr4 not use so_list internally [13/24] gdb: make solib-rocm not use so_list internally [14/24] gdb: remove lm_info_vector typedef [15/24] gdb: make so_list::lm_info a unique_ptr [16/24] gdb: make clear_so a method of struct so_list [17/24] gdb: remove target_section_table typedef [18/24] gdb: make so_list::sections not a pointer [19/24] gdb: make so_list::abfd a gdb_bfd_ref_ptr [20/24] gdb: make so_list::{so_original_name, so_name} std::strings [21/24] gdb: link so_list using intrusive_list [22/24] gdb: don't call so_list::clear in free_so [23/24] gdb: remove free_so function [24/24] gdb: rename struct so_list to so

Commit Message

Simon Marchi Oct. 10, 2023, 8:39 p.m. UTC

  From: Simon Marchi <simon.marchi@polymtl.ca>

A subsequent patch changes the interp::on_solib_loaded and
interp::on_solib_unloaded methods to take references.  This highlighted
that interps_notify did not work with reference parameters.

Fix that by changing interps_notify's `args` arg to be a universal
reference (&&).  Change the type of the method to be auto-deduced as an
additional template parameter, otherwise the signature of the callback
function would never match:

      CXX    interps.o
    /home/simark/src/binutils-gdb/gdb/interps.c: In function ‘void interps_notify_signal_received(gdb_signal)’:
    /home/simark/src/binutils-gdb/gdb/interps.c:378:18: error: no matching function for call to ‘interps_notify(void (interp::*)(gdb_signal), gdb_signal&)’
      378 |   interps_notify (&interp::on_signal_received, sig);
          |   ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    /home/simark/src/binutils-gdb/gdb/interps.c:363:1: note: candidate: ‘template<class ... Args> void interps_notify(void (interp::*)(Args ...), Args&& ...)’
      363 | interps_notify (void (interp::*method) (Args...), Args&&... args)
          | ^~~~~~~~~~~~~~
    /home/simark/src/binutils-gdb/gdb/interps.c:363:1: note:   template argument deduction/substitution failed:
    /home/simark/src/binutils-gdb/gdb/interps.c:378:18: note:   inconsistent parameter pack deduction with ‘gdb_signal’ and ‘gdb_signal&’
      378 |   interps_notify (&interp::on_signal_received, sig);
          |   ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Change-Id: I0cd9378e24ef039f30f8e14f054f8d7fb539c838
---
 gdb/interps.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Lancelot SIX Oct. 11, 2023, 8:48 a.m. UTC | #1

Hi Simon,

Just a nit below.

On Tue, Oct 10, 2023 at 04:39:58PM -0400, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> A subsequent patch changes the interp::on_solib_loaded and
> interp::on_solib_unloaded methods to take references.  This highlighted
> that interps_notify did not work with reference parameters.
> 
> Fix that by changing interps_notify's `args` arg to be a universal
> reference (&&).  Change the type of the method to be auto-deduced as an
> additional template parameter, otherwise the signature of the callback
> function would never match:
> 
>       CXX    interps.o
>     /home/simark/src/binutils-gdb/gdb/interps.c: In function ‘void interps_notify_signal_received(gdb_signal)’:
>     /home/simark/src/binutils-gdb/gdb/interps.c:378:18: error: no matching function for call to ‘interps_notify(void (interp::*)(gdb_signal), gdb_signal&)’
>       378 |   interps_notify (&interp::on_signal_received, sig);
>           |   ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     /home/simark/src/binutils-gdb/gdb/interps.c:363:1: note: candidate: ‘template<class ... Args> void interps_notify(void (interp::*)(Args ...), Args&& ...)’
>       363 | interps_notify (void (interp::*method) (Args...), Args&&... args)
>           | ^~~~~~~~~~~~~~
>     /home/simark/src/binutils-gdb/gdb/interps.c:363:1: note:   template argument deduction/substitution failed:
>     /home/simark/src/binutils-gdb/gdb/interps.c:378:18: note:   inconsistent parameter pack deduction with ‘gdb_signal’ and ‘gdb_signal&’
>       378 |   interps_notify (&interp::on_signal_received, sig);
>           |   ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Change-Id: I0cd9378e24ef039f30f8e14f054f8d7fb539c838
> ---
>  gdb/interps.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/interps.c b/gdb/interps.c
> index f91b2b62c1ba..62a30583e8c0 100644
> --- a/gdb/interps.c
> +++ b/gdb/interps.c
> @@ -358,15 +358,15 @@ current_interpreter (void)
>  /* Helper interps_notify_* functions.  Call METHOD on the top-level interpreter
>     of all UIs.  */
>  
> -template <typename ...Args>
> +template <typename MethodType, typename ...Args>
>  void
> -interps_notify (void (interp::*method) (Args...), Args... args)
> +interps_notify (MethodType method, Args&&... args)
>  {
>    SWITCH_THRU_ALL_UIS ()
>      {
>        interp *tli = top_level_interpreter ();
>        if (tli != nullptr)
> -	(tli->*method) (args...);
> +	(tli->*method) (std::forward<Args>(args)...);
                                          ^

Space missing before the "(".

Best,
Lancelot.

>      }
>  }
>  
> -- 
> 2.42.0
>

Simon Marchi Oct. 11, 2023, 2:18 p.m. UTC | #2

On 10/11/23 04:48, Lancelot SIX wrote:
>> diff --git a/gdb/interps.c b/gdb/interps.c
>> index f91b2b62c1ba..62a30583e8c0 100644
>> --- a/gdb/interps.c
>> +++ b/gdb/interps.c
>> @@ -358,15 +358,15 @@ current_interpreter (void)
>>  /* Helper interps_notify_* functions.  Call METHOD on the top-level interpreter
>>     of all UIs.  */
>>  
>> -template <typename ...Args>
>> +template <typename MethodType, typename ...Args>
>>  void
>> -interps_notify (void (interp::*method) (Args...), Args... args)
>> +interps_notify (MethodType method, Args&&... args)
>>  {
>>    SWITCH_THRU_ALL_UIS ()
>>      {
>>        interp *tli = top_level_interpreter ();
>>        if (tli != nullptr)
>> -	(tli->*method) (args...);
>> +	(tli->*method) (std::forward<Args>(args)...);
>                                           ^
> 
> Space missing before the "(".
> 
> Best,
> Lancelot.

Thanks, fixed locally.

Simon

diff mbox series

Patch

diff --git a/gdb/interps.c b/gdb/interps.c
index f91b2b62c1ba..62a30583e8c0 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -358,15 +358,15 @@  current_interpreter (void)
 /* Helper interps_notify_* functions.  Call METHOD on the top-level interpreter
    of all UIs.  */
 
-template <typename ...Args>
+template <typename MethodType, typename ...Args>
 void
-interps_notify (void (interp::*method) (Args...), Args... args)
+interps_notify (MethodType method, Args&&... args)
 {
   SWITCH_THRU_ALL_UIS ()
     {
       interp *tli = top_level_interpreter ();
       if (tli != nullptr)
-	(tli->*method) (args...);
+	(tli->*method) (std::forward<Args>(args)...);
     }
 }