diagnostics.h: GCC 13 got -Wself-move, breaks GDB build

Message ID 20221002185433.gl7dvytfh5wthifx@lug-owl.de
State Accepted, archived
Headers
Series diagnostics.h: GCC 13 got -Wself-move, breaks GDB build |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Jan-Benedict Glaw Oct. 2, 2022, 6:54 p.m. UTC
  Hi!

With GCC commit 0abb78dda084a14b3d955757c6431fff71c263f3 (PR81159),
gcc gained -Wself-move. GDB tests various warnings (and suppressing
them) in its unittests. -Wself-move is dealt for in case of clang, but
not (yet) for GCC, which breaks building GDB with recent GCC versions:

/usr/lib/gcc-snapshot/bin/g++ -x c++    -I. -I. -I./config -DLOCALEDIR="\"/tmp/gdb-m68k-linux/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../readline/readline/.. -I./../zlib  -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber  -I./../gnulib/import -I../gnulib/import -I./.. -I.. -I./../libbacktrace/ -I../libbacktrace/  -DTUI=1    -I./.. -pthread  -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-variable -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-error=maybe-uninitialized -Wno-mismatched-tags -Wsuggest-override -Wimplicit-fallthrough=3 -Wduplicated-cond -Wshadow=local -Wdeprecated-copy -Wdeprecated-copy-dtor -Wredundant-move -Wmissing-declarations -Wstrict-null-sentinel -Wformat -Wformat-nonliteral -Werror -g -O2   -c -o unittests/environ-selftests.o -MT unittests/environ-selftests.o -MMD -MP -MF unittests/.deps/environ-selftests.Tpo unittests/environ-selftests.c
unittests/environ-selftests.c: In function 'void selftests::gdb_environ_tests::test_self_move()':
unittests/environ-selftests.c:228:7: error: moving 'env' of type 'gdb_environ' to itself [-Werror=self-move]
  228 |   env = std::move (env);
      |   ~~~~^~~~~~~~~~~~~~~~~
unittests/environ-selftests.c:228:7: note: remove 'std::move' call
cc1plus: all warnings being treated as errors
make[1]: *** [Makefile:1896: unittests/environ-selftests.o] Error 1
make[1]: Leaving directory '/var/lib/laminar/run/gdb-m68k-linux/3/binutils-gdb/gdb'
make: *** [Makefile:13193: all-gdb] Error 2

I suggest the following patch.

Okay for HEAD?

Thanks,
  Jan-Benedict


include:
	* diagnostics.h (DIAGNOSTIC_IGNORE_SELF_MOVE): Define for GCC 13+.

--
  

Comments

Nick Clifton Oct. 3, 2022, 1:49 p.m. UTC | #1
Hi Jan-Benedict,

> +# if __GNUC__ >= 13
> +#  define DIAGNOSTIC_IGNORE_SELF_MOVE DIAGNOSTIC_IGNORE ("-Wself-move")
> +# endif

There appears to be a convention that the definition should be broken
up over two lines, ie:

   # if __GNUC__ >= 13
   #  define DIAGNOSTIC_IGNORE_SELF_MOVE \
      DIAGNOSTIC_IGNORE ("-Wself-move")
   # endif

Although DIAGNOSTIC_ERROR_SWITCH appears to be the exception to this rule.

More importantly however, you need to provide an empty definition at the
end of the file should the macro not be defined.  ie:

   #ifndef DIAGNOSTIC_IGNORE_SELF_MOVE
   # define DIAGNOSTIC_IGNORE_SELF_MOVE
   #endif

Cheers
   Nick
  
Jan-Benedict Glaw Oct. 3, 2022, 2:24 p.m. UTC | #2
On Mon, 2022-10-03 14:49:55 +0100, Nick Clifton <nickc@redhat.com> wrote:
> Hi Jan-Benedict,
> 
> > +# if __GNUC__ >= 13
> > +#  define DIAGNOSTIC_IGNORE_SELF_MOVE DIAGNOSTIC_IGNORE ("-Wself-move")
> > +# endif
> 
> There appears to be a convention that the definition should be broken
> up over two lines, ie:
> 
>   # if __GNUC__ >= 13
>   #  define DIAGNOSTIC_IGNORE_SELF_MOVE \
>      DIAGNOSTIC_IGNORE ("-Wself-move")
>   # endif

All the others would exceed some 80 columns and this macro, for the
`if defined (__clang__)` case, is also provided in one line.

> Although DIAGNOSTIC_ERROR_SWITCH appears to be the exception to this rule.

No, `DIAGNOSTIC_IGNORE_SELF_MOVE` is already existing (for __clang__)
and not wrapped there as well.

> More importantly however, you need to provide an empty definition at the
> end of the file should the macro not be defined.  ie:
> 
>   #ifndef DIAGNOSTIC_IGNORE_SELF_MOVE
>   # define DIAGNOSTIC_IGNORE_SELF_MOVE
>   #endif

That's already in place, see (after patch) at around line 115.

MfG, JBG

--
  
Nick Clifton Oct. 3, 2022, 2:36 p.m. UTC | #3
Hi Jan-Benedict,

   Me culpa for not reviewing properly.
   Patch approved - please apply.

Cheers
   Nick
  

Patch

diff --git a/include/diagnostics.h b/include/diagnostics.h
index 4161dff6abc..c1a2e8f520c 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -99,6 +99,10 @@ 
    DIAGNOSTIC_IGNORE ("-Wunused-but-set-variable")
 # endif
 
+# if __GNUC__ >= 13
+#  define DIAGNOSTIC_IGNORE_SELF_MOVE DIAGNOSTIC_IGNORE ("-Wself-move")
+# endif
+
 /* GCC 4.8's "diagnostic push/pop" seems broken when using this, -Wswitch
    remains enabled at the error level even after a pop.  Therefore, don't
    use it for GCC < 5.  */