[C++] Copy constructor optimization

It shouldn't. I tested it under Borland Dev Studio 2006 and it outputs an error:

[C++ Error] Unit1.cpp(21): E2285 Could not find a match for 'Foo::Foo(Foo)'

VS 2008 doesn't output any errors.

Because you did not specify a copy constructor for Foo, Foo has an implicit public copy constructor which is called. This copy constructor has to call NoCopy's copy constructor, respectively, which you declared private. So, no... the compiler shouldn't allow that.

Private copy constructor is something I've used in the past with gcc too, and I'm quite sure it worked as intended (i.e. it bailed out).

Quote:

Original post by samoth Because you did not specify a copy constructor for Foo, Foo has an implicit public copy constructor which is called. This copy constructor has to call NoCopy's copy constructor, respectively, which you declared private. So, no... the compiler shouldn't allow that.

But if I did specify a public copy constructor for Foo, the compiler would be allowed to optimize away the call to the copy constructor. So the question remains why the optimization isn't allowed in my example.

I don't think the syntactic correctness of code should depend on compiler-specific optimizations.

Your attempt to use a private copy should be an error, even if the compiler would be later able to remove the copy altogether.

I think the compiler is allowed to do it. The relevant section of the standard is:


When certain criteria are met, an implementation is allowed to omit the copy construction of a class object, even if the copy constructor and/or destructor for the object have side effects. In such cases, the implementation treats the source and target of the omitted copy operation as simply two different ways of referring to the same object, and the destruction of that object occurs at the later of the times when the two objects would have been destroyed without the optimization. This elision of copy operations is permitted in the following circumstances (which may be combined to eliminate multiple copies):

— in a return statement in a function with a class return type, when the expression is the name of a non-volatile automatic object with the same cv-unqualified type as the function return type, the copy operation can be omitted by constructing the automatic object directly into the function’s return value

— when a temporary class object that has not been bound to a reference (12.2) would be copied to a class object with the same cv-unqualified type, the copy operation can be omitted by constructing the temporary object directly into the target of the omitted copy

In the following code the compiler is allowed by the standard to optimize away the call to the copy constructor. The optimization is not just a compiler specific optimization. Note that applying the optimization results in different behaviour.



So why isn't the compiler allowed to do the same optimization in my first example?

Quote:

Original post by crocomire So why isn't the compiler allowed to do the same optimization in my first example?

I think it is, which is why it compiles. :)

I tried my example with the online Comeau C/C++ compiler:

Your Comeau C/C++ test results are as follows:

Comeau C/C++ 4.3.10.1 (May 29 2008 09:37:15) for ONLINE_EVALUATION_BETA1
Copyright 1988-2008 Comeau Computing. All rights reserved.
MODE:strict errors C++ noC++0x_extensions

"ComeauTest.c", line 11: error: "NoCopy::NoCopy(const NoCopy &)" (declared at line
4) is inaccessible
class Foo : private NoCopy
^
detected during implicit generation of "Foo::Foo(const Foo &)" at
line 20

1 error detected in the compilation of "ComeauTest.c".

In strict mode, with -tused, Compile failed

Quote:

Original post by crocomire But if I did specify a public copy constructor for Foo, the compiler would be allowed to optimize away the call to the copy constructor.

Yes, that's right, if your constructor doesn't initialize the base class, such as in:
Foo(const Foo&) {}

However, a "correct" copy constructor the way the compiler implements it would call the base constructor(s), somewhat like this:
Foo(const Foo&) : NoCopy() {}

Of course, NoCopy doesn't have any data members, and it's not meant to actually do anything, so it is silly to try to initialize something. However, that's because you know what's intended, the compiler doesn't know, to start with. The compiler can't just assume that it doesn't matter whether or not it initializes something just because it feels like it.

In the end, any decent compiler would finally decide that the constructor is useless and eliminate it, but access checking has to occur before any optimizations take place, because the first thing a compiler has to make sure is that a program is valid.

Quote:

Original post by samoth The compiler can't just assume that it doesn't matter whether or not it initializes something just because it feels like it.

It can, because the standard says it can.

The fact that it compiles on some platforms and not others is in the wording of the standard. It says an implementation "is allowed" to do it, not that it is required to do it.

Visual Studio is doing nothing that it is not allowed to do. It is not changing the meaning of the program in any way that is not allowed by the standard. It is not subverting the access protection for the base class copy constructor, it is not calling the copy constructor. The paragraph I quoted from the standard in my previous post says it is allowed to do that in certain circumstances, this is one of them.

Quote:

Original post by FFFF0000 It can, because the standard says it can.

That's funny, because my copy of the standard says:
Even when the creation of the temporary object is avoided (12.8), all the semantic
restrictions must be respected as if the temporary object was created. [Example: even if the copy constructor
is not called, all the semantic restrictions, such as accessibility (clause 11), shall be satisfied.]

Maybe I'm reading it wrong, but to me it isn't obvious that what Visual Studio is doing there is right.

