[Bug c++/16915] New: unnecessary copy construction

[Bug c++/16915] New: unnecessary copy construction

[boris at kolpackov dot net]

$ cat scope_lock.cxx

#include <iostream>

using std::cerr;
using std::endl;

struct lock_base
{
protected:
  lock_base ()
  {
  }

private:
  lock_base (lock_base const&);
};

template <typename T>
struct lock_ : lock_base
{
  lock_ (T&)
  {
    cerr << "locking" << endl;
  }

  lock_ (lock_ const&)
  {
    cerr << "copy" << endl;
  }

  ~lock_ ()
  {
    cerr << "unlocking" << endl;
  }
};

typedef lock_base const& lock;

struct mutex
{
  operator lock_<mutex> ()
  {
    cerr << "operator lock_<mutex>" << endl;
    return lock_<mutex> (*this);
  }
};

template <typename T>
lock_<T> flock (T& t)
{
  return lock_<T> (t);
}


int
main ()
{
  mutex m;
  
  {
    lock l (m);
    
    cerr << "end of block" << endl;
  }
}

$ g++-3.4 --version
g++-3.4 (GCC) 3.4.1 (Debian 3.4.1-5)

$ g++-3.4 scope_lock.cxx
scope_lock.cxx: In function `int main()':
scope_lock.cxx:20: error: `lock_base::lock_base(const lock_base&)' is private
scope_lock.cxx:67: error: within this context

Strangely, gcc tries to perform derived-to-base conversion and copy construction
when just derived-to-base conversion is already enough. The same code compiles
fine with icc 8.

-- 
           Summary: unnecessary copy construction
           Product: gcc
           Version: 3.4.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: boris at kolpackov dot net
                CC: gcc-bugs at gcc dot gnu dot org
 GCC build triplet: i686-pc-linux-gnu
  GCC host triplet: i686-pc-linux-gnu
GCC target triplet: i686-pc-linux-gnu


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

[Bug c++/16915] unnecessary copy construction

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-08-07 21:40 -------
Invalid as "operator lock_<mutex>" returns a temporary so the lock_base l(temp) requires that the 
copy constructor to be accessible.

Read <http://gcc.gnu.org/gcc-3.4/changes.html>.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID


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

[Bug c++/16915] unnecessary copy construction

[boris at kolpackov dot net]

------- Additional Comments From boris at kolpackov dot net  2004-08-09 15:21 -------
pinskia at gcc dot gnu dot org <gcc-bugzilla@gcc.gnu.org> writes:

> Invalid as "operator lock_<mutex>" returns a temporary so the
> lock_base l(temp) requires that the copy constructor to be accessible.

According to ([dcl.init.ref]/5, bullet 2, sub-bullet 1) it should be
lock_(lock_ const&), not lock_base(lock_base const&). As you can see
lock_(lock_ const&) is accessible.

Also what is so special about this case that gcc does not eliminate
the temporary?

-- 


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

[Bug c++/16915] unnecessary copy construction

[bangerth at dealii dot org]

------- Additional Comments From bangerth at dealii dot org  2004-08-09 15:29 -------
lock::lock(const lock&) tries to use the copy constructor 
of the base class, but that is inaccessible. 
 
What the standard says is basically that the copy constructor 
must be accessible. Whether the compiler actually uses it, or gets 
rid of the temporary by direct copy, is up to the compiler implementation, 
but it has to check the accessibility anyway. gcc doesn't generate the 
temporary in many cases, btw. 
 
W. 

-- 


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

[Bug c++/16915] unnecessary copy construction

[boris at kolpackov dot net]

------- Additional Comments From boris at kolpackov dot net  2004-08-09 15:45 -------
> lock::lock(const lock&) tries to use the copy constructor 
> of the base class, but that is inaccessible.

Have you read the code?

-- 


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

[Bug c++/16915] unnecessary copy construction

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-08-09 15:49 -------
Note even if the temporary is gotten rid of (in this case yes it is) the copy constructor still needs to be 
accessible, read the page which I gave.

-- 


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

[Bug c++/16915] unnecessary copy construction

[boris at kolpackov dot net]

------- Additional Comments From boris at kolpackov dot net  2004-08-09 16:15 -------
> Note even if the temporary is gotten rid of (in this case yes it is)

No, it is not. If you replace the following two lines 

private:
  lock_base (lock_base const&);

in the code above with

public:
  lock_base (lock_base const&)
  {
    cerr << "lock_base copy ctor" << endl;
  }

You will see the following output:

operator lock_<mutex>
locking
lock_base copy ctor
unlocking
end of block

> the copy constructor still needs to be accessible

As I said above the copy constructor that should be called (lock_ (lock_
const&)) is accessible.

> read the page which I gave.

I already did. For your convinience I will cite part of the standard that the
page you recommended refers to (if you don't have time to read the whole quote
just read the commend in the example at the end):

8.5.3/5 bullet 2, sub-bullet 1, sub-sub-bullet 2:

  A temporary of type  cv1 T2  [sic] is created, and a constructor is called to
copy the entire rvalue object into the temporary. The reference is bound to the
temporary or to a subobject within the temporary. 

  The constructor that would be used to make the copy shall be callable whether
or not the copy is actually done. 

  [Example: 

   struct A { }; 
   struct B : public A { } b; 
   extern B f();
   const A& rca = f(); // Either bound to the A subobject of the B rvalue,
                       // or the entire B object is copied and the reference
                       // is bound to the A subobject of the copy
  --end example]


-- 


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

[Bug c++/16915] unnecessary copy construction

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-08-27 08:57 -------
Note this is still invalid as operator lock_<mutex> returns a rvalue and lock (which is a constant 
reference to lock_base) is assigned that rvalue as we are going to use the copy constructor of 
lock_base instead of lock_<mutext>.

-- 


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

[Bug c++/16915] unnecessary copy construction

[boris at kolpackov dot net]

------- Additional Comments From boris at kolpackov dot net  2004-08-27 15:37 -------
Didn't quite get what is invalid. Is it invalid to use lock_base copy-ctor
instead of lock_<mutex> -or- the code is invalid because lock_base copy-ctor
should be used. If the former then I am glad we finally agree. If the latter
then what about the example from the standard, is it invalid too? I don't see
any semantical differences between "extern B f ()" and "operator lock_<mutex>
()" - they both return rvalue.



-- 


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