Bug 81159 - New warning idea: -Wself-move
Summary: New warning idea: -Wself-move
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 8.0
: P3 enhancement
Target Milestone: ---
Assignee: Marek Polacek
URL:
Keywords: diagnostic, patch
Depends on:
Blocks: new-warning
  Show dependency treegraph
 
Reported: 2017-06-21 19:02 UTC by Simon Marchi
Modified: 2022-10-01 08:20 UTC (History)
7 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-08-24 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Marchi 2017-06-21 19:02:30 UTC
clang has this warning, -Wself-move, which warns about things like this:

  int a = 1;

  a = std::move (a);

I don't see any reason why code like this would be desirable, so any code that ends up like this would likely be a mistake.  It would be nice to point it out.

Example output:

test.c:6:4: error: explicitly moving variable of type 'int' to itself [-Werror,-Wself-move]
        a = std::move(a);
        ~ ^           ~
1 error generated.
Comment 1 Eric Gallager 2017-08-24 05:15:34 UTC
Confirmed.
Comment 2 Eric Gallager 2019-04-17 21:35:53 UTC
cc-ing Marek since he wrote a blog post that seems relevant here: https://developers.redhat.com/blog/2019/04/12/understanding-when-not-to-stdmove-in-c/
Comment 3 Marek Polacek 2019-04-17 21:37:31 UTC
Ok, this shouldn't be too hard.  I guess I could implement it for GCC 10.
Comment 4 Nikolay Orliuk 2019-04-17 22:36:40 UTC
Just curious if this code works good with return value optimization like:

static inline T conditional_update(T&& src, bool flag) {
   if (flag) {
      return T{};
   } else {
      return std::move(src);
   }
}

T a;
a = conditional_update(a, true);
a = conditional_update(a, false);

Is it going to produce warning after inlining and propagating constants?..
Comment 5 Marek Polacek 2019-04-18 13:41:08 UTC
The warning is taking place in the front end, long before inlining/cprop has run.
Comment 6 Eric Gallager 2019-11-15 05:30:27 UTC
(In reply to Marek Polacek from comment #3)
> Ok, this shouldn't be too hard.  I guess I could implement it for GCC 10.

last chance to get it in before stage 1 closes...
Comment 7 Marek Polacek 2019-11-18 16:29:05 UTC
(In reply to Eric Gallager from comment #6)
> (In reply to Marek Polacek from comment #3)
> > Ok, this shouldn't be too hard.  I guess I could implement it for GCC 10.
> 
> last chance to get it in before stage 1 closes...

I was working on C++20 features which are more important.  So this will have to wait until the next stage 1.
Comment 8 Marek Polacek 2022-08-09 18:04:50 UTC
Patch posted: https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599503.html
Comment 9 CVS Commits 2022-08-26 18:02:29 UTC
The trunk branch has been updated by Marek Polacek <mpolacek@gcc.gnu.org>:

https://gcc.gnu.org/g:0abb78dda084a14b3d955757c6431fff71c263f3

commit r13-2227-g0abb78dda084a14b3d955757c6431fff71c263f3
Author: Marek Polacek <polacek@redhat.com>
Date:   Mon Aug 8 17:45:28 2022 -0400

    c++: Implement -Wself-move warning [PR81159]
    
    About 5 years ago we got a request to implement -Wself-move, which
    warns about useless moves like this:
    
      int x;
      x = std::move (x);
    
    This patch implements that warning.
    
            PR c++/81159
    
    gcc/c-family/ChangeLog:
    
            * c.opt (Wself-move): New option.
    
    gcc/cp/ChangeLog:
    
            * typeck.cc (maybe_warn_self_move): New.
            (cp_build_modify_expr): Call maybe_warn_self_move.
    
    gcc/ChangeLog:
    
            * doc/invoke.texi: Document -Wself-move.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/warn/Wself-move1.C: New test.
Comment 10 Marek Polacek 2022-08-26 18:03:12 UTC
Implemented in GCC 13.
Comment 11 Simon Marchi 2022-08-26 18:05:31 UTC
Awesome, thanks!
Comment 12 Jan-Benedict Glaw 2022-10-01 08:20:04 UTC
Just ftr: Building GDB with a recent GCC now breaks since GDB is testing -Wself-move in its unittests, but only covers the warning in case of clang. Found by my automated builders. I proposed a patch to fix that.