[PATCH] [gdb/build] Fix build breaker with -std=c++11

[PATCH] [gdb/build] Fix build breaker with -std=c++11

[Tom de Vries]

When building with -std=c++11 I run into:
...
gdb/dwarf2/cooked-index.c: In member function \
  ‘void cooked_index::start_writing_index(dwarf2_per_bfd*)’:
gdb/dwarf2/cooked-index.c:469:10: error: lambda capture initializers only \
  available with -std=c++14 or -std=gnu++14 [-Werror]
          ctx = std::move (ctx)] ()
          ^~~
...

Fix this by capturing a copy instead:
...
    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd, ctx] ()
...

Tested on x86_64-linux.

Reported-By: Tom Tromey <tom@tromey.com>
---
 gdb/dwarf2/cooked-index.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index df82e86b690..87aac5aaa8b 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -465,8 +465,7 @@ cooked_index::start_writing_index (dwarf2_per_bfd *per_bfd)
   /* This must be set after all the finalization tasks have been
      started, because it may call 'wait'.  */
   m_write_future
-    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd,
-						   ctx = std::move (ctx)] ()
+    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd, ctx] ()
 	{
 	  maybe_write_index (per_bfd, ctx);
 	});

base-commit: 5e66f55c62e306afbcc93856bf06e542ddd00997
-- 
2.35.3

Re: [PATCH] [gdb/build] Fix build breaker with -std=c++11

[Simon Marchi]

On August 6, 2023 7:59:15 p.m. EDT, Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> wrote:
>When building with -std=c++11 I run into:
>...
>gdb/dwarf2/cooked-index.c: In member function \
>  ‘void cooked_index::start_writing_index(dwarf2_per_bfd*)’:
>gdb/dwarf2/cooked-index.c:469:10: error: lambda capture initializers only \
>  available with -std=c++14 or -std=gnu++14 [-Werror]
>          ctx = std::move (ctx)] ()
>          ^~~
>...
>
>Fix this by capturing a copy instead:
>...
>    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd, ctx] ()
>...
>
>Tested on x86_64-linux.
>
>Reported-By: Tom Tromey <tom@tromey.com>

When we encounter things like this, should we add some greppable TODO comment to record an actionable item for when we switch to a more recent C++ version? 

Simon 

Re: [PATCH] [gdb/build] Fix build breaker with -std=c++11

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 966 bytes --]

On 8/7/23 16:35, Simon Marchi wrote:
> 
> 
> On August 6, 2023 7:59:15 p.m. EDT, Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> wrote:
>> When building with -std=c++11 I run into:
>> ...
>> gdb/dwarf2/cooked-index.c: In member function \
>>   ‘void cooked_index::start_writing_index(dwarf2_per_bfd*)’:
>> gdb/dwarf2/cooked-index.c:469:10: error: lambda capture initializers only \
>>   available with -std=c++14 or -std=gnu++14 [-Werror]
>>           ctx = std::move (ctx)] ()
>>           ^~~
>> ...
>>
>> Fix this by capturing a copy instead:
>> ...
>>     = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd, ctx] ()
>> ...
>>
>> Tested on x86_64-linux.
>>
>> Reported-By: Tom Tromey <tom@tromey.com>
> 
> When we encounter things like this, should we add some greppable TODO comment to record an actionable item for when we switch to a more recent C++ version?

Hi Simon,

thanks for the review.

Fine by me, added comment.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-build-Fix-build-breaker-with-std-c-11.patch --]
[-- Type: text/x-patch, Size: 1845 bytes --]

From ef0b55aaf4eaa9a1efece7656df1a8a997f97412 Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Mon, 7 Aug 2023 00:31:04 +0200
Subject: [PATCH] [gdb/build] Fix build breaker with -std=c++11
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When building with -std=c++11 I run into:
...
gdb/dwarf2/cooked-index.c: In member function \
  ‘void cooked_index::start_writing_index(dwarf2_per_bfd*)’:
gdb/dwarf2/cooked-index.c:469:10: error: lambda capture initializers only \
  available with -std=c++14 or -std=gnu++14 [-Werror]
          ctx = std::move (ctx)] ()
          ^~~
