diff mbox series

[v2] c++: Tweak for -Wpessimizing-move in templates [PR89780]

Message ID YvFpVOf/UFUqbaM1@redhat.com
State New, archived
Headers
Series [v2] c++: Tweak for -Wpessimizing-move in templates [PR89780] |

Commit Message

Marek Polacek Aug. 8, 2022, 7:51 p.m. UTC 
  On Sat, Aug 06, 2022 at 04:02:13PM -0700, Jason Merrill wrote:
> On 8/4/22 11:46, Marek Polacek wrote:
> > 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.
> > 
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > 	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/Wpessimizing-move7.C: Likewise.
> > 	* g++.dg/cpp0x/Wredundant-move2.C: Likewise.
> > 	* g++.dg/cpp0x/Wpessimizing-move9.C: New test.
> > ---
> >   gcc/cp/pt.cc                                  |  3 +
> >   gcc/cp/typeck.cc                              | 20 +++--
> >   gcc/diagnostic-spec.cc                        |  7 +-
> >   gcc/diagnostic-spec.h                         |  4 +-
> >   .../g++.dg/cpp0x/Wpessimizing-move3.C         |  2 +-
> >   .../g++.dg/cpp0x/Wpessimizing-move7.C         |  2 +-
> >   .../g++.dg/cpp0x/Wpessimizing-move9.C         | 89 +++++++++++++++++++
> >   gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C |  4 +-
> >   8 files changed, 119 insertions(+), 12 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move9.C
> > 
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index 6c581fe0fb7..fe7e809fc2d 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 2650beb780e..70a5efc45de 100644
> > --- a/gcc/cp/typeck.cc
> > +++ b/gcc/cp/typeck.cc
> > @@ -10430,9 +10430,10 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
> >   	  if (can_do_rvo_p (arg, type))
> >   	    {
> >   	      auto_diagnostic_group d;
> > -	      if (warning_at (loc, OPT_Wpessimizing_move,
> > -			      "moving a temporary object prevents copy "
> > -			      "elision"))
> > +	      if (!warning_suppressed_p (expr, OPT_Wpessimizing_move)
> 
> I don't think we ever want to suppress this warning; moving it to a
> different warning flag (as I suggested on the other patch) would accomplish
> that.

Agreed.  I just removed the warning_suppressed_p check though, a new flag would
need another NW_ group etc.
 
> > +		  && warning_at (loc, OPT_Wpessimizing_move,
> > +				 "moving a temporary object prevents copy "
> > +				 "elision"))
> >   		inform (loc, "remove %<std::move%> call");
> >   	    }
> >   	  /* The rest of the warnings is only relevant for when we are
> > @@ -10443,14 +10444,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
> > @@ -10695,6 +10698,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)
> 
> Should this check type_dependent_expression_p?
 
We already check that, this block is guarded by:

  if (dependent_type_p (functype)
      || type_dependent_expression_p (retval))
    {
    dependent:

> > --- a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C
> > +++ b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C
> > @@ -46,7 +46,7 @@ template <typename T1, typename T2>
> >   T1
> >   fn3 ()
> >   {
> > -  return std::move (T2{}); // { dg-warning "moving a temporary object prevents copy elision" }
> > +  return std::move (T2{});
> 
> As mentioned above, we want to keep this warning; the dangling reference
> problem is not type-dependent.

Fixed.
 
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
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.
---
 gcc/cp/pt.cc                                  |  3 +
 gcc/cp/typeck.cc                              | 13 ++-
 gcc/diagnostic-spec.cc                        |  7 +-
 gcc/diagnostic-spec.h                         |  4 +-
 .../g++.dg/cpp0x/Wpessimizing-move3.C         |  2 +-
 .../g++.dg/cpp0x/Wpessimizing-move9.C         | 89 +++++++++++++++++++
 gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C |  4 +-
 7 files changed, 114 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move9.C


base-commit: 4b0253b019943abf2cc5f4db0b7ed67caedffe4a
prerequisite-patch-id: f4862bc588ce5fed1d21016fecc4b61feb71eba5
prerequisite-patch-id: ce490c3c0b991fa7f27315387949c145c66223a4

Comments

Jason Merrill Aug. 11, 2022, 10:24 p.m. UTC | #1 
On 8/8/22 12:51, Marek Polacek wrote:
> On Sat, Aug 06, 2022 at 04:02:13PM -0700, Jason Merrill wrote:
>> On 8/4/22 11:46, Marek Polacek wrote:
>>> 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.
>>>
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>
>>> 	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/Wpessimizing-move7.C: Likewise.
>>> 	* g++.dg/cpp0x/Wredundant-move2.C: Likewise.
>>> 	* g++.dg/cpp0x/Wpessimizing-move9.C: New test.
>>> ---
>>>    gcc/cp/pt.cc                                  |  3 +
>>>    gcc/cp/typeck.cc                              | 20 +++--
>>>    gcc/diagnostic-spec.cc                        |  7 +-
>>>    gcc/diagnostic-spec.h                         |  4 +-
>>>    .../g++.dg/cpp0x/Wpessimizing-move3.C         |  2 +-
>>>    .../g++.dg/cpp0x/Wpessimizing-move7.C         |  2 +-
>>>    .../g++.dg/cpp0x/Wpessimizing-move9.C         | 89 +++++++++++++++++++
>>>    gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C |  4 +-
>>>    8 files changed, 119 insertions(+), 12 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move9.C
>>>
>>> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
>>> index 6c581fe0fb7..fe7e809fc2d 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 2650beb780e..70a5efc45de 100644
>>> --- a/gcc/cp/typeck.cc
>>> +++ b/gcc/cp/typeck.cc
>>> @@ -10430,9 +10430,10 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
>>>    	  if (can_do_rvo_p (arg, type))
>>>    	    {
>>>    	      auto_diagnostic_group d;
>>> -	      if (warning_at (loc, OPT_Wpessimizing_move,
>>> -			      "moving a temporary object prevents copy "
>>> -			      "elision"))
>>> +	      if (!warning_suppressed_p (expr, OPT_Wpessimizing_move)
>>
>> I don't think we ever want to suppress this warning; moving it to a
>> different warning flag (as I suggested on the other patch) would accomplish
>> that.
> 
> Agreed.  I just removed the warning_suppressed_p check though, a new flag would
> need another NW_ group etc.
>   
>>> +		  && warning_at (loc, OPT_Wpessimizing_move,
>>> +				 "moving a temporary object prevents copy "
>>> +				 "elision"))
>>>    		inform (loc, "remove %<std::move%> call");
>>>    	    }
>>>    	  /* The rest of the warnings is only relevant for when we are
>>> @@ -10443,14 +10444,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
>>> @@ -10695,6 +10698,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)
>>
>> Should this check type_dependent_expression_p?
>   
> We already check that, this block is guarded by:
> 
>    if (dependent_type_p (functype)
>        || type_dependent_expression_p (retval))
>      {
>      dependent:
> 
>>> --- a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C
>>> +++ b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C
>>> @@ -46,7 +46,7 @@ template <typename T1, typename T2>
>>>    T1
>>>    fn3 ()
>>>    {
>>> -  return std::move (T2{}); // { dg-warning "moving a temporary object prevents copy elision" }
>>> +  return std::move (T2{});
>>
>> As mentioned above, we want to keep this warning; the dangling reference
>> problem is not type-dependent.
> 
> Fixed.
>   
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK.

> -- >8 --
> 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.
> ---
>   gcc/cp/pt.cc                                  |  3 +
>   gcc/cp/typeck.cc                              | 13 ++-
>   gcc/diagnostic-spec.cc                        |  7 +-
>   gcc/diagnostic-spec.h                         |  4 +-
>   .../g++.dg/cpp0x/Wpessimizing-move3.C         |  2 +-
>   .../g++.dg/cpp0x/Wpessimizing-move9.C         | 89 +++++++++++++++++++
>   gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C |  4 +-
>   7 files changed, 114 insertions(+), 8 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move9.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 6c581fe0fb7..fe7e809fc2d 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 2650beb780e..f4104c75fbe 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -10443,14 +10443,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
> @@ -10695,6 +10697,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 4341ccfaae9..aece89619e7 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 28e5e5ccc75..e5f1c127d4f 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 a1af1230b68..c81f29a4ba6 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 00000000000..898040e6bfc
> --- /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 f181afeeb84..6e0aa4b0277 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
> 
> base-commit: 4b0253b019943abf2cc5f4db0b7ed67caedffe4a
> prerequisite-patch-id: f4862bc588ce5fed1d21016fecc4b61feb71eba5
> prerequisite-patch-id: ce490c3c0b991fa7f27315387949c145c66223a4
diff mbox series

Patch

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 6c581fe0fb7..fe7e809fc2d 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 2650beb780e..f4104c75fbe 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10443,14 +10443,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
@@ -10695,6 +10697,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 4341ccfaae9..aece89619e7 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 28e5e5ccc75..e5f1c127d4f 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 a1af1230b68..c81f29a4ba6 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 00000000000..898040e6bfc
--- /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 f181afeeb84..6e0aa4b0277 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