For the code to be valid per the standard, it needs to have an accessible copy constructor, simply because a compiler that is allowed to perform an optimization is not mandated to do so.

The compiler is also allowed, similarly, to detect instances of guaranteed undefined behaviour at compile-time and cause something useful to happen at runtime - for example, dereferencing a NULL pointer is allowed to throw an exception, for the reason that it's allowed to do *anything*. That doesn't make the code OK, however.

Quote:

Original post by samoth Quote: Original post by FFFF0000 It can, because the standard says it can. That's funny, because my copy of the standard says: Even when the creation of the temporary object is avoided (12.8), all the semantic restrictions must be respected as if the temporary object was created. [Example: even if the copy constructor is not called, all the semantic restrictions, such as accessibility (clause 11), shall be satisfied.] Maybe I'm reading it wrong, but to me it isn't obvious that what Visual Studio is doing there is right.

Well I would argue that the accessibility restrictions are met for the copy constructor for the type of the class being optimized out. Its copy constructor is public, it is accessible.

The only way for the compiler to know if the optimization is meeting the accessibility restrictions of the base class is if it has the code for the copy constructor of the derived class visible. This will not always be the case. I doubt the standard would intend to place such a restriction on the optimization.

As with most things though, it comes down to interpretation.

Quote:

Original post by Zahlman For the code to be valid per the standard, it needs to have an accessible copy constructor, simply because a compiler that is allowed to perform an optimization is not mandated to do so.

What does "accessible" mean in this context?

What about this example:

Uncopiable foo(Uncopiable uc){    //...    return uc;}//...    Uncopiable x;    x = foo(x);

Are you saying that if the compiler realizes that it doesn't need to make any copies here, this faulty code is allowed to be compiled?

It seems unbelievable that the syntactic correctness of some piece of code would depend on how good your compiler is at optimizations. (I think this is the point of 12.2.1 quoted above, which also seems to make VC++ wrong: even though it can avoid the temporary, the code explicitly tries to use the private copy constructor. The compiler should not be allowed to silently "correct" your code.)

Quote:

Original post by FFFF0000 Well I would argue that the accessibility restrictions are met for the copy constructor for the type of the class being optimized out. Its copy constructor is public, it is accessible.

Well, but you see... omitting a copy constructor is an optimization.
Optimizations are done only after the program has been parsed, syntax-checked, and otherwise checked for validity.
So, whether or not the compiler skips the copy constructor or any empty base constructors later, it still has to presume it's calling them, initially. And it still has to fail if it can't access them. Otherwise it can't be sure the program is valid at all. Violating access rules is a program error, and a program with errors must not compile.
Basically, that's what the standard says, in other words (at least, it's how I read it).

Well the way I see it, the intention of the author of the class is that the copy constructor of the base class should not be called. The compiler is not breaking that intention.

Given that the copy constructor is unlikely to be in the same translation unit as the offending code, the compiler has to make some assumptions. If the class has a user defined copy constructor, then the compiler can safely assume that the copy constructor is valid. If the user defined copy constructor was not valid then the translation unit that contained the user defined copy constructor would not compile. And so the compiler can safely apply the optimization in this case.

The second case is the copy constructor being implicitly declared or defined. In this case the compiler has to generate a copy constructor for the class if one is used. In the example given, it would be the copy constructor that would fail to compile, as it would try to access the base class copy constructor. The offending code simply serves to generate that copy constructor.

So it looks like your argument is that the compiler should attempt to optimize the call to the copy constructor out after it has generated the copy constructor. My argument is that it should attempt to optimize the call to the copy constructor out before it tries to generate one.

Now given Visual Studios implementation, they have provided a way to prevent Foo f(Foo(42)); from compiling. You simply need to make the derived class copy constructor private. To me that would increase readability. The class may also work with unmodifiable library code that might semantically look like it requires a copy, but in fact can be implemented without it. This will increase the reuse of your class, though the code would not be portable.

Without the VS implementation, your code would not be able to work with the same library code. On the positive side though, it would force uses such as Foo f(Foo(42)); to be cleaned up.

Summing both up, I would prefer the VS implementation. It allows you to stop Foo f(Foo(42)); from compiling by making the Foo copy constructor private, and it could possibly work with library code that the non-VS implementation could not.

Quote:

Now given Visual Studios implementation, they have provided a way to prevent Foo f(Foo(42)); from compiling. You simply need to make the derived class copy constructor private. To me that would increase readability.

I thought the whole point of inheriting it from NoCopy was to make attempts of copy like that illegal. That's also what boost::noncopyable is for, so that you wouldn't have to declare a private copy constructor/operator= in every class you don't want to be copyable.

Since it seems that many other compilers disagree with VC++, you should simply avoid lines like that (looks stupid anyway).

VS 2008 is broken:

#include <boost/utility.hpp>class Foo : boost::noncopyable {public:	Foo(int) {}};int main(){	Foo f1(Foo(5));      // this compiles	Foo f2(Foo(Foo(5))); // this does not}