15

I'm on Visual Studio 2017. Recently, because I didn't like the non-conforming standards to C++ I went ahead and disabled the non-standard language extensions in the options. So far so good. Now I have a problem.

#include <iostream>
#include <vector>


struct Vertex
{
    Vertex(float pos) { }
    Vertex(Vertex& other) { }
};

std::vector<Vertex> arrayOfVertices;

int main()
{
    arrayOfVertices.emplace_back(7.f);
}

This would not compile in Visual Studio, the only error it gives is:

"an internal error has occurred in the compiler"

If I enable the language extensions it compiles fine. If I keep the language extensions disabled and make the copy constructor take a const Vertex& it compiles fine.

So I tried on GCC on some online compilers and if the copy constructor doesn't take a const reference argument it won't compile, giving various errors. The one that seemed to make the most sense was:

error: invalid initialization of non-const reference of type ‘Vertex&’ from an rvalue of type ‘Vertex’

I thought that copy constructors didn't have to be const, in my case I would like to modify something in the other reference. I know that non-const arguments can't take r-value references, but I tested it and it turns out that in vector::emplace_back() the copy constructor isn't called at all:

#include <iostream>
#include <vector>

struct Vertex
{
    Vertex(float pos) 
    { 
        std::cout << "Calling constructor\n";
    }
    Vertex(const Vertex& other) 
    { 
        std::cout << "Calling copy constructor\n";
    }
};

std::vector<Vertex> arrayOfVertices;

int main()
{
    arrayOfVertices.emplace_back(7.f); // Normal constructor called if const,
                                       // doesn't compile if non-const

    auto buff = malloc(sizeof(Vertex)); // Placement new
    new (buff) Vertex(7.f); // Normal constructor called whether const 
                            // or non-const. This is what I thought emplace_back did

}

So I have no idea what's going on. I would like to know firstly why this is happening if the copy constructor isn't being called, and also if there's a way to take a non-const in my copy constructor in this case, that is, using vector::emplace_back(), because it seems this problem is arising only using vector::emplace_back().

Xavier Holt
  • 14,471
  • 4
  • 43
  • 56
