My recent patch caused us to call convert_for_initialization for a std:move's argument to see if it would have succeeded had the returned expression been just that argument.
That caused a bogus error in this test, because convert_for_initialization might cause additional instantiations, and they might fail. My first version of the patch fixed this by adding "cp_unevaluated e;", preventing add_pending_template from adding further instantiations, but I no longer think that's the best fix, because in this case the argument isn't an id-expression, and the implicit move wouldn't be performed, so we shouldn't warn. Thus fixed by making the maybe_warn_pessimizing_move condition more strict -- that fixes both the bogus error and the bogus warning. Specifically, make sure that the argument is of form "(T &) &t" and not "(T &) (T *) &t" or similar. Also add a test with template-ids. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-03-11 Marek Polacek <pola...@redhat.com> PR c++/89660 - bogus error with -Wredundant-move. * typeck.c (maybe_warn_pessimizing_move): Only accept (T &) &arg as the std::move's argument. Don't call convert_for_initialization when warn_redundant_move isn't on. * g++.dg/cpp0x/Wredundant-move8.C: New test. * g++.dg/cpp0x/Wredundant-move9.C: New test. diff --git gcc/cp/typeck.c gcc/cp/typeck.c index 51f47814acd..f77e9c6180d 100644 --- gcc/cp/typeck.c +++ gcc/cp/typeck.c @@ -9409,7 +9409,7 @@ maybe_warn_pessimizing_move (tree retval, tree functype) if (!CLASS_TYPE_P (functype)) return; - /* We're looking for *std::move<T&> (&arg). */ + /* We're looking for *std::move<T&> ((T &) &arg). */ if (REFERENCE_REF_P (retval) && TREE_CODE (TREE_OPERAND (retval, 0)) == CALL_EXPR) { @@ -9417,7 +9417,9 @@ maybe_warn_pessimizing_move (tree retval, tree functype) if (is_std_move_p (fn)) { tree arg = CALL_EXPR_ARG (fn, 0); - STRIP_NOPS (arg); + if (TREE_CODE (arg) != NOP_EXPR) + return; + arg = TREE_OPERAND (arg, 0); if (TREE_CODE (arg) != ADDR_EXPR) return; arg = TREE_OPERAND (arg, 0); @@ -9433,7 +9435,8 @@ maybe_warn_pessimizing_move (tree retval, tree functype) } /* Warn if the move is redundant. It is redundant when we would do maybe-rvalue overload resolution even without std::move. */ - else if (treat_lvalue_as_rvalue_p (arg, /*parm_ok*/true)) + else if (warn_redundant_move + && treat_lvalue_as_rvalue_p (arg, /*parm_ok*/true)) { /* Make sure that the overload resolution would actually succeed if we removed the std::move call. */ diff --git gcc/testsuite/g++.dg/cpp0x/Wredundant-move8.C gcc/testsuite/g++.dg/cpp0x/Wredundant-move8.C new file mode 100644 index 00000000000..c290585b18b --- /dev/null +++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move8.C @@ -0,0 +1,38 @@ +// PR c++/89660 +// { 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 S> struct D { + template <typename T> D (D<T> x) : k(&x.foo ()) {} + S &foo (); + int *k; +}; + +D<int> bar (); + +struct F { + D<int> baz () { + D<F> f = bar (); + return std::move (*reinterpret_cast<D<int> *> (&f)); // { dg-bogus "redundant move in return statement" } + } +}; diff --git gcc/testsuite/g++.dg/cpp0x/Wredundant-move9.C gcc/testsuite/g++.dg/cpp0x/Wredundant-move9.C new file mode 100644 index 00000000000..fdd3ce16092 --- /dev/null +++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move9.C @@ -0,0 +1,108 @@ +// { 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 Tp> +struct T { + T() { } + T(const T&) { } + T(T&&) { } +}; + +template<typename Tp> +struct U { + U() { } + U(const U&) { } + U(U&&) { } + U(T<Tp>) { } +}; + +T<int> +fn1 (T<int> t) +{ + return t; +} + +T<int> +fn2 (T<int> t) +{ + // Will use move even without std::move. + return std::move (t); // { dg-warning "redundant move in return statement" } +} + +T<int> +fn3 (const T<int> t) +{ + // t is const: will decay into copy. + return t; +} + +T<int> +fn4 (const T<int> t) +{ + // t is const: will decay into copy despite std::move, so it's redundant. + // We used to warn about this, but no longer since c++/87378. + return std::move (t); +} + +int +fn5 (int i) +{ + // Not a class type. + return std::move (i); +} + +T<int> +fn6 (T<int> t, bool b) +{ + if (b) + throw std::move (t); + return std::move (t); // { dg-warning "redundant move in return statement" } +} + +U<int> +fn7 (T<int> t) +{ + // Core 1579 means we'll get a move here. + return t; +} + +U<int> +fn8 (T<int> t) +{ + // Core 1579 means we'll get a move here. Even without std::move. + return std::move (t); // { dg-warning "redundant move in return statement" } +} + +T<int> +fn9 (T<int>& t) +{ + // T is a reference and the move isn't redundant. + return std::move (t); +} + +T<int> +fn10 (T<int>&& t) +{ + // T is a reference and the move isn't redundant. + return std::move (t); +}