Bug 57319 - [4.8 Regression]: bogus "defaulted move assignment for ... calls a non-trivial move assignment operator for virtual base ..."
Summary: [4.8 Regression]: bogus "defaulted move assignment for ... calls a non-trivia...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: unknown
: P3 normal
Target Milestone: 4.8.2
Assignee: Jason Merrill
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2013-05-17 20:20 UTC by Paul Pluzhnikov
Modified: 2013-05-31 13:05 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.8.2, 4.9.0
Known to fail: 4.8.1
Last reconfirmed: 2013-05-20 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Pluzhnikov 2013-05-17 20:20:35 UTC
Google reference: b/9004260


Test case:

class A { };
class B: virtual A { };
class C: virtual B { };

class D: C
{
   void operator= (D &);
};


Using current trunk (@r199023):

g++ -c t.ii -std=c++11
t.ii:3:7: warning: defaulted move assignment for ‘C’ calls a non-trivial move assignment operator for virtual base ‘B’ [-Wvirtual-move-assign]
 class C: virtual B { };
       ^

Richard Smith writes:

  The problem is that a defaulted move assignment for a class with two
  inheritance paths to a virtual base may move-assign that virtual base
  multiple times (and thus may lose state).

  However, this particular case *isn't* the problematic case, because
  (a) this sample code should not trigger the definition of C's move
      assignment operator, and
  (b) there is only one inheritance path from C to B, so it won't be
      move-assigned multiple times, and
  (c) the issue isn't with *non-trivial* move assignments, it's with
      *user-provided* move-assignments (for the virtual base or any of its
      subobjects), which B does not have.

  => This is a false positive.
Comment 1 Jason Merrill 2013-05-20 13:37:18 UTC
(In reply to Paul Pluzhnikov from comment #0)
>   However, this particular case *isn't* the problematic case, because
>   (a) this sample code should not trigger the definition of C's move
>       assignment operator, and

True, the warning is given at declaration time; it would be possible to move the warning to when the move assignment is used, but that might mean design errors don't get caught until later.

>   (b) there is only one inheritance path from C to B, so it won't be
>       move-assigned multiple times, and

True, the warning is given at the point of first virtual derivation rather than when it appears in a diamond-shaped hierarchy.  But the purpose of virtual derivation is to support diamond-shaped hierarchies, so again it seems appropriate to warn sooner rather than later.

>   (c) the issue isn't with *non-trivial* move assignments, it's with
>       *user-provided* move-assignments (for the virtual base or any of its
>       subobjects), which B does not have.

Agreed, it would definitely be better for the warning to check that none of the subobjects of B have user-provided move-assignments.
Comment 2 Jason Merrill 2013-05-20 17:02:34 UTC
Fixed for 4.9 for now.
Comment 3 Paul Pluzhnikov 2013-05-23 17:33:17 UTC
Thanks for the fix.
Confirmed for both the reduced test case, and the original source.

Can this be back-ported to 4.8 branch?
Comment 4 Jason Merrill 2013-05-23 21:00:15 UTC
(In reply to Paul Pluzhnikov from comment #3)
> Can this be back-ported to 4.8 branch?

After 4.8.1, I think.
Comment 5 Richard Smith 2013-05-23 21:42:21 UTC
(In reply to Jason Merrill from comment #1)
> (In reply to Paul Pluzhnikov from comment #0)
> >   However, this particular case *isn't* the problematic case, because
> >   (a) this sample code should not trigger the definition of C's move
> >       assignment operator, and
> 
> True, the warning is given at declaration time; it would be possible to move
> the warning to when the move assignment is used, but that might mean design
> errors don't get caught until later.

OK, that makes sense. However, delaying the check until the operator= is lazily declared does not fully achieve this goal. Could the check be performed at the end of the definition of class C (rather than, presumably, when looking for a virtual C::operator= for D::operator= to override)?

> >   (b) there is only one inheritance path from C to B, so it won't be
> >       move-assigned multiple times, and
> 
> True, the warning is given at the point of first virtual derivation rather
> than when it appears in a diamond-shaped hierarchy.  But the purpose of
> virtual derivation is to support diamond-shaped hierarchies, so again it
> seems appropriate to warn sooner rather than later.

OK. The class which brings together the diamond may provide a move assignment operator which does not blindly call the move assignment operators on the base classes, so this can still have some false positives even if every virtual base is involved in diamond inheritance.
Comment 6 Jakub Jelinek 2013-05-31 10:58:18 UTC
GCC 4.8.1 has been released.
Comment 7 Jason Merrill 2013-05-31 13:05:48 UTC
Fixed for 4.8.2.