c++: Tweak for -Wpessimizing-move in templates [PR89780]
authorMarek Polacek <polacek@redhat.com>
Wed, 3 Aug 2022 23:47:30 +0000 (19:47 -0400)
committerMarek Polacek <polacek@redhat.com>
Wed, 17 Aug 2022 16:39:44 +0000 (12:39 -0400)
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
gcc/cp/typeck.cc
gcc/diagnostic-spec.cc
gcc/diagnostic-spec.h
gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move3.C
gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move9.C [new file with mode: 0644]
gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C

index 6c581fe..fe7e809 100644 (file)
@@ -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);
index 19bb7f7..e95ce8f 100644 (file)
@@ -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;
     }
 
index 4341ccf..aece896 100644 (file)
@@ -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;
index 28e5e5c..e5f1c12 100644 (file)
@@ -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 () { }
index a1af123..c81f29a 100644 (file)
@@ -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 (file)
index 0000000..898040e
--- /dev/null
@@ -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);
+}
index f181afe..6e0aa4b 100644 (file)
@@ -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