Bug 18975 - [4.0 regression] Copying objects with mutable non-static data members
Summary: [4.0 regression] Copying objects with mutable non-static data members
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 3.3
: P2 normal
Target Milestone: 4.0.0
Assignee: Nathan Sidwell
URL:
Keywords: rejects-valid
Depends on:
Blocks:
 
Reported: 2004-12-14 07:19 UTC by Wolfgang Roehrl
Modified: 2005-07-23 22:49 UTC (History)
3 users (show)

See Also:
Host: i386-pc-mingw32
Target: powerpc-wrs-vxworks
Build: sparc-sun-solaris2.5.1
Known to work: 3.4.3
Known to fail: 4.0.0
Last reconfirmed: 2004-12-20 01:24:43


Attachments
patch (1.55 KB, patch)
2004-12-17 16:20 UTC, Nathan Sidwell
Details | Diff
testcase (312 bytes, text/plain)
2004-12-17 16:21 UTC, Nathan Sidwell
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Wolfgang Roehrl 2004-12-14 07:19:26 UTC
Dear all,

I would like to post a bug report for the GNU C/C++ compiler 3.3-e500.

We use the compiler to generate code for a PowerPC processor.

Used invokation line for the GNU C++ compiler:

ccppc -c -x c++ -ansi -Wall -Werror -mcpu=8540 -fverbose-asm -mbig
      -fmerge-templates -mmultiple -mno-string -mstrict-align -O3
      -fno-exceptions -fno-rtti -fno-builtin-printf
      -I<different include paths>
      -D<differen #define's>
      K.CPP -oK.O


// [ start of example program

struct PTR
{
    PTR ();
    PTR (PTR&);
    PTR& operator= (PTR&);

private:
    PTR (const PTR&);                             // <--- line 8
    PTR& operator= (const PTR&);                  // <--- line 9

    void* ptr;
};


struct XYZ
{
    XYZ (PTR& p) : ptr(p) {}

//  XYZ (const XYZ& src) : ptr(src.ptr) {}        // <--- line 19
//  XYZ& operator= (const XYZ& src) { ptr = src.ptr; return *this; }

    mutable PTR ptr;
};


XYZ f1 ();


XYZ f2 (void) { return f1(); }                    // <--- line 29
void f3 (XYZ& dst, const XYZ& src) { dst = src; } // <--- line 30

// end of example program ]


The compiler gives the following error messages:

z.CPP: In copy constructor `XYZ::XYZ(const XYZ&)':
z.CPP:8: error: `PTR::PTR(const PTR&)' is private
z.CPP:29: error: within this context
z.CPP: In member function `XYZ& XYZ::operator=(const XYZ&)':
z.CPP:9: error: `PTR& PTR::operator=(const PTR&)' is private
z.CPP:30: error: within this context



