c++: Silence -Wuseless-cast warnings during move [PR103480]

Message ID 20211231093006.GE2646553@tucnak
State New
Headers
Series c++: Silence -Wuseless-cast warnings during move [PR103480] |

Commit Message

Jakub Jelinek Dec. 31, 2021, 9:30 a.m. UTC
  Hi!

This is maybe just a shot in the dark, but IMHO we shouldn't be diagnosing
-Wuseless-cast on casts the compiler adds on its own when calling its move
function.  We don't seem to warn when user calls std::move either.
We call move on elinit (*NON_LVALUE_EXPR <(struct C[2] &&) &D.2497->b>)[0]
so it is already an xvalue_p and try to static_cast it to struct C &&.
But we don't warn e.g. on std::move (std::move (whatever)).

Bootstrapped/regtested on x86_64-linux and i686-linux.

2021-12-31  Jakub Jelinek  <jakub@redhat.com>

	PR c++/103480
	* tree.c (move): Add warn_useless_cast warning sentinel.

	* g++.dg/warn/Wuseless-cast2.C: New test.


	Jakub
  

Comments

Jason Merrill Jan. 11, 2022, 4:50 p.m. UTC | #1
On 12/31/21 04:30, Jakub Jelinek wrote:
> Hi!
> 
> This is maybe just a shot in the dark, but IMHO we shouldn't be diagnosing
> -Wuseless-cast on casts the compiler adds on its own when calling its move
> function.  We don't seem to warn when user calls std::move either.
> We call move on elinit (*NON_LVALUE_EXPR <(struct C[2] &&) &D.2497->b>)[0]
> so it is already an xvalue_p and try to static_cast it to struct C &&.
> But we don't warn e.g. on std::move (std::move (whatever)).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux.

Maybe we should

if (xvalue_p (expr))
   return expr;

instead?  OK either way.

> 2021-12-31  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/103480
> 	* tree.c (move): Add warn_useless_cast warning sentinel.
> 
> 	* g++.dg/warn/Wuseless-cast2.C: New test.
> 
> --- gcc/cp/tree.c.jj	2021-12-30 15:12:42.739157171 +0100
> +++ gcc/cp/tree.c	2021-12-30 18:39:08.050679041 +0100
> @@ -1288,6 +1288,7 @@ move (tree expr)
>     tree type = TREE_TYPE (expr);
>     gcc_assert (!TYPE_REF_P (type));
>     type = cp_build_reference_type (type, /*rval*/true);
> +  warning_sentinel w (warn_useless_cast);
>     return build_static_cast (input_location, type, expr,
>   			    tf_warning_or_error);
>   }
> --- gcc/testsuite/g++.dg/warn/Wuseless-cast2.C.jj	2021-12-30 18:46:17.437651681 +0100
> +++ gcc/testsuite/g++.dg/warn/Wuseless-cast2.C	2021-12-30 18:45:41.044162541 +0100
> @@ -0,0 +1,24 @@
> +// PR c++/103480
> +// { dg-do compile { target c++14 } }
> +// { dg-options "-Wuseless-cast" }
> +
> +template <typename T, int N>
> +struct A { typedef T t[N]; };
> +template <typename T, int N>
> +struct B { typename A<T, N>::t b; };
> +struct C {
> +  constexpr C (C &&) {}
> +  template <int N>
> +  static auto bar ()
> +  {
> +    B<C, N> r;
> +    return r;		// { dg-bogus "useless cast to type" }
> +  }
> +  C () = default;
> +};
> +
> +void
> +foo ()
> +{
> +  C::bar<2> ();
> +}
> 
> 	Jakub
>
  
