Skip to content

Commit 6602a2b

Browse files
committed
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.
1 parent 8d22c7c commit 6602a2b

File tree

7 files changed

+114
-8
lines changed

7 files changed

+114
-8
lines changed

gcc/cp/pt.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21215,6 +21215,9 @@ tsubst_copy_and_build (tree t,
2121521215
CALL_EXPR_ORDERED_ARGS (call) = ord;
2121621216
CALL_EXPR_REVERSE_ARGS (call) = rev;
2121721217
}
21218+
if (warning_suppressed_p (t, OPT_Wpessimizing_move))
21219+
/* This also suppresses -Wredundant-move. */
21220+
suppress_warning (ret, OPT_Wpessimizing_move);
2121821221
}
2121921222

2122021223
RETURN (ret);

gcc/cp/typeck.cc

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10447,14 +10447,16 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
1044710447
else if (can_do_nrvo_p (arg, type))
1044810448
{
1044910449
auto_diagnostic_group d;
10450-
if (warning_at (loc, OPT_Wpessimizing_move,
10451-
"moving a local object in a return statement "
10452-
"prevents copy elision"))
10450+
if (!warning_suppressed_p (expr, OPT_Wpessimizing_move)
10451+
&& warning_at (loc, OPT_Wpessimizing_move,
10452+
"moving a local object in a return statement "
10453+
"prevents copy elision"))
1045310454
inform (loc, "remove %<std::move%> call");
1045410455
}
1045510456
/* Warn if the move is redundant. It is redundant when we would
1045610457
do maybe-rvalue overload resolution even without std::move. */
1045710458
else if (warn_redundant_move
10459+
&& !warning_suppressed_p (expr, OPT_Wredundant_move)
1045810460
&& (moved = treat_lvalue_as_rvalue_p (arg, /*return*/true)))
1045910461
{
1046010462
/* Make sure that overload resolution would actually succeed
@@ -10699,6 +10701,11 @@ check_return_expr (tree retval, bool *no_warning)
1069910701
/* We don't know if this is an lvalue or rvalue use, but
1070010702
either way we can mark it as read. */
1070110703
mark_exp_read (retval);
10704+
/* Disable our std::move warnings when we're returning
10705+
a dependent expression (c++/89780). */
10706+
if (retval && TREE_CODE (retval) == CALL_EXPR)
10707+
/* This also suppresses -Wredundant-move. */
10708+
suppress_warning (retval, OPT_Wpessimizing_move);
1070210709
return retval;
1070310710
}
1070410711

gcc/diagnostic-spec.cc

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ nowarn_spec_t::nowarn_spec_t (opt_code opt)
9696
case OPT_Winit_self:
9797
case OPT_Wuninitialized:
9898
case OPT_Wmaybe_uninitialized:
99-
m_bits = NW_UNINIT;
99+
m_bits = NW_UNINIT;
100100
break;
101101

102102
case OPT_Wdangling_pointer_:
@@ -105,6 +105,11 @@ nowarn_spec_t::nowarn_spec_t (opt_code opt)
105105
m_bits = NW_DANGLING;
106106
break;
107107

108+
case OPT_Wpessimizing_move:
109+
case OPT_Wredundant_move:
110+
m_bits = NW_REDUNDANT;
111+
break;
112+
108113
default:
109114
/* A catchall group for everything else. */
110115
m_bits = NW_OTHER;

gcc/diagnostic-spec.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,11 @@ class nowarn_spec_t
4545
NW_DANGLING = 1 << 5,
4646
/* All other unclassified warnings. */
4747
NW_OTHER = 1 << 6,
48+
/* Warnings about redundant calls. */
49+
NW_REDUNDANT = 1 << 7,
4850
/* All groups of warnings. */
4951
NW_ALL = (NW_ACCESS | NW_LEXICAL | NW_NONNULL
50-
| NW_UNINIT | NW_VFLOW | NW_DANGLING | NW_OTHER)
52+
| NW_UNINIT | NW_VFLOW | NW_DANGLING | NW_REDUNDANT | NW_OTHER)
5153
};
5254

5355
nowarn_spec_t (): m_bits () { }

gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move3.C

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ Tp1
3939
fn2 ()
4040
{
4141
Tp2 t;
42-
return std::move (t); // { dg-warning "moving a local object in a return statement prevents copy elision" }
42+
return std::move (t);
4343
}
4444

4545
template<typename Tp1, typename Tp2>
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// PR c++/89780
2+
// { dg-do compile { target c++11 } }
3+
// { dg-options "-Wpessimizing-move -Wredundant-move" }
4+
5+
// Define std::move.
6+
namespace std {
7+
template<typename _Tp>
8+
struct remove_reference
9+
{ typedef _Tp type; };
10+
11+
template<typename _Tp>
12+
struct remove_reference<_Tp&>
13+
{ typedef _Tp type; };
14+
15+
template<typename _Tp>
16+
struct remove_reference<_Tp&&>
17+
{ typedef _Tp type; };
18+
19+
template<typename _Tp>
20+
constexpr typename std::remove_reference<_Tp>::type&&
21+
move(_Tp&& __t) noexcept
22+
{ return static_cast<typename std::remove_reference<_Tp>::type&&>(__t); }
23+
}
24+
25+
struct Dest {
26+
Dest() = default;
27+
Dest(Dest&&);
28+
Dest(const Dest&);
29+
};
30+
struct Source : Dest {};
31+
32+
template <typename T>
33+
Dest withMove() {
34+
T x;
35+
return std::move(x);
36+
}
37+
38+
template Dest withMove<Dest>();
39+
template Dest withMove<Source>();
40+
41+
template<typename T>
42+
Dest bar () {
43+
return std::move(T()); // { dg-warning "moving a temporary object prevents copy elision" }
44+
}
45+
46+
template Dest bar<Dest>();
47+
template Dest bar<Source>();
48+
49+
template<typename T>
50+
Dest baz (T x) {
51+
return std::move(x);
52+
}
53+
54+
void
55+
call_baz ()
56+
{
57+
Dest d;
58+
Source s;
59+
baz (d);
60+
baz (s);
61+
}
62+
63+
template<typename>
64+
Dest foo ()
65+
{
66+
Dest d;
67+
return std::move(d); // { dg-warning "moving a local object in a return statement prevents copy elision" }
68+
}
69+
70+
template Dest foo<int>();
71+
72+
template<typename>
73+
Dest qux () {
74+
return std::move(Dest()); // { dg-warning "moving a temporary object prevents copy elision" }
75+
}
76+
77+
template Dest qux<int>();
78+
79+
template<typename>
80+
Dest qui (Dest x) {
81+
return std::move(x); // { dg-warning "redundant move in return statement" }
82+
}
83+
84+
void
85+
call_qui ()
86+
{
87+
Dest d;
88+
qui<int> (d);
89+
}

gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,14 @@ template<typename Tp1, typename Tp2>
3737
Tp1
3838
fn2 (Tp2 t)
3939
{
40-
return std::move (t); // { dg-warning "redundant move in return statement" }
40+
return std::move (t);
4141
}
4242

4343
template<typename Tp1, typename Tp2>
4444
Tp1
4545
fn3 (Tp2 t)
4646
{
47-
return std::move (t); // { dg-warning "redundant move in return statement" }
47+
return std::move (t);
4848
}
4949

5050
int

0 commit comments

Comments
 (0)