7

I'm getting some very wierd errors. The compiler seems to want to call the copy constructor for some reason I don't understand.

(118) std::map<int, layer> xs;
(119) xs.begin()->first; // error?!

layer is a non-copyable, movable type.

class layer : public observable
{
    layer(const layer&);
    layer& operator=(const layer&);
public:
    layer(int index = -1);
    layer(layer&& other);
    layer& operator=(layer&& other);
   //...
};

For some reason the line 119 caused the compiler to try to invoke the copy constructor for std::pair, why?

1>c:\program files (x86)\microsoft visual studio 10.0\vc\include\utility(131): error C2248: 'layer::layer' : cannot access private member declared in class 'layer'
1> ..\layer.h(55) : see declaration of 'layer::layer'
1> ..\layer.h(53) : see declaration of 'layer'
1> c:\program files (x86)\microsoft visual studio 10.0\vc\include\utility(129) : while compiling class template member function 'std::_Pair_base<_Ty1,_Ty2>::_Pair_base(const std::_Pair_base<_Ty1,_Ty2> &)'
1> with
1> [
1>     _Ty1=const int,
1>     _Ty2=layer
1> ]
1> c:\program files (x86)\microsoft visual studio 10.0\vc\include\utility(174) : see reference to class template instantiation 'std::_Pair_base<_Ty1,_Ty2>' being compiled
1> with
1> [
1>     _Ty1=const int,
1>     _Ty2=layer
1> ]
1> ..\stage.cpp(119) : see reference to class template instantiation 'std::pair<_Ty1,_Ty2>' being compiled
1> with
1> [
1>     _Ty1=const int,
1>     _Ty2=layer
1> ]

I've also tried the following, where it fails similarly.

(118) std::map<int, layer> xs;
(119) auto& t1 = *xs.begin();
(120) auto& t2 = t1.first; // error?!

What is going on here?

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
ronag
  • 49,529
  • 25
  • 126
  • 221
  • Why would I need that? I'm just reading the value of the member variable "first". – ronag Dec 31 '11 at 17:22
  • `layer(int index = -1);` is already a public default constructor. – Mark B Dec 31 '11 at 17:23
  • 1
    Ok, so perhaps the private copy constructor is the issue, check this: http://stackoverflow.com/questions/1440287/how-to-create-a-container-of-noncopyable-elements The first error clearly indicates that an attempt to access layer::layer fails because it is private. – Yaniro Dec 31 '11 at 17:33
  • 1
    Your code sample compiles just fine for me (without observable base class) using Visual Studio 2010 SP1. – Nikola Smiljanić Dec 31 '11 at 17:34
  • @NikolaSmiljanić: Interesting, I don't see how any inherited class could cause this problem. I''m starting to lean towards compiler bug. – ronag Dec 31 '11 at 20:04
  • 3
    Calling `begin` on an empty collection yields UB. Can you post a **real** example? [SSCCE](http://sscce.org/) – ildjarn Dec 31 '11 at 21:03
  • @ildjarn: It might cause undefined behavior in run-time. But this is a compile-time problem, it should compile just fine. – ronag Jan 01 '12 at 02:07
  • 1
    That doesn't change the fact that this isn't a real example. If you actually want help then post something self-contained, not just two or three lines of code. – ildjarn Jan 01 '12 at 02:47
  • Compiles fine in mine VS 2010 too:) – Mr.Anubis Jan 01 '12 at 20:33
  • @ildjarn: Where does it say calling begin() on an empty behavior is UB? It simply returns the same iterator as end() in that case, such that std::distance(begin(), end()) returns 0. – Drew Hall Jan 04 '12 at 18:55
  • @Drew : Apologies, I meant _dereferencing_ `begin()` on an empty container, not merely calling it. – ildjarn Jan 04 '12 at 18:56
  • gcc compiles it just fine too. – n. m. could be an AI Jan 24 '12 at 18:52
  • The sample does not define observable. If I omit `: public observable` or add `class observable {};`, then the code compiles without an error message on MSVC 2010 SP1. Do you have SP1 installed? – Werner Henze Feb 01 '12 at 14:32