...

Fix this by capturing a copy instead:
...
    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd, ctx] ()
...

Tested on x86_64-linux.

Reported-By: Tom Tromey <tom@tromey.com>
---
 gdb/dwarf2/cooked-index.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index df82e86b690..775b1383ce5 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -463,10 +463,11 @@ cooked_index::start_writing_index (dwarf2_per_bfd *per_bfd)
   index_cache_store_context ctx (global_index_cache, per_bfd);
 
   /* This must be set after all the finalization tasks have been
-     started, because it may call 'wait'.  */
+     started, because it may call 'wait'.
+     TODO (c++14): The capture of ctx here can be rewritten to
+     "ctx = std::move (ctx)" once the standard is updated to c++14.  */
   m_write_future
-    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd,
-						   ctx = std::move (ctx)] ()
+    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd, ctx] ()
 	{
 	  maybe_write_index (per_bfd, ctx);
 	});

base-commit: 8d27b09d0877982dc2252c01a3f1d9c631b2d6eb
-- 
2.35.3

Re: [PATCH] [gdb/build] Fix build breaker with -std=c++11

[John Baldwin]

On 8/7/23 10:15 AM, Tom de Vries via Gdb-patches wrote:
> On 8/7/23 16:35, Simon Marchi wrote:
>>
>>
>> On August 6, 2023 7:59:15 p.m. EDT, Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> wrote:
>>> When building with -std=c++11 I run into:
>>> ...
>>> gdb/dwarf2/cooked-index.c: In member function \
>>>    ‘void cooked_index::start_writing_index(dwarf2_per_bfd*)’:
>>> gdb/dwarf2/cooked-index.c:469:10: error: lambda capture initializers only \
>>>    available with -std=c++14 or -std=gnu++14 [-Werror]
>>>            ctx = std::move (ctx)] ()
>>>            ^~~
>>> ...
>>>
>>> Fix this by capturing a copy instead:
>>> ...
>>>      = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd, ctx] ()
>>> ...
>>>
>>> Tested on x86_64-linux.
>>>
>>> Reported-By: Tom Tromey <tom@tromey.com>
>>
>> When we encounter things like this, should we add some greppable TODO comment to record an actionable item for when we switch to a more recent C++ version?
> 
> Hi Simon,
> 
> thanks for the review.
> 
> Fine by me, added comment.

An alternative that is slightly more work but might be nicer long term would be
to #if on the C++ version, so for example:

#if __cplusplus >= 201402L
   = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd,
                                                 ctx = std::move (ctx)] ()
#else
   = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd, ctx]) ()
#endif

-- 
John Baldwin

Re: [PATCH] [gdb/build] Fix build breaker with -std=c++11

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 1690 bytes --]

On 8/7/23 20:01, John Baldwin wrote:
> On 8/7/23 10:15 AM, Tom de Vries via Gdb-patches wrote:
>> On 8/7/23 16:35, Simon Marchi wrote:
>>>
>>>
>>> On August 6, 2023 7:59:15 p.m. EDT, Tom de Vries via Gdb-patches 
>>> <gdb-patches@sourceware.org> wrote:
>>>> When building with -std=c++11 I run into:
>>>> ...
>>>> gdb/dwarf2/cooked-index.c: In member function \
>>>>    ‘void cooked_index::start_writing_index(dwarf2_per_bfd*)’:
>>>> gdb/dwarf2/cooked-index.c:469:10: error: lambda capture initializers 
>>>> only \
>>>>    available with -std=c++14 or -std=gnu++14 [-Werror]
>>>>            ctx = std::move (ctx)] ()
>>>>            ^~~
>>>> ...
>>>>
>>>> Fix this by capturing a copy instead:
>>>> ...
>>>>      = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd, 
>>>> ctx] ()
>>>> ...
>>>>
>>>> Tested on x86_64-linux.
>>>>
>>>> Reported-By: Tom Tromey <tom@tromey.com>
>>>
>>> When we encounter things like this, should we add some greppable TODO 
>>> comment to record an actionable item for when we switch to a more 
>>> recent C++ version?
>>
>> Hi Simon,
>>
>> thanks for the review.
>>
>> Fine by me, added comment.
> 
> An alternative that is slightly more work but might be nicer long term 
> would be
> to #if on the C++ version, so for example:
> 
> #if __cplusplus >= 201402L
>    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd,
>                                                  ctx = std::move (ctx)] ()
> #else
>    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd, ctx]) ()
> #endif
> 

