8

I was halfway through working on this piece of code and thought this is obviously not going to compile before hitting the build button. I was surprised that it not only compiled, but linked and worked as well.

If I were to guess I would say that SFINAE is responsible for it compiling... is it?

struct BaseClass
{
public:
  BaseClass() {}

  template<typename T>
  BaseClass(const T& a_other)
  {
    int i = 0; // for break point
  }

  template<typename T>
  BaseClass& operator= (const T& a_other)
  {
    int i = 0; // for break point
    return *this;
  }

private:

  BaseClass(const BaseClass& a_other); // Does not have a definition
  BaseClass& operator= (const BaseClass& a_other); // Does not have a definition

};

struct MyClass : public BaseClass
{
};

int main()
{
  MyClass i, j;
  i = j;

  return 0;
}

EDIT: I am using Visual-C++ 2008, maybe it is an odd quirk of VS

Samaursa
  • 16,527
  • 21
  • 89
  • 160
  • Which part are you asking about, specifically? There's the template functions, and there's the undefined copy and assignment operators, any of which I could imagine somebody expecting some kind of failure with. – Greg Hewgill Feb 27 '12 at 21:23
  • @GregHewgill: Line `i = j` ... should it not simply tell me that the assignment operator is private? – Samaursa Feb 27 '12 at 21:24
  • I think this question is relevant: http://stackoverflow.com/questions/8984013/can-sfinae-detect-private-access-violations. – Oliver Charlesworth Feb 27 '12 at 21:34
  • 2
    This doesn't compile with gcc 4.6.1, C++11 enabled: The error(s) says: In function 'int main()' ... use of deleted function 'MyClass& MyClass::operator=(const MyClass&) MyClass& MyClass::operator=(const MyClass&)' is implicitly deleted because the default definition would be ill-formed:| BaseClass& BaseClass::operator=(const BaseClass&)' is private| – jrok Feb 27 '12 at 21:41
  • 1
    It must not compile. If it does, it is a VC++ bug. – Maxim Egorushkin Feb 27 '12 at 21:48

5 Answers5

3

The code is not legal.

i = j calls the implicitly defined copy assignment operator in MyClass. This function calls the copy assignment operator for each of its sub-objects, including direct base classes [class.copy 12.8 p28].

If you add code to the copy assignment operator for BaseClass you can see where VS is going wrong:

  template<typename T>
  BaseClass& operator= (const T& a_other)
  {
    std::cout << typeid(T).name() << '\n';
    int i = 0; // for break point
    return *this;
  }

For me this prints out "struct MyClass". VS is calling the BaseClass copy assignment operator by passing the parameter received in MyClass:operator= directly, rather than just the BaseClass sub object of j.

SFINAE doesn't come into play because the template functions aren't failing. VS is simply generating the implicit copy assignment operator incorrectly.

To sum up: VS is generating the implicit copy assignment operator as

MyClass &operator=(const MyClass& rhs) {
    static_cast<BaseClass&>(*this).operator=(rhs);
    return *this;
}

When it should be:

MyClass &operator=(const MyClass& rhs) {
    static_cast<BaseClass&>(*this).operator=(static_cast<const BaseClass&>(rhs));
    return *this;
}
bames53
  • 86,085
  • 15
  • 179
  • 244
  • Looks like a bug has just been opened: http://connect.microsoft.com/VisualStudio/feedback/details/726856/vc-incorrectly-copy-assigns-direct-base-sub-objects-in-implicitly-defined-copy-assignment-operator#details – Michael Burr Feb 28 '12 at 07:47
1

Shot in the dark: the compiler instantiates the base class’ operator = with T = MyClass. I do now know whether this is legal or even required but it makes a certain kind of sense: the auto-generated code for operator = essentially looks like this (well, pseudo-code):

MyClass& operator =(MyClass const& other) {
    BaseClass::operator =(other);
    return *this;
}

Now the compiler finds that BaseClass::operator =<MyClass>(MyClass const&) is the best match and instantiates it.

Konrad Rudolph
  • 530,221
  • 131
  • 937
  • 1,214
  • That's what I thought, too. But I'm not sure this is correct behaviour. – Christian Rau Feb 27 '12 at 21:43
  • 1
    @Christian Given that the GCC doesn’t compile this, I’d bargain that it’s in fact faulty. But I’m not sure. And the standard is complicated enough to make this non-obvious. – Konrad Rudolph Feb 27 '12 at 21:45
0

