[RFC] Fix build error in macroexp.c

Message ID 20180622071433.nsoekb62ctcluwap@localhost.localdomain
State New, archived
Headers

Commit Message

Tom de Vries June 22, 2018, 7:14 a.m. UTC
  Hi,

When doing a combined build with the gcc and binutils-gdb repos, I run into
this build error in gdb:
...
gdb/macroexp.c: \
  In function ‘void get_next_token_for_substitution(macro_buffer*, \
  macro_buffer*, char**, macro_buffer*, char**, int*, bool*)’:
gdb/macroexp.c:925:17: error: \
  implicitly-declared ‘constexpr macro_buffer& \
  macro_buffer::operator=(const macro_buffer&)’ is deprecated \
  [-Werror=deprecated-copy]
       *token = *lookahead;
...

This patch fixes the build error by adding an explicit copy operator to the
macro_buffer class.  I've added asserts to ensure that both the dest and src
of the copy are shared, in other words, neither is owner of the text pointer.

I've run the gdb testsuite on x86_64-linux and the asserts did not trigger.

Any comments?

Thanks,
- Tom

[gdb] Fix build error in macroexp.c

2018-06-22  Tom de Vries  <tdevries@suse.de>

	* macroexp.c (macro_buffer) <operator=>: New member function.

---
 gdb/macroexp.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
  

Comments

Tom de Vries July 3, 2018, 2:22 p.m. UTC | #1
On 06/22/2018 09:14 AM, Tom de Vries wrote:
> Hi,
> 
> When doing a combined build with the gcc and binutils-gdb repos, I run into
> this build error in gdb:
> ...
> gdb/macroexp.c: \
>   In function ‘void get_next_token_for_substitution(macro_buffer*, \
>   macro_buffer*, char**, macro_buffer*, char**, int*, bool*)’:
> gdb/macroexp.c:925:17: error: \
>   implicitly-declared ‘constexpr macro_buffer& \
>   macro_buffer::operator=(const macro_buffer&)’ is deprecated \
>   [-Werror=deprecated-copy]
>        *token = *lookahead;
> ...
> 
> This patch fixes the build error by adding an explicit copy operator to the
> macro_buffer class.  I've added asserts to ensure that both the dest and src
> of the copy are shared, in other words, neither is owner of the text pointer.
> 
> I've run the gdb testsuite on x86_64-linux and the asserts did not trigger.
> 
> Any comments?
> 
> Thanks,
> - Tom
> 
> [gdb] Fix build error in macroexp.c
> 
> 2018-06-22  Tom de Vries  <tdevries@suse.de>
> 
> 	* macroexp.c (macro_buffer) <operator=>: New member function.
> 
> ---
>  gdb/macroexp.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/gdb/macroexp.c b/gdb/macroexp.c
> index 1fa37d2875..8b4dfb20b8 100644
> --- a/gdb/macroexp.c
> +++ b/gdb/macroexp.c
> @@ -112,6 +112,16 @@ struct macro_buffer
>      shared = true;
>    }
>  
> +  macro_buffer& operator=(const macro_buffer &src)
> +  {
> +    gdb_assert (src.shared);
> +    gdb_assert (shared);
> +    set_shared (src.text, src.len);
> +    last_token = src.last_token;
> +    is_identifier = src.is_identifier;
> +    return *this;
> +  }
> +
>    ~macro_buffer ()
>    {
>      if (! shared && size)
>
  
Simon Marchi July 4, 2018, 3:51 p.m. UTC | #2
On 2018-06-22 03:14, Tom de Vries wrote:
> Hi,
> 
> When doing a combined build with the gcc and binutils-gdb repos, I run 
> into
> this build error in gdb:
> ...
> gdb/macroexp.c: \
>   In function ‘void get_next_token_for_substitution(macro_buffer*, \
>   macro_buffer*, char**, macro_buffer*, char**, int*, bool*)’:
> gdb/macroexp.c:925:17: error: \
>   implicitly-declared ‘constexpr macro_buffer& \
>   macro_buffer::operator=(const macro_buffer&)’ is deprecated \
>   [-Werror=deprecated-copy]
>        *token = *lookahead;
> ...
> 
> This patch fixes the build error by adding an explicit copy operator to 
> the
> macro_buffer class.  I've added asserts to ensure that both the dest 
> and src
> of the copy are shared, in other words, neither is owner of the text 
> pointer.
> 
> I've run the gdb testsuite on x86_64-linux and the asserts did not 
> trigger.
> 
> Any comments?

Hi Tom,

Maybe just mention in the commit log that this new warning 
(deprecated-copy) is present in recent gccs (> 8 I think?).  LGTM with 
the following nit fixed:

> diff --git a/gdb/macroexp.c b/gdb/macroexp.c
> index 1fa37d2875..8b4dfb20b8 100644
> --- a/gdb/macroexp.c
> +++ b/gdb/macroexp.c
> @@ -112,6 +112,16 @@ struct macro_buffer
>      shared = true;
>    }
> 
> +  macro_buffer& operator=(const macro_buffer &src)

Missing space before parenthesis.

Thanks,

Simon
  

Patch

diff --git a/gdb/macroexp.c b/gdb/macroexp.c
index 1fa37d2875..8b4dfb20b8 100644
--- a/gdb/macroexp.c
+++ b/gdb/macroexp.c
@@ -112,6 +112,16 @@  struct macro_buffer
     shared = true;
   }
 
+  macro_buffer& operator=(const macro_buffer &src)
+  {
+    gdb_assert (src.shared);
+    gdb_assert (shared);
+    set_shared (src.text, src.len);
+    last_token = src.last_token;
+    is_identifier = src.is_identifier;
+    return *this;
+  }
+
   ~macro_buffer ()
   {
     if (! shared && size)