On 5/23/24 19:57, Marek Polacek wrote:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
We already warn for:

   x = std::move (x);

which triggers:

   warning: moving 'x' of type 'int' to itself [-Wself-move]

but bug 109396 reports that this doesn't work for a member-initializer-list:

   X() : x(std::move (x))

so this patch amends that.

        PR c++/109396

gcc/cp/ChangeLog:

        * cp-tree.h (maybe_warn_self_move): Declare.
        * init.cc (perform_member_init): Call maybe_warn_self_move.
        * typeck.cc (maybe_warn_self_move): No longer static.  Change the
        return type to bool.  Also warn when called from
        a member-initializer-list.

gcc/testsuite/ChangeLog:

        * g++.dg/warn/Wself-move2.C: New test.
---
  gcc/cp/cp-tree.h                        |  1 +
  gcc/cp/init.cc                          |  5 ++--
  gcc/cp/typeck.cc                        | 28 +++++++++++++------
  gcc/testsuite/g++.dg/warn/Wself-move2.C | 37 +++++++++++++++++++++++++
  4 files changed, 60 insertions(+), 11 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/warn/Wself-move2.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index ba9e848c177..ea3fa6f4aac 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8263,6 +8263,7 @@ extern cp_expr build_c_cast                       
(location_t loc, tree type,
                                                 cp_expr expr);
  extern tree cp_build_c_cast                   (location_t, tree, tree,
                                                 tsubst_flags_t);
+extern bool maybe_warn_self_move               (location_t, tree, tree);
  extern cp_expr build_x_modify_expr            (location_t, tree,
                                                 enum tree_code, tree,
                                                 tree, tsubst_flags_t);
diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
index 52396d87a8c..4a7ed7f5302 100644
--- a/gcc/cp/init.cc
+++ b/gcc/cp/init.cc
@@ -999,7 +999,7 @@ perform_member_init (tree member, tree init, hash_set<tree> 
&uninitialized)
    if (decl == error_mark_node)
      return;
- if ((warn_init_self || warn_uninitialized)
+  if ((warn_init_self || warn_uninitialized || warn_self_move)
        && init
        && TREE_CODE (init) == TREE_LIST
        && TREE_CHAIN (init) == NULL_TREE)
@@ -1013,7 +1013,8 @@ perform_member_init (tree member, tree init, hash_set<tree> 
&uninitialized)
        warning_at (DECL_SOURCE_LOCATION (current_function_decl),
                    OPT_Winit_self, "%qD is initialized with itself",
                    member);
-      else
+      else if (!maybe_warn_self_move (input_location, member,
+                                     TREE_VALUE (init)))
        find_uninit_fields (&val, &uninitialized, decl);
      }
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index d7fa6e0dd96..e058ce18276 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -9355,27 +9355,27 @@ cp_build_c_cast (location_t loc, tree type, tree expr,
/* Warn when a value is moved to itself with std::move. LHS is the target,
     RHS may be the std::move call, and LOC is the location of the whole
-   assignment.  */
+   assignment.  Return true if we warned.  */
-static void
+bool
  maybe_warn_self_move (location_t loc, tree lhs, tree rhs)
  {
    if (!warn_self_move)
-    return;
+    return false;
/* C++98 doesn't know move. */
    if (cxx_dialect < cxx11)
-    return;
+    return false;
if (processing_template_decl)
-    return;
+    return false;
if (!REFERENCE_REF_P (rhs)
        || TREE_CODE (TREE_OPERAND (rhs, 0)) != CALL_EXPR)
-    return;
+    return false;
    tree fn = TREE_OPERAND (rhs, 0);
    if (!is_std_move_p (fn))
-    return;
+    return false;
/* Just a little helper to strip * and various NOPs. */
    auto extract_op = [] (tree &op) {
@@ -9393,13 +9393,23 @@ maybe_warn_self_move (location_t loc, tree lhs, tree 
rhs)
    tree type = TREE_TYPE (lhs);
    tree orig_lhs = lhs;
    extract_op (lhs);
-  if (cp_tree_equal (lhs, arg))
+  if (cp_tree_equal (lhs, arg)
+      /* Also warn in a member-initializer-list, as in : i(std::move(i)).  */
+      || (TREE_CODE (lhs) == FIELD_DECL
+         && TREE_CODE (arg) == COMPONENT_REF
+         && cp_tree_equal (TREE_OPERAND (arg, 0), current_class_ref)
+         && TREE_OPERAND (arg, 1) == lhs))
      {
        auto_diagnostic_group d;
        if (warning_at (loc, OPT_Wself_move,
                      "moving %qE of type %qT to itself", orig_lhs, type))
-       inform (loc, "remove %<std::move%> call");
+       {
+         inform (loc, "remove %<std::move%> call");

The patch is OK, but why do we suggest removing std::move? That just changes the warning to self-init. Maybe drop the inform?

Jason

Reply via email to