Thanks for the suggestion.  Updated patch attached.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-build-Fix-build-breaker-with-std-c-11.patch --]
[-- Type: text/x-patch, Size: 1622 bytes --]

From ddbe8c3c8a8665e77f9ebb108c5627a6ebecb81f Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Mon, 7 Aug 2023 00:31:04 +0200
Subject: [PATCH] [gdb/build] Fix build breaker with -std=c++11
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When building with -std=c++11 I run into:
...
gdb/dwarf2/cooked-index.c: In member function \
  ‘void cooked_index::start_writing_index(dwarf2_per_bfd*)’:
gdb/dwarf2/cooked-index.c:469:10: error: lambda capture initializers only \
  available with -std=c++14 or -std=gnu++14 [-Werror]
          ctx = std::move (ctx)] ()
          ^~~
...

Fix this by capturing a copy instead when using -std=c++11:
...
    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd, ctx] ()
...

Tested by building with and without -stdc=++11 on x86_64-linux.

Reported-By: Tom Tromey <tom@tromey.com>
---
 gdb/dwarf2/cooked-index.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index df82e86b690..92ef0e35c5e 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -466,7 +466,12 @@ cooked_index::start_writing_index (dwarf2_per_bfd *per_bfd)
      started, because it may call 'wait'.  */
   m_write_future
     = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd,
-						   ctx = std::move (ctx)] ()
+#if __cplusplus >= 201402L
+						   ctx = std::move (ctx)
+#else
+						   ctx
+#endif
+						   ] ()
 	{
 	  maybe_write_index (per_bfd, ctx);
 	});

base-commit: 8d27b09d0877982dc2252c01a3f1d9c631b2d6eb
-- 
2.35.3

Re: [PATCH] [gdb/build] Fix build breaker with -std=c++11

[John Baldwin]

On 8/7/23 4:11 PM, Tom de Vries wrote:
> On 8/7/23 20:01, John Baldwin wrote:
>> On 8/7/23 10:15 AM, Tom de Vries via Gdb-patches wrote:
>>> On 8/7/23 16:35, Simon Marchi wrote:
>>>>
>>>>
>>>> On August 6, 2023 7:59:15 p.m. EDT, Tom de Vries via Gdb-patches
>>>> <gdb-patches@sourceware.org> wrote:
>>>>> When building with -std=c++11 I run into:
>>>>> ...
>>>>> gdb/dwarf2/cooked-index.c: In member function \
>>>>>     ‘void cooked_index::start_writing_index(dwarf2_per_bfd*)’:
>>>>> gdb/dwarf2/cooked-index.c:469:10: error: lambda capture initializers
>>>>> only \
>>>>>     available with -std=c++14 or -std=gnu++14 [-Werror]
>>>>>             ctx = std::move (ctx)] ()
>>>>>             ^~~
>>>>> ...
>>>>>
>>>>> Fix this by capturing a copy instead:
>>>>> ...
>>>>>       = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd,
>>>>> ctx] ()
>>>>> ...
>>>>>
>>>>> Tested on x86_64-linux.
>>>>>
>>>>> Reported-By: Tom Tromey <tom@tromey.com>
>>>>
>>>> When we encounter things like this, should we add some greppable TODO
>>>> comment to record an actionable item for when we switch to a more
>>>> recent C++ version?
>>>
>>> Hi Simon,
>>>
>>> thanks for the review.
>>>
>>> Fine by me, added comment.
>>
>> An alternative that is slightly more work but might be nicer long term
>> would be
>> to #if on the C++ version, so for example:
>>
>> #if __cplusplus >= 201402L
>>     = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd,
>>                                                   ctx = std::move (ctx)] ()
>> #else
>>     = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd, ctx]) ()
>> #endif
>>
> 
> Thanks for the suggestion.  Updated patch attached.

