Bug 70647 - Feature request: warning for self-moving in constructors
Summary: Feature request: warning for self-moving in constructors
Status: RESOLVED DUPLICATE of bug 19808
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: unknown
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-13 20:24 UTC by Matt Godbolt
Modified: 2016-04-14 22:05 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Godbolt 2016-04-13 20:24:34 UTC
Consider the following code:

#include <utility> // for std::move
struct A {
  int a; int b;
  
  A(A &&o) 
    : a(a), // I get a warning here...
      b(o.b) {}  
};
struct B { 
  int a; int b; 
  B(B &&o) 
    : a(std::move(a)),  // but sadly not here
      b(std::move(o.b)) {}
};

[ c.f https://godbolt.org/g/v7zPYL ]

In the non-moving case, I get a warning that I've typoed and assigned a with itself.

In the move case, there's no such warning. I appreciate this is probably difficult (if not impossible) to detect, but if there's any way it can be done, it would save a painful debugging session or two!
Comment 1 Manuel López-Ibáñez 2016-04-13 22:09:51 UTC
(In reply to Matt Godbolt from comment #0)
> In the move case, there's no such warning. I appreciate this is probably
> difficult (if not impossible) to detect, but if there's any way it can be
> done, it would save a painful debugging session or two!

I don't think "move" has anything to do with this. If you use "a + 1" or "foo(a)", you are still initializing "a" with an uninitialized value.

In that sense, this is what PR19808 is about, for which there is a patch but, sadly, nobody has had so far the time or interest to finish it and properly submit it.

*** This bug has been marked as a duplicate of bug 19808 ***
Comment 2 Matt Godbolt 2016-04-14 12:31:51 UTC
Thanks Manuel. Interestingly this does elicit a warning:

struct B { 
  int a; int b; 
  B(B &&o) 
    : a(static_cast<int>(a)),
      b(std::move(o.b)) {}
};

but this does not:

struct B { 
  int a; int b; 
  B(B &&o) 
    : a(static_cast<int&&>(a)),
      b(std::move(o.b)) {}
};

That said; I believe bug 19808 would indeed help in this situation!
Comment 3 Manuel López-Ibáñez 2016-04-14 21:50:59 UTC
> --- Comment #2 from Matt Godbolt <matt at godbolt dot org> ---
> Thanks Manuel. Interestingly this does elicit a warning:
>
> struct B {
>   int a; int b;
>   B(B &&o)
>     : a(static_cast<int>(a)),
>       b(std::move(o.b)) {}
> };

Most probable, the FE removes the cast before reaching this warning, so the
warning does not see an expression.  If you cast to something non trivial,
it remains an expression and the warning doesn't try to look inside it.

I wonder what happens if you do a(b), a(a). Does it still warn? My guess
would be yes. If so, it is just checking for equality on both sides, no
attempt to look within expressions nor track initialisation.
Comment 4 Matt Godbolt 2016-04-14 22:05:30 UTC
Agreed re: cast/FE.

I couldn't quite get your example to fail as the "o" parameter is unusued (which would be a good clue!

#include <utility> // for std::move

struct B { 
  int a; int b; 
  B(B &&o) 
    : a(b),
      b(o.a) {}
};

passes just fine though; which is grist for the mill for bug 19808 being fixed

(e.g. https://godbolt.org/g/QHLCQv )

Thanks again!