[review] gdb: fix build error in unittests/vec-utils-selftests.c

[review] gdb: fix build error in unittests/vec-utils-selftests.c

[Simon Marchi (Code Review)]

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/626
......................................................................

gdb: fix build error in unittests/vec-utils-selftests.c

When building with gcc 9.2.0, I get the following build error:

    In file included from /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:23:
    /home/simark/src/binutils-gdb/gdb/gdbsupport/gdb_vecs.h: In instantiation of ‘T unordered_remove(std::__debug::vector<T>&, typename std::__debug::vector<T>::iterator) [with T = selftests::vector_utils_tests::unordered_remove_tests()::obj; typename std::__debug::vector<T>::iterator = __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<selftests::vector_utils_tests::unordered_remove_tests()::obj*, std::__cxx1998::vector<selftests::vector_utils_tests::unordered_remove_tests()::obj, std::allocator<selftests::vector_utils_tests::unordered_remove_tests()::obj> > >, std::__debug::vector<selftests::vector_utils_tests::unordered_remove_tests()::obj>, std::random_access_iterator_tag>]’:
    /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:53:26:   required from here
    /home/simark/src/binutils-gdb/gdb/gdbsupport/gdb_vecs.h:53:5: error: implicitly-declared ‘selftests::vector_utils_tests::unordered_remove_tests()::obj::obj(const selftests::vector_utils_tests::unordered_remove_tests()::obj&)’ is deprecated [-Werror=deprecated-copy]
       53 |   T removed = std::move (*it);
          |     ^~~~~~~
    /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:41:10: note: because ‘selftests::vector_utils_tests::unordered_remove_tests()::obj’ has user-provided ‘selftests::vector_utils_tests::unordered_remove_tests()::obj& selftests::vector_utils_tests::unordered_remove_tests()::obj::operator=(const selftests::vector_utils_tests::unordered_remove_tests()::obj&)’
       41 |     obj &operator= (const obj &other)
          |          ^~~~~~~~
    In file included from /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:23:
    /home/simark/src/binutils-gdb/gdb/gdbsupport/gdb_vecs.h:58:10: error: implicitly-declared ‘selftests::vector_utils_tests::unordered_remove_tests()::obj::obj(const selftests::vector_utils_tests::unordered_remove_tests()::obj&)’ is deprecated [-Werror=deprecated-copy]
       58 |   return removed;
          |          ^~~~~~~
    /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:41:10: note: because ‘selftests::vector_utils_tests::unordered_remove_tests()::obj’ has user-provided ‘selftests::vector_utils_tests::unordered_remove_tests()::obj& selftests::vector_utils_tests::unordered_remove_tests()::obj::operator=(const selftests::vector_utils_tests::unordered_remove_tests()::obj&)’
       41 |     obj &operator= (const obj &other)
          |          ^~~~~~~~

I think gcc is just trying to be nice and recommends the good practice
of providing a copy constructor if an assignment operator is provided.

Silence the warning by providing that copy constructor.

Change-Id: I323361b1c120bf8525613b74e7e5983910e002df
---
M gdb/unittests/vec-utils-selftests.c
1 file changed, 9 insertions(+), 0 deletions(-)



diff --git a/gdb/unittests/vec-utils-selftests.c b/gdb/unittests/vec-utils-selftests.c
index 823bbb6..d0ead99 100644
--- a/gdb/unittests/vec-utils-selftests.c
+++ b/gdb/unittests/vec-utils-selftests.c
@@ -38,6 +38,15 @@
   {
     std::vector<void *> var;
 
+    obj() = default;
+
+    /* gcc complains if we provide an assignment operator but no copy
+       constructor, so provide even if don't really care for this test.  */
+    obj(const obj &other)
+    {
+      this->var = other.var;
+    }
+
     obj &operator= (const obj &other)
     {
       if (this == &other)

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I323361b1c120bf8525613b74e7e5983910e002df
Gerrit-Change-Number: 626
Gerrit-PatchSet: 1
Gerrit-Owner: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: newchange

[review] gdb: fix build error in unittests/vec-utils-selftests.c

[Tom de Vries (Code Review)]

Tom de Vries has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/626
......................................................................


Patch Set 1: Code-Review+1

I also looked at this and came up with the similar:
...
diff --git a/gdb/unittests/vec-utils-selftests.c b/gdb/unittests/vec-utils-selftests.c
index 823bbb61c25..61b9851dfd9 100644
--- a/gdb/unittests/vec-utils-selftests.c
+++ b/gdb/unittests/vec-utils-selftests.c
@@ -45,6 +45,9 @@ unordered_remove_tests ()
       this->var = other.var;
       return *this;
     }
+
+    obj () {}
+    obj (const obj &other) {}
   };
 
   std::vector<obj> v;
...

I've tested your patch, and can confirm it fixes my build.  LGTM.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I323361b1c120bf8525613b74e7e5983910e002df
Gerrit-Change-Number: 626
Gerrit-PatchSet: 1
Gerrit-Owner: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom de Vries <tdevries@suse.de>
Gerrit-Comment-Date: Thu, 14 Nov 2019 09:00:13 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[review] gdb: fix build error in unittests/vec-utils-selftests.c

