Are copy constructors defined implicitly always, or only when they are used?
Asked Answered
M

3

8

Consider the following code:

#include <memory>
#include <vector>

class A
{
private:
  std::vector<std::unique_ptr<int>> _vals;
};

int main()
{
  A a;
  //A a2(a);
  return 0;
}

Compiler A compiles this without issue unless I uncomment out the line A a2(a); at which point it complains about the copy constructor for std::unique_ptr being deleted, and therefore I can't copy construct A. Compiler B, however, makes that complaint even if I leave that line commented out. That is, compiler A only generates an implicitly defined copy constructor when I actually try to use it, whereas compiler B does so unconditionally. Which one is correct? Note that if I were to have used std::unique_ptr<int> _vals; instead of std::vector<std::unique_ptr<int>> _vals; both compilers correctly implicitly delete both copy constructor and assignment operator (std::unique_ptr has a explicitly deleted copy constructor, while std::vector does not).

(Note: Getting the code to compile in compiler B is easy enough - just explicitly delete the copy constructor and assignment operator, and it works correctly. That isn't the point of the question; it is to understand the correct behavior.)

Megdal answered 21/6, 2018 at 17:26 Comment(19)
This copy constructor reference might help you.Salzburg
Which compiler is compiler B? Feel free to use the real name of the compilers you are using.Gatehouse
Out of curiosity, which are compilers A and B?Tendance
Compiler A is GNU 7.1, compiler B is various versions of the Intel compilerMegdal
I can't repro with any intel compiler here... Are you using some very old version?Supporting
The compiler should not give you an error on a comment. If it does it has a bug.Bradbradan
@HolyBlackCat: Now that you mentioned that, it only appears to happen on the Intel compiler on Windows - when I try it using the Intel compiler on Linux, it works correctly... That is a bit bizarre to me. I am using the latest version of the compiler (18.3) that is outMegdal
Please use gcc.godbolt.org and share the link which demonstrates error with icc. Generating error with commented out line is a showstopper bug for any decent compiler.Isoagglutination
@Megdal Are you specifying the C++ standard version explicitly or allowing the compiler to choose?Kristinakristine
@NathanOliver: It's not generating the error on the comment, but on the implicitly generated copy constructor. If I don't uncomment the particular line in question, I never actually call the copy constructor, and GNU 7.1 doesn't generate one. Intel, though (only on Windows, and only when I declare class A a DLL export), generates a copy constructor regardless of whether I need it.Megdal
@SergeyA: As I mentioned in another comment, it's only on Windows, and only when I explicitly list class A as a DLL export (via, e.g., class __declspec(dllexport) A) that this happens. That might have been a clue to me...Megdal
@ZanLynx: I explicitly specify /Qstd=c++11 (Intel Windows version of std=c++11)Megdal
@Megdal That is information that should be in the question. Your "mvce" doesn't represent what you are actually doing.Bradbradan
@Megdal if you are looking for any sensible conversation, you need to provide all those details in MCVE.Isoagglutination
@SergeyA: This is why I left it as "compiler A" and "compiler B" in the question - I'm not looking to see if anyone else can reproduce the issue, I'm looking to see which one is right.Megdal
@Megdal MCVE should be provided with 'who is right' questions as well. I am still not convinced your analysis is correct, like I said before, this would be a showstopper bug with the compiler.Isoagglutination
@SergeyA: I guess I don't really care if anyone's "convinced" that I found this? How is that at all relevant to the question? Surely that's something that I would have to take up with Intel support?Megdal
@NathanOliver: That information is tangential to the problem - it is highly relevant if and when I file a bug report against a specific compiler, but it is not relevant to whether the attached program should compile. Being on Windows or Linux (or exporting something or importing something) does not affect the C++ standard of copy constructors.Megdal
Since it only happens when using __declspec(dllexport), that should be part of the MCVE. (Especially if there is a chance that declaring a class for export causes its implicitly defined constructors to be instantiated for the DLL interface.)Groove
G
9

From [class.copy.ctor]/12:

A copy/move constructor that is defaulted and not defined as deleted is implicitly defined when it is odr-used ([basic.def.odr]), when it is needed for constant evaluation ([expr.const]), or when it is explicitly defaulted after its first declaration.

A's copy constructor is defaulted, so it's implicitly defined only when it is odr-used. A a2(a); is just such an odr-use - so it's that statement that would trigger its definition, that would make the program ill-formed. Until the copy constructor is odr-used, it should not be defined.

Compiler B is wrong to reject the program.

Grounds answered 21/6, 2018 at 17:53 Comment(1)
Your conclusion is true iff * compiler B* rejects the code as it is actually written in the question. Since the comments reveal that __declspec(dllexport) is also in the game, I think things might be a little less trivial.Gylys
G
3

Note: My answer is based on your comment:

[...] it's only on Windows, and only when I explicitly list class A as a DLL export (via, e.g., class __declspec(dllexport) A) that this happens. [...]

On MSDN we can learn that declaring a class dllexport makes all members exported and required a definition for all of them. I suspect the compiler generates the definitions for all non-deleted functions in order to comply with this rule.

As you can read here, std::is_copy_constructible<std::vector<std::unique_ptr<int>>>::value is actually true and I would expect the supposed mechanism (that defines the copy constructor in your case for export purposes) checks the value of this trait (or uses a similar mechanism) instead of actually checking whether it would compile. That would explain why the bahviour is correct when you use unique_ptr<T> instead of vector<unique_ptr<T>>.

The issue is thus, that std::vector actually defines the copy constructor even when it wouldn't compile.

Imho, a is_copy_constructible check is sufficient because at the point where your dllexport happens you cannot know whether the implicit function will be odr-used at the place where you use dllimport (possibly even another project). Thus, I wouldn't think of it as a bug in compiler B.

Gylys answered 21/6, 2018 at 20:20 Comment(4)
There are times though, where the compiler will inline something from a header in a different project - wouldn't this be a perfect use case for that? I.e., if I don't declare the copy constructor or odr-use it within my module, don't include at compile time - instead, inline it into any module that does odr-use it. To me, that's a perfectly valid fix (especially as an implicitly defined copy constructor should be fairly simple) - would that cause any other issues?Megdal
@R_Kapp: How will the other project know whether you (or your compiler) defined the copy constructor in your dll or whether it should generate one "inline" if and only if there is an odr-use? If the definition is in the header file it can be inlined but if the function is implicit you cannot tell at compiler time of project Y whether dll X ships a definition or not. Since As copy constructor is not implicitly deleted because that of std::vector isn't deleted either, an implicit definition should be generated.Gylys
That's a fair point... I suppose an easier fix would be to always inline (and not export) implicitly defined copy constructors. This way, you won't violate ODR (nothing's ever exported), and it is still only defined when it is ever actually odr-used.Megdal
(Feel free to tell me I should ask this as a separate question if you'd prefer) Also, how does this work on Linux with GNU? The class is listed as "for export" there as well, and I'm not getting that function to be generated implicitly. How would that work if I were to use it in another module/project? My guess is that it would do something like what I've suggested above (i.e., inline it always if it's implicit), but there might be some specific differences between the two OSes that I'm not aware of.Megdal
I
0

While I can't confirm this behavior (I do not have access to Windows compiler, and OP claims the bug happens with icc on Windows platform), taking the question at it's face value, the answer is - compiler B has a gross bug.

In particular, implicitly-declared copy constructor is defined as deleted, when ...

T has non-static data members that cannot be copied (have deleted, inaccessible, or ambiguous copy constructors);

https://en.cppreference.com/w/cpp/language/copy_constructor

Thus, conforming compiler must semantically generate deleted copy-constructor, and successfully compile the program since such constructor never called.

Isoagglutination answered 21/6, 2018 at 17:40 Comment(5)
std::vector is copyable, though. As I mention in the question, if I replace std::vector<std::unique_ptr<int>> _vals with std::unique_ptr<int> _vals, I don't get the issue - std::unique_ptr isn't copyable and therefore the copy constructor is correctly implicitly deleted. std::vector, which is what the compiler has, though, is copyable.Megdal
@Megdal why do you think std::vector of std::unique_ptrs is copyable?Isoagglutination
@R_Kapp: std::vector is not a type. std::vector<std::unique_ptr<int>> is not copyable.Permission
std::vector<T> has a copy constructor defined. It is not deleted, inaccessible, or ambiguous. It so happens that the template parameter that it uses in this case (std::unique_ptr) is not copyable, but that doesn't change the definition of std::vectorMegdal
See this link as evidence that std::vector<std::unique_ptr<int>> does not match the description provided in this answer.Megdal

© 2022 - 2024 — McMap. All rights reserved.