diff options
author | Marek Polacek <polacek@redhat.com> | 2022-08-03 19:47:30 -0400 |
---|---|---|
committer | Marek Polacek <polacek@redhat.com> | 2022-08-17 12:39:44 -0400 |
commit | 6602a2b2dee16af6e2d451c704789356042b5881 (patch) | |
tree | 413969ce72c7d74b591aa588fa5024cca4665e6c /gcc | |
parent | 8d22c7cb8b1a6f9b67c54a798dd5504244614e51 (diff) | |
download | gcc-6602a2b2dee16af6e2d451c704789356042b5881.zip gcc-6602a2b2dee16af6e2d451c704789356042b5881.tar.gz gcc-6602a2b2dee16af6e2d451c704789356042b5881.tar.bz2 |
c++: Tweak for -Wpessimizing-move in templates [PR89780]
In my previous patches I've been extending our std::move warnings,
but this tweak actually dials it down a little bit. As reported in
bug 89780, it's questionable to warn about expressions in templates
that were type-dependent, but aren't anymore because we're instantiating
the template. As in,
template <typename T>
Dest withMove() {
T x;
return std::move(x);
}
template Dest withMove<Dest>(); // #1
template Dest withMove<Source>(); // #2
Saying that the std::move is pessimizing for #1 is not incorrect, but
it's not useful, because removing the std::move would then pessimize #2.
So the user can't really win. At the same time, disabling the warning
just because we're in a template would be going too far, I still want to
warn for
template <typename>
Dest withMove() {
Dest x;
return std::move(x);
}
because the std::move therein will be pessimizing for any instantiation.
So I'm using the suppress_warning machinery to that effect.
Problem: I had to add a new group to nowarn_spec_t, otherwise
suppressing the -Wpessimizing-move warning would disable a whole bunch
of other warnings, which we really don't want.
PR c++/89780
gcc/cp/ChangeLog:
* pt.cc (tsubst_copy_and_build) <case CALL_EXPR>: Maybe suppress
-Wpessimizing-move.
* typeck.cc (maybe_warn_pessimizing_move): Don't issue warnings
if they are suppressed.
(check_return_expr): Disable -Wpessimizing-move when returning
a dependent expression.
gcc/ChangeLog:
* diagnostic-spec.cc (nowarn_spec_t::nowarn_spec_t): Handle
OPT_Wpessimizing_move and OPT_Wredundant_move.
* diagnostic-spec.h (nowarn_spec_t): Add NW_REDUNDANT enumerator.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/Wpessimizing-move3.C: Remove dg-warning.
* g++.dg/cpp0x/Wredundant-move2.C: Likewise.
Diffstat (limited to 'gcc')
-rw-r--r-- | gcc/cp/pt.cc | 3 | ||||
-rw-r--r-- | gcc/cp/typeck.cc | 13 | ||||
-rw-r--r-- | gcc/diagnostic-spec.cc | 7 | ||||
-rw-r--r-- | gcc/diagnostic-spec.h | 4 | ||||
-rw-r--r-- | gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move3.C | 2 | ||||
-rw-r--r-- | gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move9.C | 89 | ||||
-rw-r--r-- | gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C | 4 |
7 files changed, 114 insertions, 8 deletions
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index 6c581fe0..fe7e809 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -21215,6 +21215,9 @@ tsubst_copy_and_build (tree t, CALL_EXPR_ORDERED_ARGS (call) = ord; CALL_EXPR_REVERSE_ARGS (call) = rev; } + if (warning_suppressed_p (t, OPT_Wpessimizing_move)) + /* This also suppresses -Wredundant-move. */ + suppress_warning (ret, OPT_Wpessimizing_move); } RETURN (ret); diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 19bb7f7..e95ce8f 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -10447,14 +10447,16 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p) else if (can_do_nrvo_p (arg, type)) { auto_diagnostic_group d; - if (warning_at (loc, OPT_Wpessimizing_move, - "moving a local object in a return statement " - "prevents copy elision")) + if (!warning_suppressed_p (expr, OPT_Wpessimizing_move) + && warning_at (loc, OPT_Wpessimizing_move, + "moving a local object in a return statement " + "prevents copy elision")) inform (loc, "remove %<std::move%> call"); } /* 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 + && !warning_suppressed_p (expr, OPT_Wredundant_move) && (moved = treat_lvalue_as_rvalue_p (arg, /*return*/true))) { /* Make sure that overload resolution would actually succeed @@ -10699,6 +10701,11 @@ 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/diagnostic-spec.cc b/gcc/diagnostic-spec.cc index 4341ccf..aece896 100644 --- a/gcc/diagnostic-spec.cc +++ b/gcc/diagnostic-spec.cc @@ -96,7 +96,7 @@ nowarn_spec_t::nowarn_spec_t (opt_code opt) case OPT_Winit_self: case OPT_Wuninitialized: case OPT_Wmaybe_uninitialized: - m_bits = NW_UNINIT; + m_bits = NW_UNINIT; break; case OPT_Wdangling_pointer_: @@ -105,6 +105,11 @@ nowarn_spec_t::nowarn_spec_t (opt_code opt) m_bits = NW_DANGLING; break; + case OPT_Wpessimizing_move: + case OPT_Wredundant_move: + m_bits = NW_REDUNDANT; + break; + default: /* A catchall group for everything else. */ m_bits = NW_OTHER; diff --git a/gcc/diagnostic-spec.h b/gcc/diagnostic-spec.h index 28e5e5c..e5f1c12 100644 --- a/gcc/diagnostic-spec.h +++ b/gcc/diagnostic-spec.h @@ -45,9 +45,11 @@ public: NW_DANGLING = 1 << 5, /* All other unclassified warnings. */ NW_OTHER = 1 << 6, + /* Warnings about redundant calls. */ + NW_REDUNDANT = 1 << 7, /* All groups of warnings. */ NW_ALL = (NW_ACCESS | NW_LEXICAL | NW_NONNULL - | NW_UNINIT | NW_VFLOW | NW_DANGLING | NW_OTHER) + | NW_UNINIT | NW_VFLOW | NW_DANGLING | NW_REDUNDANT | NW_OTHER) }; nowarn_spec_t (): m_bits () { } diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move3.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move3.C index a1af123..c81f29a 100644 --- a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move3.C +++ b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move3.C @@ -39,7 +39,7 @@ Tp1 fn2 () { Tp2 t; - return std::move (t); // { dg-warning "moving a local object in a return statement prevents copy elision" } + return std::move (t); } template<typename Tp1, typename Tp2> diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move9.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move9.C new file mode 100644 index 0000000..898040e --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move9.C @@ -0,0 +1,89 @@ +// PR c++/89780 +// { dg-do compile { target c++11 } } +// { dg-options "-Wpessimizing-move -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 Dest { + Dest() = default; + Dest(Dest&&); + Dest(const Dest&); +}; +struct Source : Dest {}; + +template <typename T> +Dest withMove() { + T x; + return std::move(x); +} + +template Dest withMove<Dest>(); +template Dest withMove<Source>(); + +template<typename T> +Dest bar () { + return std::move(T()); // { dg-warning "moving a temporary object prevents copy elision" } +} + +template Dest bar<Dest>(); +template Dest bar<Source>(); + +template<typename T> +Dest baz (T x) { + return std::move(x); +} + +void +call_baz () +{ + Dest d; + Source s; + baz (d); + baz (s); +} + +template<typename> +Dest foo () +{ + Dest d; + return std::move(d); // { dg-warning "moving a local object in a return statement prevents copy elision" } +} + +template Dest foo<int>(); + +template<typename> +Dest qux () { + return std::move(Dest()); // { dg-warning "moving a temporary object prevents copy elision" } +} + +template Dest qux<int>(); + +template<typename> +Dest qui (Dest x) { + return std::move(x); // { dg-warning "redundant move in return statement" } +} + +void +call_qui () +{ + Dest d; + qui<int> (d); +} diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C index f181afe..6e0aa4b 100644 --- a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C @@ -37,14 +37,14 @@ template<typename Tp1, typename Tp2> Tp1 fn2 (Tp2 t) { - return std::move (t); // { dg-warning "redundant move in return statement" } + return std::move (t); } template<typename Tp1, typename Tp2> Tp1 fn3 (Tp2 t) { - return std::move (t); // { dg-warning "redundant move in return statement" } + return std::move (t); } int |