3 Answers3

3

This is one of the strange subtleties of template errors. Template code is not code, it's almost closer to a scripting language for generating code. You can even have a syntax error in a function that won't necessarily generate a compiler error until that function is used (directly or indirectly) by your code.

In this case xs.first() caused the generation of std::map<int, layer>::iterator, which also necessitates the generation of std::pair<int, layer>. The default implementation of std::pair has a copy constructor, which fails to compile.

You could get around this with a template specialization of std::pair which does not have the copy constructor, but then you can't insert anything into your map. xs[0] = myLayer creates and inserts std::make_pair<0, myLayer> into your map, which obviously requires the copy construction of a layer.

The typical solutions for this is to change your type to std::map<int, std::shared_ptr<layer> >. Copying a shared_ptr doesn't copy the referenced object.

Thomas Bouldin
  • 3,707
  • 19
  • 21
-1

It depends on where you initialize this member, and its first member. If you initialize it as a static member or on the stack without calling the constructor it will try and call the default contructor (without parameters) and won't be able to access it since you have made it private.

You have to explicitly call the public constructor for elements in your map

Giel
  • 397
  • 1
  • 9
-1

Your example:

(118) std::map<int, layer> xs;
(119) xs.begin()->first; // error?!

Per http://www.cplusplus.com/reference/stl/map/

xs.begin() Return iterator to beginning

... it->first; // same as (*it).first (the key value)

Therefore

xs.begin()->first;

is equivalent to

pair<int,layer> piltmp = (*xs.begin());
piltmp.first;

Et voila. A copy of the pair in the map has been created. Which involves creating a copy of the layer.

(This would not be a problem if the map held pointers or autopointers to layer, rather than a layer itself.)

Now, this would not be happening if map::iterator::operator-> returned a value_type reference, rather than a value_type. I.e. if it returned an lvalue rather than an rvalue. Seems strange that it does not, but I have not worked my way through the standard.

You might get around it by doing

pair<int,layer>& piltmp = *xs.begin();
return piltmp.first;

(Not tested.)

Krazy Glew
  • 7,210
  • 2
  • 49
  • 62
  • No, `xs.begin()->first` is equivalent to `std::map::iterator it(xs.begin()); it->first;`. The iterator points to a `std::pair` and not to a `layer`. – rve Jan 24 '12 at 07:54
  • Fair enough. The first example becomes piltmp = (*xs.begin(); piltmp.first. Yes, it was a an error in my example, but the point remains: there apparently is a copy of the the pair in the map being. And the pair contains a layer, so the layer is being copied. If it was a reference, an lvalue, there would be no problem. /// Which reminds me: the question poser might conside making the map hold, not layers, but pointers or autoptrs to layers. – Krazy Glew Jan 24 '12 at 18:36
  • In your example you are right a copy is made. But your example is not the same as `xs.begin()->first`. It is equivalent to `pair &tmp(*xs.begin()); tmp.first;`. The iterator is copied and not what the iterator points to. `operator*()` on the iterator returns a reference to the item in the map which is a `std::pair`. And `operator->()` on the iterator returns a pointer to the item in the map and not a `value_type` as you mention in your answer. – rve Jan 24 '12 at 21:31
  • Agreed, that is what should happen. I was just grasping at why the copy constructor was being invoked. What is your correct explanation? – Krazy Glew Jan 25 '12 at 04:04
  • As the comments under the question mention, other have compiled it without problems so it is probably something on his machine or code. – rve Jan 25 '12 at 11:54
  • This answer is incorrect. `a->b` does not make a copy of `a`; it operates directly on `a`. That's kind of the point. If it copied then `something->setValue(42);` wouldn't achieve anything. – Karu Feb 26 '12 at 01:24