[Andrew Burgess (Code Review)]

Andrew Burgess has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/626
......................................................................


Patch Set 1: Code-Review+2

Sorry for introducing this breakage.  Oddly I tried on a GCC 10 built from git a few weeks ago and I don't see the build warning/error.

Anyway, I tested that with this patch the original bug in unordered_remove is still detected, so this LGTM.

Thanks for picking up after me :)


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I323361b1c120bf8525613b74e7e5983910e002df
Gerrit-Change-Number: 626
Gerrit-PatchSet: 1
Gerrit-Owner: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom de Vries <tdevries@suse.de>
Gerrit-Comment-Date: Thu, 14 Nov 2019 11:19:12 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[pushed] gdb: fix build error in unittests/vec-utils-selftests.c

[Sourceware to Gerrit sync (Code Review)]

Sourceware to Gerrit sync has submitted this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/626
......................................................................

gdb: fix build error in unittests/vec-utils-selftests.c

When building with gcc 9.2.0, I get the following build error:

    In file included from /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:23:
    /home/simark/src/binutils-gdb/gdb/gdbsupport/gdb_vecs.h: In instantiation of ‘T unordered_remove(std::__debug::vector<T>&, typename std::__debug::vector<T>::iterator) [with T = selftests::vector_utils_tests::unordered_remove_tests()::obj; typename std::__debug::vector<T>::iterator = __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<selftests::vector_utils_tests::unordered_remove_tests()::obj*, std::__cxx1998::vector<selftests::vector_utils_tests::unordered_remove_tests()::obj, std::allocator<selftests::vector_utils_tests::unordered_remove_tests()::obj> > >, std::__debug::vector<selftests::vector_utils_tests::unordered_remove_tests()::obj>, std::random_access_iterator_tag>]’:
    /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:53:26:   required from here
    /home/simark/src/binutils-gdb/gdb/gdbsupport/gdb_vecs.h:53:5: error: implicitly-declared ‘selftests::vector_utils_tests::unordered_remove_tests()::obj::obj(const selftests::vector_utils_tests::unordered_remove_tests()::obj&)’ is deprecated [-Werror=deprecated-copy]
       53 |   T removed = std::move (*it);
          |     ^~~~~~~
    /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:41:10: note: because ‘selftests::vector_utils_tests::unordered_remove_tests()::obj’ has user-provided ‘selftests::vector_utils_tests::unordered_remove_tests()::obj& selftests::vector_utils_tests::unordered_remove_tests()::obj::operator=(const selftests::vector_utils_tests::unordered_remove_tests()::obj&)’
       41 |     obj &operator= (const obj &other)
          |          ^~~~~~~~
    In file included from /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:23:
    /home/simark/src/binutils-gdb/gdb/gdbsupport/gdb_vecs.h:58:10: error: implicitly-declared ‘selftests::vector_utils_tests::unordered_remove_tests()::obj::obj(const selftests::vector_utils_tests::unordered_remove_tests()::obj&)’ is deprecated [-Werror=deprecated-copy]
       58 |   return removed;
          |          ^~~~~~~
    /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:41:10: note: because ‘selftests::vector_utils_tests::unordered_remove_tests()::obj’ has user-provided ‘selftests::vector_utils_tests::unordered_remove_tests()::obj& selftests::vector_utils_tests::unordered_remove_tests()::obj::operator=(const selftests::vector_utils_tests::unordered_remove_tests()::obj&)’
       41 |     obj &operator= (const obj &other)
          |          ^~~~~~~~

I think gcc is just trying to be nice and recommends the good practice
of providing a copy constructor if an assignment operator is provided.

Silence the warning by providing that copy constructor.

gdb/ChangeLog:

	* unittests/vec-utils-selftests.c (unordered_remove_tests::obj):
	Provide explicit default and copy constructor.

Change-Id: I323361b1c120bf8525613b74e7e5983910e002df
---
M gdb/ChangeLog
M gdb/unittests/vec-utils-selftests.c
2 files changed, 14 insertions(+), 0 deletions(-)


diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8be8efb..719574f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2019-11-14  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* unittests/vec-utils-selftests.c (unordered_remove_tests::obj):
+	Provide explicit default and copy constructor.
+
 2019-11-14  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* python/py-finishbreakpoint.c (gdbpy_breakpoint_created):
