* [PATCH] c++: Quash bogus -Wredundant-move warning
@ 2022-08-23 0:10 Marek Polacek
2022-08-23 21:53 ` Jason Merrill
0 siblings, 1 reply; 2+ messages in thread
From: Marek Polacek @ 2022-08-23 0:10 UTC (permalink / raw)
To: GCC Patches, Jason Merrill
This patch fixes a pretty stoopid thinko. When I added code to warn
about pessimizing std::move in initializations like
T t{std::move(T())};
I also added code to unwrap the expression from { }. But when we have
return {std::move(t)};
we cannot warn about a redundant std::move because the implicit move
wouldn't happen for "return {t};" because the expression isn't just
a name. However, we still want to warn about
return {std::move(T())};
so let's not disable the -Wpessimizing-move warning. Tests added for
both cases.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
gcc/cp/ChangeLog:
* typeck.cc (maybe_warn_pessimizing_move): Don't warn about
redundant std::move when the expression was wrapped in { }.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/Wpessimizing-move10.C: New test.
* g++.dg/cpp0x/Wredundant-move12.C: New test.
---
gcc/cp/typeck.cc | 13 +++++--
.../g++.dg/cpp0x/Wpessimizing-move10.C | 30 ++++++++++++++++
.../g++.dg/cpp0x/Wredundant-move12.C | 36 +++++++++++++++++++
3 files changed, 77 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move10.C
create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wredundant-move12.C
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 992ebfd99fb..7fde65adaa4 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10397,11 +10397,15 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
if (!CLASS_TYPE_P (type))
return;
+ bool wrapped_p = false;
/* A a = std::move (A()); */
if (TREE_CODE (expr) == TREE_LIST)
{
if (list_length (expr) == 1)
- expr = TREE_VALUE (expr);
+ {
+ expr = TREE_VALUE (expr);
+ wrapped_p = true;
+ }
else
return;
}
@@ -10410,7 +10414,10 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
else if (TREE_CODE (expr) == CONSTRUCTOR)
{
if (CONSTRUCTOR_NELTS (expr) == 1)
- expr = CONSTRUCTOR_ELT (expr, 0)->value;
+ {
+ expr = CONSTRUCTOR_ELT (expr, 0)->value;
+ wrapped_p = true;
+ }
else
return;
}
@@ -10458,6 +10465,8 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
/* Warn if the move is redundant. It is redundant when we would
do maybe-rvalue overload resolution even without std::move. */
else if (warn_redundant_move
+ /* This doesn't apply for return {std::move (t)};. */
+ && !wrapped_p
&& !warning_suppressed_p (expr, OPT_Wredundant_move)
&& (moved = treat_lvalue_as_rvalue_p (arg, /*return*/true)))
{
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move10.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move10.C
new file mode 100644
index 00000000000..77314141da3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move10.C
@@ -0,0 +1,30 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wpessimizing-move" }
+
+// Define std::move.
+namespace std {
+ template<typename _Tp>
+ struct remove_reference
+ { typedef _Tp type; };
+
+ template<typename _Tp>
+ struct remove_reference<_Tp&>
+ { typedef _Tp type; };
+
+ template<typename _Tp>
+ struct remove_reference<_Tp&&>
+ { typedef _Tp type; };
+
+ template<typename _Tp>
+ constexpr typename std::remove_reference<_Tp>::type&&
+ move(_Tp&& __t) noexcept
+ { return static_cast<typename std::remove_reference<_Tp>::type&&>(__t); }
+}
+
+struct S { };
+
+S
+f ()
+{
+ return {std::move(S())}; // { dg-warning "moving a temporary object prevents copy elision" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move12.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move12.C
new file mode 100644
index 00000000000..192d981ba5e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move12.C
@@ -0,0 +1,36 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wredundant-move" }
+
+// Define std::move.
+namespace std {
+ template<typename _Tp>
+ struct remove_reference
+ { typedef _Tp type; };
+
+ template<typename _Tp>
+ struct remove_reference<_Tp&>
+ { typedef _Tp type; };
+
+ template<typename _Tp>
+ struct remove_reference<_Tp&&>
+ { typedef _Tp type; };
+
+ template<typename _Tp>
+ constexpr typename std::remove_reference<_Tp>::type&&
+ move(_Tp&& __t) noexcept
+ { return static_cast<typename std::remove_reference<_Tp>::type&&>(__t); }
+}
+
+struct S1 {
+ S1();
+ S1(S1 const &) = delete;
+ S1(S1 &&);
+ S1 operator =(S1 const &) = delete;
+ S1 operator =(S1 &&);
+};
+
+struct S2 { S2(S1); };
+S2 f() {
+ S1 s;
+ return { std::move(s) }; // { dg-bogus "redundant move" }
+}
base-commit: cc4fa7a210b638d6a46f14dab17f2361389d18e1
--
2.37.2
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] c++: Quash bogus -Wredundant-move warning
2022-08-23 0:10 [PATCH] c++: Quash bogus -Wredundant-move warning Marek Polacek
@ 2022-08-23 21:53 ` Jason Merrill
0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2022-08-23 21:53 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
On 8/22/22 17:10, Marek Polacek wrote:
> This patch fixes a pretty stoopid thinko. When I added code to warn
> about pessimizing std::move in initializations like
>
> T t{std::move(T())};
>
> I also added code to unwrap the expression from { }. But when we have
>
> return {std::move(t)};
>
> we cannot warn about a redundant std::move because the implicit move
> wouldn't happen for "return {t};" because the expression isn't just
> a name. However, we still want to warn about
>
> return {std::move(T())};
>
> so let's not disable the -Wpessimizing-move warning. Tests added for
> both cases.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
OK.
> gcc/cp/ChangeLog:
>
> * typeck.cc (maybe_warn_pessimizing_move): Don't warn about
> redundant std::move when the expression was wrapped in { }.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp0x/Wpessimizing-move10.C: New test.
> * g++.dg/cpp0x/Wredundant-move12.C: New test.
> ---
> gcc/cp/typeck.cc | 13 +++++--
> .../g++.dg/cpp0x/Wpessimizing-move10.C | 30 ++++++++++++++++
> .../g++.dg/cpp0x/Wredundant-move12.C | 36 +++++++++++++++++++
> 3 files changed, 77 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move10.C
> create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wredundant-move12.C
>
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 992ebfd99fb..7fde65adaa4 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -10397,11 +10397,15 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
> if (!CLASS_TYPE_P (type))
> return;
>
> + bool wrapped_p = false;
> /* A a = std::move (A()); */
> if (TREE_CODE (expr) == TREE_LIST)
> {
> if (list_length (expr) == 1)
> - expr = TREE_VALUE (expr);
> + {
> + expr = TREE_VALUE (expr);
> + wrapped_p = true;
> + }
> else
> return;
> }
> @@ -10410,7 +10414,10 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
> else if (TREE_CODE (expr) == CONSTRUCTOR)
> {
> if (CONSTRUCTOR_NELTS (expr) == 1)
> - expr = CONSTRUCTOR_ELT (expr, 0)->value;
> + {
> + expr = CONSTRUCTOR_ELT (expr, 0)->value;
> + wrapped_p = true;
> + }
> else
> return;
> }
> @@ -10458,6 +10465,8 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
> /* Warn if the move is redundant. It is redundant when we would
> do maybe-rvalue overload resolution even without std::move. */
> else if (warn_redundant_move
> + /* This doesn't apply for return {std::move (t)};. */
> + && !wrapped_p
> && !warning_suppressed_p (expr, OPT_Wredundant_move)
> && (moved = treat_lvalue_as_rvalue_p (arg, /*return*/true)))
> {
> diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move10.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move10.C
> new file mode 100644
> index 00000000000..77314141da3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move10.C
> @@ -0,0 +1,30 @@
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wpessimizing-move" }
> +
> +// Define std::move.
> +namespace std {
> + template<typename _Tp>
> + struct remove_reference
> + { typedef _Tp type; };
> +
> + template<typename _Tp>
> + struct remove_reference<_Tp&>
> + { typedef _Tp type; };
> +
> + template<typename _Tp>
> + struct remove_reference<_Tp&&>
> + { typedef _Tp type; };
> +
> + template<typename _Tp>
> + constexpr typename std::remove_reference<_Tp>::type&&
> + move(_Tp&& __t) noexcept
> + { return static_cast<typename std::remove_reference<_Tp>::type&&>(__t); }
> +}
> +
> +struct S { };
> +
> +S
> +f ()
> +{
> + return {std::move(S())}; // { dg-warning "moving a temporary object prevents copy elision" }
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move12.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move12.C
> new file mode 100644
> index 00000000000..192d981ba5e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move12.C
> @@ -0,0 +1,36 @@
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wredundant-move" }
> +
> +// Define std::move.
> +namespace std {
> + template<typename _Tp>
> + struct remove_reference
> + { typedef _Tp type; };
> +
> + template<typename _Tp>
> + struct remove_reference<_Tp&>
> + { typedef _Tp type; };
> +
> + template<typename _Tp>
> + struct remove_reference<_Tp&&>
> + { typedef _Tp type; };
> +
> + template<typename _Tp>
> + constexpr typename std::remove_reference<_Tp>::type&&
> + move(_Tp&& __t) noexcept
> + { return static_cast<typename std::remove_reference<_Tp>::type&&>(__t); }
> +}
> +
> +struct S1 {
> + S1();
> + S1(S1 const &) = delete;
> + S1(S1 &&);
> + S1 operator =(S1 const &) = delete;
> + S1 operator =(S1 &&);
> +};
> +
> +struct S2 { S2(S1); };
> +S2 f() {
> + S1 s;
> + return { std::move(s) }; // { dg-bogus "redundant move" }
> +}
>
> base-commit: cc4fa7a210b638d6a46f14dab17f2361389d18e1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-08-23 21:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-23 0:10 [PATCH] c++: Quash bogus -Wredundant-move warning Marek Polacek
2022-08-23 21:53 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).