* [PATCH] c++: Tweaks for -Wredundant-move [PR107363]
@ 2022-10-28 20:42 Marek Polacek
2022-11-07 14:07 ` Marek Polacek
2022-11-07 22:59 ` Jason Merrill
0 siblings, 2 replies; 3+ messages in thread
From: Marek Polacek @ 2022-10-28 20:42 UTC (permalink / raw)
To: GCC Patches, Jason Merrill
Two things here:
1) when we're pointing out that std::move on a constant object is
redundant, don't say "in return statement" when we aren't in a
return statement;
2) suppress the warning when the std::move call was dependent, because
removing the std::move may not be correct for a different
instantiation of the original template.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
PR c++/107363
gcc/cp/ChangeLog:
* semantics.cc (finish_call_expr): Suppress OPT_Wpessimizing_move.
* typeck.cc (maybe_warn_pessimizing_move): Check warn_redundant_move
and warning_suppressed_p. Adjust a message depending on return_p.
(check_return_expr): Don't suppress OPT_Wpessimizing_move here.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/Wredundant-move13.C: New test.
---
gcc/cp/semantics.cc | 4 ++
gcc/cp/typeck.cc | 16 ++---
.../g++.dg/cpp0x/Wredundant-move13.C | 61 +++++++++++++++++++
3 files changed, 73 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wredundant-move13.C
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 36aa9c4499f..caaa40fde19 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -2738,6 +2738,10 @@ finish_call_expr (tree fn, vec<tree, va_gc> **args, bool disallow_virtual,
result = build_min_nt_call_vec (orig_fn, *args);
SET_EXPR_LOCATION (result, cp_expr_loc_or_input_loc (fn));
KOENIG_LOOKUP_P (result) = koenig_p;
+ /* Disable the std::move warnings since this call was dependent
+ (c++/89780, c++/107363). This also suppresses the
+ -Wredundant-move warning. */
+ suppress_warning (result, OPT_Wpessimizing_move);
if (is_overloaded_fn (fn))
fn = get_fns (fn);
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 2e0fd8fbf17..5f5fb2a212b 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10885,7 +10885,9 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
and where the std::move does nothing if T does not have a T(const T&&)
constructor, because the argument is const. It will not use T(T&&)
because that would mean losing the const. */
- else if (TYPE_REF_P (TREE_TYPE (arg))
+ else if (warn_redundant_move
+ && !warning_suppressed_p (expr, OPT_Wredundant_move)
+ && TYPE_REF_P (TREE_TYPE (arg))
&& CP_TYPE_CONST_P (TREE_TYPE (TREE_TYPE (arg))))
{
tree rtype = TREE_TYPE (TREE_TYPE (arg));
@@ -10901,8 +10903,11 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
return;
}
auto_diagnostic_group d;
- if (warning_at (loc, OPT_Wredundant_move,
- "redundant move in return statement"))
+ if (return_p
+ ? warning_at (loc, OPT_Wredundant_move,
+ "redundant move in return statement")
+ : warning_at (loc, OPT_Wredundant_move,
+ "redundant move in initialization"))
inform (loc, "remove %<std::move%> call");
}
}
@@ -11126,11 +11131,6 @@ check_return_expr (tree retval, bool *no_warning)
/* We don't know if this is an lvalue or rvalue use, but
either way we can mark it as read. */
mark_exp_read (retval);
- /* Disable our std::move warnings when we're returning
- a dependent expression (c++/89780). */
- if (retval && TREE_CODE (retval) == CALL_EXPR)
- /* This also suppresses -Wredundant-move. */
- suppress_warning (retval, OPT_Wpessimizing_move);
return retval;
}
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move13.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move13.C
new file mode 100644
index 00000000000..80e7d80cd02
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move13.C
@@ -0,0 +1,61 @@
+// PR c++/107363
+// { 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); }
+}
+
+template <typename T, typename U>
+struct Optional {
+ U &value();
+ T release_value() {
+ T t = std::move (value ());
+ return t;
+ }
+};
+
+struct Foo {};
+void test(Optional<const Foo, const Foo> o) { o.release_value(); }
+
+struct F {
+ F(const F&);
+ F(F&&) = delete;
+};
+
+struct Z {
+ Z(const Z&) = delete;
+ Z(Z&&) = delete;
+ Z(const Z&&);
+};
+
+const F& constfref();
+const Z& constzref();
+
+void
+g ()
+{
+ // Will call F::F(const F&) w/ and w/o std::move. So it's redundant.
+ F f = std::move (constfref()); // { dg-warning "redundant move in initialization" }
+ (void) f;
+ // Will call Z::Z(const Z&&) w/ std::move, and Z::Z(const Z&) w/o.
+ // So it's not redundant.
+ Z z = std::move (constzref());
+ (void) z;
+}
base-commit: e583c86f49b9ef6991b25308a0ad60de9697f24a
--
2.38.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] c++: Tweaks for -Wredundant-move [PR107363]
2022-10-28 20:42 [PATCH] c++: Tweaks for -Wredundant-move [PR107363] Marek Polacek
@ 2022-11-07 14:07 ` Marek Polacek
2022-11-07 22:59 ` Jason Merrill
1 sibling, 0 replies; 3+ messages in thread
From: Marek Polacek @ 2022-11-07 14:07 UTC (permalink / raw)
To: GCC Patches, Jason Merrill
Ping.
On Fri, Oct 28, 2022 at 04:42:33PM -0400, Marek Polacek via Gcc-patches wrote:
> Two things here:
>
> 1) when we're pointing out that std::move on a constant object is
> redundant, don't say "in return statement" when we aren't in a
> return statement;
> 2) suppress the warning when the std::move call was dependent, because
> removing the std::move may not be correct for a different
> instantiation of the original template.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> PR c++/107363
>
> gcc/cp/ChangeLog:
>
> * semantics.cc (finish_call_expr): Suppress OPT_Wpessimizing_move.
> * typeck.cc (maybe_warn_pessimizing_move): Check warn_redundant_move
> and warning_suppressed_p. Adjust a message depending on return_p.
> (check_return_expr): Don't suppress OPT_Wpessimizing_move here.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp0x/Wredundant-move13.C: New test.
> ---
> gcc/cp/semantics.cc | 4 ++
> gcc/cp/typeck.cc | 16 ++---
> .../g++.dg/cpp0x/Wredundant-move13.C | 61 +++++++++++++++++++
> 3 files changed, 73 insertions(+), 8 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wredundant-move13.C
>
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index 36aa9c4499f..caaa40fde19 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -2738,6 +2738,10 @@ finish_call_expr (tree fn, vec<tree, va_gc> **args, bool disallow_virtual,
> result = build_min_nt_call_vec (orig_fn, *args);
> SET_EXPR_LOCATION (result, cp_expr_loc_or_input_loc (fn));
> KOENIG_LOOKUP_P (result) = koenig_p;
> + /* Disable the std::move warnings since this call was dependent
> + (c++/89780, c++/107363). This also suppresses the
> + -Wredundant-move warning. */
> + suppress_warning (result, OPT_Wpessimizing_move);
> if (is_overloaded_fn (fn))
> fn = get_fns (fn);
>
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 2e0fd8fbf17..5f5fb2a212b 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -10885,7 +10885,9 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
> and where the std::move does nothing if T does not have a T(const T&&)
> constructor, because the argument is const. It will not use T(T&&)
> because that would mean losing the const. */
> - else if (TYPE_REF_P (TREE_TYPE (arg))
> + else if (warn_redundant_move
> + && !warning_suppressed_p (expr, OPT_Wredundant_move)
> + && TYPE_REF_P (TREE_TYPE (arg))
> && CP_TYPE_CONST_P (TREE_TYPE (TREE_TYPE (arg))))
> {
> tree rtype = TREE_TYPE (TREE_TYPE (arg));
> @@ -10901,8 +10903,11 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
> return;
> }
> auto_diagnostic_group d;
> - if (warning_at (loc, OPT_Wredundant_move,
> - "redundant move in return statement"))
> + if (return_p
> + ? warning_at (loc, OPT_Wredundant_move,
> + "redundant move in return statement")
> + : warning_at (loc, OPT_Wredundant_move,
> + "redundant move in initialization"))
> inform (loc, "remove %<std::move%> call");
> }
> }
> @@ -11126,11 +11131,6 @@ check_return_expr (tree retval, bool *no_warning)
> /* We don't know if this is an lvalue or rvalue use, but
> either way we can mark it as read. */
> mark_exp_read (retval);
> - /* Disable our std::move warnings when we're returning
> - a dependent expression (c++/89780). */
> - if (retval && TREE_CODE (retval) == CALL_EXPR)
> - /* This also suppresses -Wredundant-move. */
> - suppress_warning (retval, OPT_Wpessimizing_move);
> return retval;
> }
>
> diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move13.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move13.C
> new file mode 100644
> index 00000000000..80e7d80cd02
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move13.C
> @@ -0,0 +1,61 @@
> +// PR c++/107363
> +// { 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); }
> +}
> +
> +template <typename T, typename U>
> +struct Optional {
> + U &value();
> + T release_value() {
> + T t = std::move (value ());
> + return t;
> + }
> +};
> +
> +struct Foo {};
> +void test(Optional<const Foo, const Foo> o) { o.release_value(); }
> +
> +struct F {
> + F(const F&);
> + F(F&&) = delete;
> +};
> +
> +struct Z {
> + Z(const Z&) = delete;
> + Z(Z&&) = delete;
> + Z(const Z&&);
> +};
> +
> +const F& constfref();
> +const Z& constzref();
> +
> +void
> +g ()
> +{
> + // Will call F::F(const F&) w/ and w/o std::move. So it's redundant.
> + F f = std::move (constfref()); // { dg-warning "redundant move in initialization" }
> + (void) f;
> + // Will call Z::Z(const Z&&) w/ std::move, and Z::Z(const Z&) w/o.
> + // So it's not redundant.
> + Z z = std::move (constzref());
> + (void) z;
> +}
>
> base-commit: e583c86f49b9ef6991b25308a0ad60de9697f24a
> --
> 2.38.1
>
Marek
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] c++: Tweaks for -Wredundant-move [PR107363]
2022-10-28 20:42 [PATCH] c++: Tweaks for -Wredundant-move [PR107363] Marek Polacek
2022-11-07 14:07 ` Marek Polacek
@ 2022-11-07 22:59 ` Jason Merrill
1 sibling, 0 replies; 3+ messages in thread
From: Jason Merrill @ 2022-11-07 22:59 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
On 10/28/22 10:42, Marek Polacek wrote:
> Two things here:
>
> 1) when we're pointing out that std::move on a constant object is
> redundant, don't say "in return statement" when we aren't in a
> return statement;
> 2) suppress the warning when the std::move call was dependent, because
> removing the std::move may not be correct for a different
> instantiation of the original template.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
OK. I'd probably do #2 with a warning_sentinel in
tsubst_copy_and_build/CALL_EXPR, but this way makes sense, too.
> PR c++/107363
>
> gcc/cp/ChangeLog:
>
> * semantics.cc (finish_call_expr): Suppress OPT_Wpessimizing_move.
> * typeck.cc (maybe_warn_pessimizing_move): Check warn_redundant_move
> and warning_suppressed_p. Adjust a message depending on return_p.
> (check_return_expr): Don't suppress OPT_Wpessimizing_move here.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp0x/Wredundant-move13.C: New test.
> ---
> gcc/cp/semantics.cc | 4 ++
> gcc/cp/typeck.cc | 16 ++---
> .../g++.dg/cpp0x/Wredundant-move13.C | 61 +++++++++++++++++++
> 3 files changed, 73 insertions(+), 8 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wredundant-move13.C
>
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index 36aa9c4499f..caaa40fde19 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -2738,6 +2738,10 @@ finish_call_expr (tree fn, vec<tree, va_gc> **args, bool disallow_virtual,
> result = build_min_nt_call_vec (orig_fn, *args);
> SET_EXPR_LOCATION (result, cp_expr_loc_or_input_loc (fn));
> KOENIG_LOOKUP_P (result) = koenig_p;
> + /* Disable the std::move warnings since this call was dependent
> + (c++/89780, c++/107363). This also suppresses the
> + -Wredundant-move warning. */
> + suppress_warning (result, OPT_Wpessimizing_move);
> if (is_overloaded_fn (fn))
> fn = get_fns (fn);
>
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 2e0fd8fbf17..5f5fb2a212b 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -10885,7 +10885,9 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
> and where the std::move does nothing if T does not have a T(const T&&)
> constructor, because the argument is const. It will not use T(T&&)
> because that would mean losing the const. */
> - else if (TYPE_REF_P (TREE_TYPE (arg))
> + else if (warn_redundant_move
> + && !warning_suppressed_p (expr, OPT_Wredundant_move)
> + && TYPE_REF_P (TREE_TYPE (arg))
> && CP_TYPE_CONST_P (TREE_TYPE (TREE_TYPE (arg))))
> {
> tree rtype = TREE_TYPE (TREE_TYPE (arg));
> @@ -10901,8 +10903,11 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
> return;
> }
> auto_diagnostic_group d;
> - if (warning_at (loc, OPT_Wredundant_move,
> - "redundant move in return statement"))
> + if (return_p
> + ? warning_at (loc, OPT_Wredundant_move,
> + "redundant move in return statement")
> + : warning_at (loc, OPT_Wredundant_move,
> + "redundant move in initialization"))
> inform (loc, "remove %<std::move%> call");
> }
> }
> @@ -11126,11 +11131,6 @@ check_return_expr (tree retval, bool *no_warning)
> /* We don't know if this is an lvalue or rvalue use, but
> either way we can mark it as read. */
> mark_exp_read (retval);
> - /* Disable our std::move warnings when we're returning
> - a dependent expression (c++/89780). */
> - if (retval && TREE_CODE (retval) == CALL_EXPR)
> - /* This also suppresses -Wredundant-move. */
> - suppress_warning (retval, OPT_Wpessimizing_move);
> return retval;
> }
>
> diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move13.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move13.C
> new file mode 100644
> index 00000000000..80e7d80cd02
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move13.C
> @@ -0,0 +1,61 @@
> +// PR c++/107363
> +// { 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); }
> +}
> +
> +template <typename T, typename U>
> +struct Optional {
> + U &value();
> + T release_value() {
> + T t = std::move (value ());
> + return t;
> + }
> +};
> +
> +struct Foo {};
> +void test(Optional<const Foo, const Foo> o) { o.release_value(); }
> +
> +struct F {
> + F(const F&);
> + F(F&&) = delete;
> +};
> +
> +struct Z {
> + Z(const Z&) = delete;
> + Z(Z&&) = delete;
> + Z(const Z&&);
> +};
> +
> +const F& constfref();
> +const Z& constzref();
> +
> +void
> +g ()
> +{
> + // Will call F::F(const F&) w/ and w/o std::move. So it's redundant.
> + F f = std::move (constfref()); // { dg-warning "redundant move in initialization" }
> + (void) f;
> + // Will call Z::Z(const Z&&) w/ std::move, and Z::Z(const Z&) w/o.
> + // So it's not redundant.
> + Z z = std::move (constzref());
> + (void) z;
> +}
>
> base-commit: e583c86f49b9ef6991b25308a0ad60de9697f24a
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-11-07 22:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-28 20:42 [PATCH] c++: Tweaks for -Wredundant-move [PR107363] Marek Polacek
2022-11-07 14:07 ` Marek Polacek
2022-11-07 22:59 ` 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).