LGTM

Reviewed-by: John Baldwin <jhb@FreeBSD.org>

-- 
John Baldwin

Re: [PATCH] [gdb/build] Fix build breaker with -std=c++11

[Tom Tromey]

Simon> When we encounter things like this, should we add some greppable
Simon> TODO comment to record an actionable item for when we switch to a
Simon> more recent C++ version?

FWIW I know of one other instance of this, in gdb_demangle:

	  std::string copy = name;
	  run_on_main_thread ([=] ()
	    {
	      report_failed_demangle (copy.c_str (), core_dump_allowed,

Here, the name is copied twice, but with move captures there could be
just a single copy.

I wonder when we can switch to C++14.

Tom

Re: [PATCH] [gdb/build] Fix build breaker with -std=c++11

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]

On 8/8/23 00:16, Tom Tromey wrote:
> Simon> When we encounter things like this, should we add some greppable
> Simon> TODO comment to record an actionable item for when we switch to a
> Simon> more recent C++ version?
> 
> FWIW I know of one other instance of this, in gdb_demangle:
> 
> 	  std::string copy = name;
> 	  run_on_main_thread ([=] ()
> 	    {
> 	      report_failed_demangle (copy.c_str (), core_dump_allowed,
> 
> Here, the name is copied twice, but with move captures there could be
> just a single copy.

Like so?

Thanks,
- Tom



[-- Attachment #2: 0001-gdb-build-Use-move-capture-in-gdb_demangle.patch --]
[-- Type: text/x-patch, Size: 1141 bytes --]

From 7a6f3e7e39701b2b0e2f3b8f761e7c32f46af4a6 Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Tue, 8 Aug 2023 01:34:45 +0200
Subject: [PATCH] [gdb/build] Use move capture in gdb_demangle

Use move capture in gdb_demangle when compiling for c++14 or higher, to save a
std::string copy.

Tested on x86_64-linux.

Reported-By: Tom Tromey <tom@tromey.com>
---
 gdb/cp-support.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 2fb9d9ef0b2..0300727434d 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -1656,7 +1656,13 @@ gdb_demangle (const char *name, int options)
 	     we might be in a background thread.  Instead, arrange for
 	     the reporting to happen on the main thread.  */
 	  std::string copy = name;
-	  run_on_main_thread ([=] ()
+	  run_on_main_thread ([
+#if __cplusplus >= 201402L
+			       =, copy = std::move (copy)
+#else
+			       =
+#endif
+			       ] ()
 	    {
 	      report_failed_demangle (copy.c_str (), core_dump_allowed,
 				      crash_signal);

base-commit: ddbe8c3c8a8665e77f9ebb108c5627a6ebecb81f
-- 
2.35.3

Re: [PATCH] [gdb/build] Fix build breaker with -std=c++11

[Tom Tromey]

>> Here, the name is copied twice, but with move captures there could be
>> just a single copy.

Tom> Like so?

Yes, thank you.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH] [gdb/build] Fix build breaker with -std=c++11

[Simon Marchi]

On August 7, 2023 6:16:19 p.m. EDT, Tom Tromey <tom@tromey.com> wrote:
>Simon> When we encounter things like this, should we add some greppable
>Simon> TODO comment to record an actionable item for when we switch to a
>Simon> more recent C++ version?
>
>FWIW I know of one other instance of this, in gdb_demangle:
>
>	  std::string copy = name;
>	  run_on_main_thread ([=] ()
>	    {
>	      report_failed_demangle (copy.c_str (), core_dump_allowed,
>
>Here, the name is copied twice, but with move captures there could be
>just a single copy.
>
>I wonder when we can switch to C++14.

If I recall, Tom de Vries said they have to support some SLES version that has gcc 4.x as the system compiler for a while still.  Other than that I am not aware of anything holding back. 

Simon