Re: [PATCH v3 3/3] gdb/amdgpu: add precise-memory support

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 9/15/23 12:04, Lancelot SIX wrote:
> Hi Simon,
> 
> It looks like your latest change is not c++11 compliant (see below).
> 
> I do have a WIP branch to bump the C++ version to C++17
> (users/lsix/try-c++17).  Maybe it's time I write proper commit messages
> and send this as RFC, see if there is a concensis to allow c++17.

Thanks for pointing this out.

As far as I know, members of the community need to support building with
gcc 4.8 for a while still.  But there's nothing wrong sending an RFC to
get an updated status on the situation.

> 
>> diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
>> index 22c269b7992c..507824decf8d 100644
>> --- a/gdb/amd-dbgapi-target.c
>> +++ b/gdb/amd-dbgapi-target.c
>> @@ -116,8 +117,9 @@ get_amd_dbgapi_target_inferior_created_observer_token ()
>>  
>>  struct amd_dbgapi_inferior_info
>>  {
>> -  explicit amd_dbgapi_inferior_info (inferior *inf)
>> -    : inf (inf)
>> +  explicit amd_dbgapi_inferior_info (inferior *inf,
>> +				     bool precise_memory_requested = false)
>> +    : inf {inf}, precise_memory {precise_memory_requested}
>>    {}
> 
> If I build using -std=c++11 I get the following error:
> 
> ../../gdb/amd-dbgapi-target.c: In constructor ‘amd_dbgapi_inferior_info::amd_dbgapi_inferior_info(inferior*, bool)’:
> ../../gdb/amd-dbgapi-target.c:122:18: error: no matching function for call to ‘amd_dbgapi_inferior_info::<unnamed struct>::._anon_207(<brace-enclosed initializer list>)’
>   122 |     : inf {inf}, precise_memory {precise_memory_requested}
>       |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../../gdb/amd-dbgapi-target.c:145:3: note: candidate: ‘amd_dbgapi_inferior_info::<unnamed struct>::<constructor>()’
>   145 |   {
>       |   ^
> 
> In c++17 mode, it works fine though…
> 
> Moving the initialization in the ctor's body would solve the issue
> 
>   explicit amd_dbgapi_inferior_info (inferior *inf,
>                                      bool precise_memory_requested = false)
>     : inf {inf}
>   {
>     precise_memory.requested = precise_memory_requested;
>   }
> 
> The alternative would be to name
> amd_dbgapi_inferior_info::precise_memory's type and give it a ctor.

In this case I would just do:

diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
index 507824decf8d..40baf156d83f 100644
--- a/gdb/amd-dbgapi-target.c
+++ b/gdb/amd-dbgapi-target.c
@@ -119,8 +119,10 @@ struct amd_dbgapi_inferior_info
 {
   explicit amd_dbgapi_inferior_info (inferior *inf,
 				     bool precise_memory_requested = false)
-    : inf {inf}, precise_memory {precise_memory_requested}
-  {}
+    : inf (inf)
+  {
+    precise_memory.requested = precise_memory_requested;
+  }

   /* Backlink to inferior.  */
   inferior *inf;

---

Or, we can just have two separate fields (precise_memory_requested and
precise_memory_enabled), they don't really need to be in a struct.

Simon