Zebrafish
  • 11,682
  • 3
  • 43
  • 119
  • But the copy/move constructor have to be called in case of reallocation of the space by vector... I'd say all what happens here (maybe except non const copy constructor accepted by MS) has its reasonable rationale... – W.F. Oct 14 '17 at 11:40
  • @In the case the vector has to be reallocated the copy constructor would be called with an lvalue, that residing in the old container. – Zebrafish Oct 14 '17 at 11:45
  • have in mind that it would be very undesirable if the copy constructor could change the refered by lvalue reference old object – W.F. Oct 14 '17 at 11:48
  • 2
    Please note that Stephan Lavavej, the maintainer of the MS standard library, has repeatedly stated that the `/Za` switch is poorly tested and not really supported by the standard library. – Sebastian Redl Oct 14 '17 at 12:02
  • Another thing is that there's not really any good reason to have non-const copy constructors. Using `mutable` class members may be a slightly better solution to whatever motivation you have to do this – M.M Oct 14 '17 at 12:22
  • The preferred way to enforce more conformance is to use [dedicated Conformance switches](https://learn.microsoft.com/en-us/cpp/build/reference/zc-conformance). – user7860670 Oct 14 '17 at 12:30
  • I'm surprised no one has mentioned the `/permissive-` flag. – chris Oct 14 '17 at 22:23
  • @Zebrafish The copy constructor will *not* be called with an lvalue. It will be called with `std::move(old-value)`. That's not going to bind to a non-const reference. What sort of changes do you want to make in the previous value? You might be better off with a move constructor. – Martin Bonner supports Monica Oct 15 '17 at 08:07
  • @Marting Bonner I've ended up just using a move constructor and deleting the copy constructor. As far as I know actions which would normally call a copy constructor will call the move instructor instead. I'm not sure if that's only for RValues and std::move, for example Object obj1; Object obj2 = obj1; I think would call the move constructor, I'm probably wrong. – Zebrafish Oct 15 '17 at 09:38

2 Answers2

17

The issue is that you don't have a move constructor.

When you request std::vector to emplace_back something, it must make sure that it has enough storage to construct the new object. Part of this routine is to instantiate a bunch of code that moves the elements from the old buffer to any newly allocated buffer, if it's required. That code will be instantiated by the template even if there won't be a reallocation at run-time.

Your class has a user defined copy constructor, so the move constructor is implicitly deleted. As such, the attempt to move any element in the original buffer into the new, will be turned into an attempt to copy, by overload resolution. Your focus on placement new is in fact a red herring, the real issue is evident in this simple example:

Vertex v1{7.f},
       v2{std::move(v1)};
       // Error, the xvalue from `move` can't bind to a non-const reference

You can silence the error rather easily by bringing the move constructor back, for instance, by explicitly defaulting it:

struct Vertex
{
    Vertex(float) 
    { 
        std::cout << "Calling constructor\n";
    }

    Vertex(Vertex&&) = default;

    Vertex(Vertex&) 
    { 
        std::cout << "Calling copy constructor\n";
    }
};

Never forget that in C++11, the rule of 0/3 became the rule of 0/3/5. Think carefully about move semantics for you classes too.

StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458
  • So when is my non-const copy constructor called with an R-value reference? When it reallocates? When it reallocates it used the copy constructor to move all the old container elements, but those old container elements are lvalues. Which is the R-value? The one being emplaced back? Does it reallocate new memory, copy construct the existing ones over, and then construct the new emplaced back one in the last position of the new memory? – Zebrafish Oct 14 '17 at 11:53
  • Oh sorry, I see, it doesn't copy construct them over, it move constructs them, and I'm missing the move constructor. Weird, the error message doesn't mention this. – Zebrafish Oct 14 '17 at 11:55
  • 3
    @Zebrafish - No, at reallocation it attempts to explicitly move them. Not copy by default. Overload resolution can call the copy constructor as a fallback, when it accepts a const reference. But in your case, there is no viable overload at all. – StoryTeller - Unslander Monica Oct 14 '17 at 11:57
  • @Zebrafish - Yeah the error messages could use some work. – StoryTeller - Unslander Monica Oct 14 '17 at 11:58
  • Although adding the default move constructor works when I try it on the online compilers, Visual Studio still refuses to compile it, with the error "An internal error has occurred in the compiler". Even with the warnings turned all the way up that's the only message I get. – Zebrafish Oct 14 '17 at 14:16
  • 1
    @Zebrafish - MSVC has an implementation bug. That's unfortunate, but ultimately not a problem with your code – StoryTeller - Unslander Monica Oct 14 '17 at 14:20
  • Then I can just delete the copy constructor and define the move constructor, that seems to work. The differences are what exactly? If there is a move constructor available it'll default to using that instead of the copy constructor, right? – Zebrafish Oct 14 '17 at 14:31
  • @Zebrafish - Yes. What it means is that your class is now only move constructible and not copy constructible. IMO it's a better situation than a mutating copy c'tor. – StoryTeller - Unslander Monica Oct 14 '17 at 14:47
  • 2
    Remember that if you're using your class with `vector` it's a good idea to make sure the move constructor is `noexcept`, or else it will try to use the copy constructor, and if that's not available then you lose guarantees on exception safety of the vector. (The default move constructor should usually end up being `noexcept` implicitly.) – Daniel Schepler Oct 14 '17 at 21:40
  • FYI, the rule of 3 is actually wrong. It turns out you frequently need to implement copy ctor/assign without needing to implement destructors... but many people don't realize this. – user541686 Oct 14 '17 at 23:28
13

Obviously it's a compiler bug if the compiler gives an internal error.

emplace_back(7.f) uses the constructor Vertex(float pos) to emplace the object -- the copy-constructor is not directly involved.

The actual reason for the error is different. When you emplace in a vector, in general, a reallocation may occur. If it does, then all objects in the vector must be relocated to a new place in memory.

Clearly, it's a runtime condition as to whether or not reallocation occurs. It's not feasible to have a compile error at runtime; so the error must occur upon compile-time use of emplace_back if the objects do not support reallocation; even if the vector happens to be empty for this call.

The Standard terminology is found in C++14 Table 87: in order to emplace into a vector, the element type must be MoveInsertable and MoveAssignable.

Without going into too much detail, the combination of a non-const copy-constructor, and no move-constructor, means the object fails the MoveInsertable requirement, as the rvalue argument in said requirement will not bind to the non-const lvalue reference.

M.M
  • 138,810
  • 21
  • 208
  • 365
  • Which is the r-value exactly when it reallocates? All the old container elements are l values, and the emplace_back argument for the last new element is sent to the normal constructor, no? – Zebrafish Oct 14 '17 at 11:58
  • @Zebrafish Please see the description of MoveInsertable that I linked. The rvalue is hypothetical for purposes of specifying the MoveInsertable requirement: The contained type must be constructible from an rvalue. – M.M Oct 14 '17 at 12:01
  • 1
    You will probably find a typical vector implementation does the equivalent of std::move on the old element to move to the new location (with some extra hokum to deal with non-noexcept move constructors) – M.M Oct 14 '17 at 12:05