[Bug c++/57319] New: [4.8/4.9] Regression: bogus "defaulted move assignment for ... calls a non-trivial move assignment operator for virtual base ..."

[Bug c++/57319] New: [4.8/4.9] Regression: bogus "defaulted move assignment for ... calls a non-trivial move assignment operator for virtual base ..."

[ppluzhnikov at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57319

            Bug ID: 57319
           Summary: [4.8/4.9] Regression: bogus "defaulted move assignment
                    for ... calls a non-trivial move assignment operator
                    for virtual base ..."
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ppluzhnikov at google dot com

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.
>From gcc-bugs-return-422547-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri May 17 20:36:59 2013
Return-Path: <gcc-bugs-return-422547-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 645 invoked by alias); 17 May 2013 20:36:59 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 595 invoked by uid 48); 17 May 2013 20:36:55 -0000
From: "glisse at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug tree-optimization/57318] optimizer takes several seconds on nested loops
Date: Fri, 17 May 2013 20:36:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: tree-optimization
X-Bugzilla-Version: 4.9.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: glisse at gcc dot gnu.org
X-Bugzilla-Status: NEW
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_status cf_reconfirmed_on everconfirmed
Message-ID: <bug-57318-4-PMDi8eOsy2@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-57318-4@http.gcc.gnu.org/bugzilla/>
References: <bug-57318-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2013-05/txt/msg01220.txt.bz2
Content-length: 584

http://gcc.gnu.org/bugzilla/show_bug.cgi?idW318

Marc Glisse <glisse at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-05-17
     Ever confirmed|0                           |1

--- Comment #1 from Marc Glisse <glisse at gcc dot gnu.org> ---
All the time is spent in sched2, but the first strange thing is cunroll going
wild at -O2 (1000 iterations unrolled...).

[Bug c++/57319] [4.8/4.9] Regression: bogus "defaulted move assignment for ... calls a non-trivial move assignment operator for virtual base ..."

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57319

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-05-20
                 CC|                            |jason at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #1 from Jason Merrill <jason at gcc dot gnu.org> ---
(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.

[Bug c++/57319] [4.8 Regression]: bogus "defaulted move assignment for ... calls a non-trivial move assignment operator for virtual base ..."

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57319

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jason at gcc dot gnu.org
            Summary|[4.8/4.9] Regression: bogus |[4.8 Regression]: bogus
                   |"defaulted move assignment  |"defaulted move assignment
                   |for ... calls a non-trivial |for ... calls a non-trivial
                   |move assignment operator    |move assignment operator
                   |for virtual base ..."       |for virtual base ..."

--- Comment #2 from Jason Merrill <jason at gcc dot gnu.org> ---
Fixed for 4.9 for now.

[Bug c++/57319] [4.8 Regression]: bogus "defaulted move assignment for ... calls a non-trivial move assignment operator for virtual base ..."

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57319

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.9.0
   Target Milestone|---                         |4.8.1

[Bug c++/57319] [4.8 Regression]: bogus "defaulted move assignment for ... calls a non-trivial move assignment operator for virtual base ..."

[ppluzhnikov at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57319

--- Comment #3 from Paul Pluzhnikov <ppluzhnikov at google dot com> ---
Thanks for the fix.
Confirmed for both the reduced test case, and the original source.

Can this be back-ported to 4.8 branch?

[Bug c++/57319] [4.8 Regression]: bogus "defaulted move assignment for ... calls a non-trivial move assignment operator for virtual base ..."

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57319

--- Comment #4 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Paul Pluzhnikov from comment #3)
> Can this be back-ported to 4.8 branch?

After 4.8.1, I think.

[Bug c++/57319] [4.8 Regression]: bogus "defaulted move assignment for ... calls a non-trivial move assignment operator for virtual base ..."

[richard-gccbugzilla at metafoo dot co.uk]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57319

Richard Smith <richard-gccbugzilla at metafoo dot co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |richard-gccbugzilla@metafoo
                   |                            |.co.uk

--- Comment #5 from Richard Smith <richard-gccbugzilla at metafoo dot co.uk> ---
(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.

[Bug c++/57319] [4.8 Regression]: bogus "defaulted move assignment for ... calls a non-trivial move assignment operator for virtual base ..."

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57319

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.1                       |4.8.2

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.1 has been released.