Well, since it cannot call BaseClass::operator=(const BaseClass&) (from the default generated MyClass::operator= for what it's worth) because it's private, it just calls template<typename T> BaseClass::operator(const T &) with T=BaseClass. So there is no call to a non-defined function.

I guess the situation would be different if the others were public, in which case the compiler would prefer those over the template, but since he cannot see them when being private, the template versions match equally well.

Christian Rau
  • 45,360
  • 10
  • 108
  • 185
  • 2
    But isn't name resolution done *before* public/private is considered? (This is a genuine question; I thought I had read that somewhere.) – Oliver Charlesworth Feb 27 '12 at 21:32
  • So in this case, it is following the SFINAE technique? – Samaursa Feb 27 '12 at 21:33
  • @Samaursa I don't think SFINAE (at least the use of the term I know) has anything to do with this. It is just because the compiler cannot see/use the private versions, but they are still exisiting, just not called. – Christian Rau Feb 27 '12 at 21:34
  • @OliCharlesworth Don't know, would have thoguht so, too maybe. But this is the only explanation I could come up with. The other answers don't really address the problem at all. – Christian Rau Feb 27 '12 at 21:36
  • I see... the reason I thought it may be SFINAE is because a non-template method should be the given priority over template method, no? If yes, then the compiler should compile it and should report that I am trying to access a private method. – Samaursa Feb 27 '12 at 21:38
  • @Samaursa Yes, that's also what Oli is after. Seems strange indeed. But SFINAE means that a function is not considered for template instantiation (or the instantiation doesn't actually exist) if the instantiation would cause a compile error. This doesn't happen here anywhere. – Christian Rau Feb 27 '12 at 21:39
0

The compiler will auto generate a (default) copy constructor for MyClass as one is not defined. If you change i and j's type from MyClass to BaseClass you will see the error you expected, as the compiler then tries to bind the private, unimplemented assignment operator.


Going into this a little deeper using MSVC 2010 Ultimate SP1, we can see the exact reason:

MyClass::operator=:
00201230  push        ebp  
00201231  mov         ebp,esp  
00201233  push        ecx  
00201234  mov         dword ptr [ebp-4],ecx  
00201237  mov         eax,dword ptr [__that]  
0020123A  push        eax  
0020123B  mov         ecx,dword ptr [this]  
0020123E  call        BaseClass::operator=<MyClass> (202130h)  
00201243  mov         eax,dword ptr [this]  
00201246  mov         esp,ebp  
00201248  pop         ebp  
00201249  ret         4 

the assignment operator is called for MyClass by BaseClass::=<T> using MyClass as the type through MyClasses default copy operator, whether this is a bug or an MSVC-ism is up to MS and the C++ standard to decide.

Necrolis
  • 25,836
  • 3
  • 63
  • 101
  • But I can break into `BaseClass`'s `operator=()` (hence the comment _for break point_), at least in Visual-C++. If it generates a default one, then why is the base assignment operator being called? – Samaursa Feb 27 '12 at 21:29
  • 2
    But the default-generated copy-constructor invokes the base-class copy constructor... – Oliver Charlesworth Feb 27 '12 at 21:32
  • @Samaursa: if you actually trace into the breakpoint you set, you see it return to the auto generated function `MyClass::operator=:` (at least thats what I get under MSVC 2010). – Necrolis Feb 27 '12 at 21:37
  • Yes, I see the same behavior. But it _is_ going to the base class overload instead of giving an error since I am trying to access a private assignment operator (the private operator and constructor should be chosen first because they are non-template versions). – Samaursa Feb 27 '12 at 21:40
  • 1
    @Samaursa: the compiler chose the only option visible to it, which is the public templated assignment operator. – Necrolis Feb 27 '12 at 21:46
0

MyClass will use an implicitly-defined copy assignment operator, since there's no user-declared version. Ac cording to the standard, that implicit copy-assignment will perform the following (C++03 12.8/13 "Copying class objects"):

  • Each subobject is assigned in the manner appropriate to its type: — if the subobject is of class type, the copy assignment operator for the class is used (as if by explicit qualification; that is, ignoring any possible virtual overriding functions in more derived classes);

Note that in 12.8/9 the standard defines a user-declared copy assignment operator as " a non-static non-template member function of class X with exactly one parameter of type X, X&, const X&, volatile X& or const volatile X&" (emphasis mine).

So according to the standard, the template function

  template<typename T>
  BaseClass& operator= (const T& a_other);

Should not be called by the MyClass implicit copy assignment operator. MSVC is acting in a non-standard fashion here. GCC rightly diagnoses this:

C:\temp\test.cpp: In member function 'MyClass& MyClass::operator=(const MyClass&)':
C:\temp\test.cpp:29:14: error: 'BaseClass& BaseClass::operator=(const BaseClass&)' is private
C:\temp\test.cpp:33:8: error: within this context
C:\temp\test.cpp: In function 'int main()':
C:\temp\test.cpp:40:7: note: synthesized method 'MyClass& MyClass::operator=(const MyClass&)' first required here 
Michael Burr
  • 333,147
  • 50
  • 533
  • 760