diff --git a/gdb/unittests/vec-utils-selftests.c b/gdb/unittests/vec-utils-selftests.c
index 823bbb6..5149091 100644
--- a/gdb/unittests/vec-utils-selftests.c
+++ b/gdb/unittests/vec-utils-selftests.c
@@ -38,6 +38,15 @@
   {
     std::vector<void *> var;
 
+    obj() = default;
+
+    /* gcc complains if we provide an assignment operator but no copy
+       constructor, so provide one even if don't really care for this test.  */
+    obj(const obj &other)
+    {
+      this->var = other.var;
+    }
+
     obj &operator= (const obj &other)
     {
       if (this == &other)

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I323361b1c120bf8525613b74e7e5983910e002df
Gerrit-Change-Number: 626
Gerrit-PatchSet: 2
Gerrit-Owner: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom de Vries <tdevries@suse.de>
Gerrit-MessageType: merged

[pushed] gdb: fix build error in unittests/vec-utils-selftests.c

[Sourceware to Gerrit sync (Code Review)]

The original change was created by Simon Marchi.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/626
......................................................................

gdb: fix build error in unittests/vec-utils-selftests.c

When building with gcc 9.2.0, I get the following build error:

    In file included from /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:23:
    /home/simark/src/binutils-gdb/gdb/gdbsupport/gdb_vecs.h: In instantiation of ‘T unordered_remove(std::__debug::vector<T>&, typename std::__debug::vector<T>::iterator) [with T = selftests::vector_utils_tests::unordered_remove_tests()::obj; typename std::__debug::vector<T>::iterator = __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<selftests::vector_utils_tests::unordered_remove_tests()::obj*, std::__cxx1998::vector<selftests::vector_utils_tests::unordered_remove_tests()::obj, std::allocator<selftests::vector_utils_tests::unordered_remove_tests()::obj> > >, std::__debug::vector<selftests::vector_utils_tests::unordered_remove_tests()::obj>, std::random_access_iterator_tag>]’:
    /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:53:26:   required from here
    /home/simark/src/binutils-gdb/gdb/gdbsupport/gdb_vecs.h:53:5: error: implicitly-declared ‘selftests::vector_utils_tests::unordered_remove_tests()::obj::obj(const selftests::vector_utils_tests::unordered_remove_tests()::obj&)’ is deprecated [-Werror=deprecated-copy]
       53 |   T removed = std::move (*it);
          |     ^~~~~~~
    /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:41:10: note: because ‘selftests::vector_utils_tests::unordered_remove_tests()::obj’ has user-provided ‘selftests::vector_utils_tests::unordered_remove_tests()::obj& selftests::vector_utils_tests::unordered_remove_tests()::obj::operator=(const selftests::vector_utils_tests::unordered_remove_tests()::obj&)’
       41 |     obj &operator= (const obj &other)
          |          ^~~~~~~~
    In file included from /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:23:
    /home/simark/src/binutils-gdb/gdb/gdbsupport/gdb_vecs.h:58:10: error: implicitly-declared ‘selftests::vector_utils_tests::unordered_remove_tests()::obj::obj(const selftests::vector_utils_tests::unordered_remove_tests()::obj&)’ is deprecated [-Werror=deprecated-copy]
       58 |   return removed;
          |          ^~~~~~~
    /home/simark/src/binutils-gdb/gdb/unittests/vec-utils-selftests.c:41:10: note: because ‘selftests::vector_utils_tests::unordered_remove_tests()::obj’ has user-provided ‘selftests::vector_utils_tests::unordered_remove_tests()::obj& selftests::vector_utils_tests::unordered_remove_tests()::obj::operator=(const selftests::vector_utils_tests::unordered_remove_tests()::obj&)’
       41 |     obj &operator= (const obj &other)
          |          ^~~~~~~~

I think gcc is just trying to be nice and recommends the good practice
of providing a copy constructor if an assignment operator is provided.

Silence the warning by providing that copy constructor.

gdb/ChangeLog:

	* unittests/vec-utils-selftests.c (unordered_remove_tests::obj):
	Provide explicit default and copy constructor.

Change-Id: I323361b1c120bf8525613b74e7e5983910e002df
---
M gdb/ChangeLog
M gdb/unittests/vec-utils-selftests.c
2 files changed, 14 insertions(+), 0 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8be8efb..719574f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2019-11-14  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* unittests/vec-utils-selftests.c (unordered_remove_tests::obj):
+	Provide explicit default and copy constructor.
+
 2019-11-14  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* python/py-finishbreakpoint.c (gdbpy_breakpoint_created):
diff --git a/gdb/unittests/vec-utils-selftests.c b/gdb/unittests/vec-utils-selftests.c
index 823bbb6..5149091 100644
--- a/gdb/unittests/vec-utils-selftests.c
+++ b/gdb/unittests/vec-utils-selftests.c
@@ -38,6 +38,15 @@
   {
     std::vector<void *> var;
 
+    obj() = default;
+
+    /* gcc complains if we provide an assignment operator but no copy
+       constructor, so provide one even if don't really care for this test.  */
+    obj(const obj &other)
+    {
+      this->var = other.var;
+    }
+
     obj &operator= (const obj &other)
     {
       if (this == &other)

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I323361b1c120bf8525613b74e7e5983910e002df
Gerrit-Change-Number: 626
Gerrit-PatchSet: 2
Gerrit-Owner: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom de Vries <tdevries@suse.de>
Gerrit-MessageType: newpatchset