I think that the compiler should implicitly declare and define a copy
constructor and a copy assignment operator for XYZ as follows:
- The copy constructor should be declared as "XYZ::XYZ (const XYZ&);" since PTR
  has a (private) copy constructor whose first parameter is "const PTR&". (Cf.
  12.8/5; it doesn't matter if the copy constructor is accessible.)
- The copy assignment operator should be declared as "XYZ& XYZ::operator=
  (const XYZ&);" since PTR has a (private) copy assignment operator whose first
  parameter is "const PTR&". (Cf. 12.8/10; it doesn't matter if the copy
  assignment operator is accessible.)
- The implicitly defined copy constructor should use the copy constructor of
  PTR (12.8/8). Since the member ptr of XYZ is MUTABLE the (public) copy
  constructor PTR::PTR (PTR&) should be called after overload resolution:
  "PTR::PTR (PTR&)" is a better match than "PTR::PTR (const PTR&)".
- The same reasoning is true of the implicitly defined copy assignment
  operator.

By the way I don't get an error if I define the copy constructor and the copy
assignment operator explicitly in XYZ: please un-comment line 19 und 20 in the
code fragment above.


Kind regards
W. Roehrl
Comment 1 Andrew Pinski 2004-12-14 13:25:53 UTC
    XYZ (PTR& p) : ptr(p) {}

That will always call the constructor for ptr so this is invalid.
Comment 2 Giovanni Bajo 2004-12-14 13:41:30 UTC
No, this is a subtler issue. I need to think about it. Nathan may want to 
express an opinion on this issue too.
Comment 3 Nathan Sidwell 2004-12-14 13:57:59 UTC
The code is invalid and the compiler correct.

The following line of reasoning is incorrect
- The implicitly defined copy constructor should use the copy constructor of
  PTR (12.8/8). Since the member ptr of XYZ is MUTABLE the (public) copy
  constructor PTR::PTR (PTR&) should be called after overload resolution:
  "PTR::PTR (PTR&)" is a better match than "PTR::PTR (const PTR&)".

[7.1.1/9] is unclear about *when* the mutable storage class specifier
'nullifies a const specifier applied to the containing class'.  However,
it has always been my understanding (and edg 3.4 appears to agree), that
the mutable applies at the point of assignment, not before, such as at
overload resolution.  After all, overload resolution has no knowledge that
an assignment will actually occur to the const value.
Comment 4 Wolfgang Roehrl 2004-12-14 14:19:45 UTC
Subject: Antwort:  Copying objects with mutable non-static data
 members





Hi all,

I'm responding to  Comments From nathan at gcc dot gnu dot org  2004-12-14
13:57 (bug 18975):

I just tried the code fragment with the Comeau online compiler. The
compiler does generate implicitly the copy contructor but it refuses to
generate
implicitly the copy assignment operator. That means the Comeau compiler
translates the mentioned code fragment if line 20 is un-commented, i.e. if
we have an user-defined copy assignment operator. But it is not necessary
to un-comment line 19, i.e. we don't need an user-defined copy constructor.

Regards,
W. Roehrl

Comment 5 Nathan Sidwell 2004-12-14 16:55:37 UTC
(In reply to comment #4)
I can't tell what's being said here.  IIUC, you're saying that
Comeau (which uses the edg frontend) also rejects the code.
Comment 6 Nathan Sidwell 2004-12-15 14:44:41 UTC
I am dubious about my own analysis of this. Wolfgang sent me more detailed
comparison between g++ and comeau

Hi Nathan,

I try to explain what I meant with my last comment. I have three variations
of my code example and I tried to compile them with GNU 3.3 and Comeau
online.

===========================================================================

The following code is accepted by GNU as well as by Comeau:

struct PTR
{
    PTR ();
    PTR (PTR&);
    PTR& operator= (PTR&);

private:
    PTR (const PTR&);
    PTR& operator= (const PTR&);

    void* ptr;
};


struct XYZ
{
    XYZ (PTR& p) : ptr(p) {}

    XYZ (const XYZ& src) : ptr(src.ptr) {}
    XYZ& operator= (const XYZ& src) { ptr = src.ptr; return *this; }

    mutable PTR ptr;
};


XYZ f1 ();


XYZ f2 (void) { return f1(); }
void f3 (XYZ& dst, const XYZ& src) { dst = src; }

===========================================================================

The following code is rejected by GNU (GNU flags line 28) and accepted by
Comeau:

struct PTR
{
    PTR ();
    PTR (PTR&);
    PTR& operator= (PTR&);

private:
    PTR (const PTR&);
    PTR& operator= (const PTR&);

    void* ptr;
};


struct XYZ
{
    XYZ (PTR& p) : ptr(p) {}

    XYZ& operator= (const XYZ& src) { ptr = src.ptr; return *this; }

    mutable PTR ptr;
};


XYZ f1 ();


XYZ f2 (void) { return f1(); }                     // <--- this is line 28
void f3 (XYZ& dst, const XYZ& src) { dst = src; }

===========================================================================

The following code is rejected by both GNU (GNU flags line 26 and 27) and
Comeau (Comeau flags line 27):

struct PTR
{
    PTR ();
    PTR (PTR&);
    PTR& operator= (PTR&);

private:
    PTR (const PTR&);
    PTR& operator= (const PTR&);

    void* ptr;
};


struct XYZ
{
    XYZ (PTR& p) : ptr(p) {}

    mutable PTR ptr;
};


XYZ f1 ();


XYZ f2 (void) { return f1(); }                        // <--- this is line
26
void f3 (XYZ& dst, const XYZ& src) { dst = src; }     // <--- this is line
27

===========================================================================

Two remarks:
- In my first code fragment I have defined an explicit copy constructor and
  an explicit copy assigment operator for class XYZ, in my third code
fragment
  there is neither an explicit copy constructor nor an explicit copy
assigment
  operator for class XYZ.
  Why is the first code legal and the third code illegal? The functionality
  seems to be identical.
- The EDG frontend seems to treat the mutable attribute differently in the
  generation of the implicit copy constructor than in the generation of the
  implicit copy assignment operator.

With kind regards,
W. Roehrl
Comment 7 Giovanni Bajo 2004-12-17 13:19:35 UTC
Wolfgang, I suggest you to bring this up in comp.std.c++, to get some official 
answer about this.
Comment 8 Nathan Sidwell 2004-12-17 13:44:14 UTC
I now think the code is ok, and both compilers are incorrect to reject any of
the versions.

The versions that do not contain user defined 
  XYZ (const XYZ& src)
  XYZ& operator= (const XYZ& src)
functions are the problematic ones.  In those cases, the compiler should
synthesize suitable definitions of those functions.  It correctly determines
the CV qualifier of the SRC argument, but fails to generate the correct
body.  That body should be identical to the body supplied in the user-defined
version.  Namely copy construction or copy assignment of each member in turn.
Comment 9 Nathan Sidwell 2004-12-17 16:10:51 UTC
Fix for 3.4 branch
2004-12-17  Nathan Sidwell  <nathan@codesourcery.com>

	PR c++/18975
	* method.c (do_build_copy_constructor): Refactor. Don't const
	qualify a mutable field.
	(do_build_assign_ref): Likewise.
Comment 10 GCC Commits 2004-12-17 16:19:36 UTC
Subject: Bug 18975

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	nathan@gcc.gnu.org	2004-12-17 16:19:24

Modified files:
	gcc/cp         : ChangeLog method.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/other: synth1.C 

Log message:
	cp:
	PR c++/18975
	* method.c (do_build_copy_constructor): Refactor. Don't const
	qualify a mutable field.
	(do_build_assign_ref): Likewise.
	testsuite:
	PR c++/18975
	* g++.dg/other/synth1.C: New.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3892.2.185&r2=1.3892.2.186
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/method.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.275.4.4&r2=1.275.4.5
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3389.2.330&r2=1.3389.2.331
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/other/synth1.C.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=NONE&r2=1.1.2.1

Comment 11 Nathan Sidwell 2004-12-17 16:20:42 UTC
Created attachment 7770 [details]
patch

patch for mainline, apply when non-regressions can be fixed
Comment 12 Nathan Sidwell 2004-12-17 16:21:14 UTC
Created attachment 7771 [details]
testcase

testcase for the patch
Comment 13 Wolfgang Bangerth 2004-12-17 16:34:49 UTC
Nathan, if this isn't a regression but a patch has been applied to the 
3.4 branch, then you should also apply it to mainline. Otherwise you have 
just created a regression (3.4.4 will work as expected, but 4.0.0 will 
not). You therefore just came up with your own excuse to also commit the 
patch to present mainline (future 4.0.0). 
 
W. 
Comment 14 Steven Bosscher 2004-12-17 17:21:39 UTC
Nathan, this is stage3, you *are* allowed to apply non-regression fixes. 
By not applying it there you've just created a new 4.0 regression... 
Comment 15 Nathan Sidwell 2004-12-21 17:27:34 UTC
2004-12-21  Nathan Sidwell  <nathan@codesourcery.com>

	PR c++/18975
	* method.c (do_build_copy_constructor): Refactor. Don't const
	qualify a mutable field.
	(do_build_assign_ref): Likewise.
Comment 16 GCC Commits 2004-12-21 17:28:49 UTC
Subject: Bug 18975

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	nathan@gcc.gnu.org	2004-12-21 17:28:35

Modified files:
	gcc/cp         : ChangeLog method.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/other: synth1.C 

Log message:
	cp:
	PR c++/18975
	* method.c (do_build_copy_constructor): Refactor. Don't const
	qualify a mutable field.
	(do_build_assign_ref): Likewise.
	testsuite:
	PR c++/18975
	* g++.dg/other/synth1.C: New.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.4544&r2=1.4545
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/method.c.diff?cvsroot=gcc&r1=1.317&r2=1.318
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.4790&r2=1.4791
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/other/synth1.C.diff?cvsroot=gcc&r1=1.1&r2=1.2