Jakub Jelinek Jan. 12, 2022, 8:52 a.m. UTC | #2
On Tue, Jan 11, 2022 at 11:50:42AM -0500, Jason Merrill wrote:
> On 12/31/21 04:30, Jakub Jelinek wrote:
> > Hi!
> > 
> > This is maybe just a shot in the dark, but IMHO we shouldn't be diagnosing
> > -Wuseless-cast on casts the compiler adds on its own when calling its move
> > function.  We don't seem to warn when user calls std::move either.
> > We call move on elinit (*NON_LVALUE_EXPR <(struct C[2] &&) &D.2497->b>)[0]
> > so it is already an xvalue_p and try to static_cast it to struct C &&.
> > But we don't warn e.g. on std::move (std::move (whatever)).
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux.
> 
> Maybe we should
> 
> if (xvalue_p (expr))
>   return expr;
> 
> instead?  OK either way.

That worked well, so I've committed following instead:

2022-01-11  Jakub Jelinek  <jakub@redhat.com>
	    Jason Merrill  <jason@redhat.com>

	PR c++/103480
	* tree.c (move): If expr is xvalue_p, just return expr without
	build_static_cast.

	* g++.dg/warn/Wuseless-cast2.C: New test.

--- gcc/cp/tree.c.jj	2022-01-10 10:53:48.983925610 +0100
+++ gcc/cp/tree.c	2022-01-11 19:41:12.787433061 +0100
@@ -1304,6 +1304,8 @@ move (tree expr)
 {
   tree type = TREE_TYPE (expr);
   gcc_assert (!TYPE_REF_P (type));
+  if (xvalue_p (expr))
+    return expr;
   type = cp_build_reference_type (type, /*rval*/true);
   return build_static_cast (input_location, type, expr,
 			    tf_warning_or_error);
--- gcc/testsuite/g++.dg/warn/Wuseless-cast2.C.jj	2022-01-11 19:38:08.552039028 +0100
+++ gcc/testsuite/g++.dg/warn/Wuseless-cast2.C	2022-01-11 19:38:08.552039028 +0100
@@ -0,0 +1,24 @@
+// PR c++/103480
+// { dg-do compile { target c++14 } }
+// { dg-options "-Wuseless-cast" }
+
+template <typename T, int N>
+struct A { typedef T t[N]; };
+template <typename T, int N>
+struct B { typename A<T, N>::t b; };
+struct C {
+  constexpr C (C &&) {}
+  template <int N>
+  static auto bar ()
+  {
+    B<C, N> r;
+    return r;		// { dg-bogus "useless cast to type" }
+  }
+  C () = default;
+};
+
+void
+foo ()
+{
+  C::bar<2> ();
+}


	Jakub
  

Patch

--- gcc/cp/tree.c.jj	2021-12-30 15:12:42.739157171 +0100
+++ gcc/cp/tree.c	2021-12-30 18:39:08.050679041 +0100
@@ -1288,6 +1288,7 @@  move (tree expr)
   tree type = TREE_TYPE (expr);
   gcc_assert (!TYPE_REF_P (type));
   type = cp_build_reference_type (type, /*rval*/true);
+  warning_sentinel w (warn_useless_cast);
   return build_static_cast (input_location, type, expr,
 			    tf_warning_or_error);
 }
--- gcc/testsuite/g++.dg/warn/Wuseless-cast2.C.jj	2021-12-30 18:46:17.437651681 +0100
+++ gcc/testsuite/g++.dg/warn/Wuseless-cast2.C	2021-12-30 18:45:41.044162541 +0100
@@ -0,0 +1,24 @@ 
+// PR c++/103480
+// { dg-do compile { target c++14 } }
+// { dg-options "-Wuseless-cast" }
+
+template <typename T, int N>
+struct A { typedef T t[N]; };
+template <typename T, int N>
+struct B { typename A<T, N>::t b; };
+struct C {
+  constexpr C (C &&) {}
+  template <int N>
+  static auto bar ()
+  {
+    B<C, N> r;
+    return r;		// { dg-bogus "useless cast to type" }
+  }
+  C () = default;
+};
+
+void
+foo ()
+{
+  C::bar<2> ();
+}