11

I have simplified my code as follows.

#include <vector>
class NoncopyableItem {
 public:
  NoncopyableItem() { }
  NoncopyableItem(NoncopyableItem &&nt) { };
};
class Iterator {
  friend class Factory;
 public:
  ~Iterator() { }  // weird
 private:
  Iterator() { }
  std::vector<NoncopyableItem> buffer_;
};
class Factory {
 public:
  Iterator NewIterator() {
    return Iterator();
  }
};
int main() {
  Factory fa;
  auto it = fa.NewIterator();
  return 0;
}

I want to leverage the RVO (return value optimization) in function NewIterator, but I got the following error:

In file included from /usr/lib/gcc/x86_64-pc-cygwin/4.9.3/include/c++/vector:62:0,
                 from /cygdrive/c/Users/DELL/ClionProjects/destructor_test/main.cpp:1:
/usr/lib/gcc/x86_64-pc-cygwin/4.9.3/include/c++/bits/stl_construct.h: In instantiation of 'void std::_Construct(_T1*, _Args&& ...) [with _T1 = NoncopyableItem; _Args = {const NoncopyableItem&}]':
/usr/lib/gcc/x86_64-pc-cygwin/4.9.3/include/c++/bits/stl_uninitialized.h:75:53:   required from 'static _ForwardIterator std::__uninitialized_copy<_TrivialValueTypes>::__uninit_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = __gnu_cxx::__normal_iterator<const NoncopyableItem*, std::vector<NoncopyableItem> >; _ForwardIterator = NoncopyableItem*; bool _TrivialValueTypes = false]'
/usr/lib/gcc/x86_64-pc-cygwin/4.9.3/include/c++/bits/stl_uninitialized.h:126:41:   required from '_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = __gnu_cxx::__normal_iterator<const NoncopyableItem*, std::vector<NoncopyableItem> >; _ForwardIterator = NoncopyableItem*]'
/usr/lib/gcc/x86_64-pc-cygwin/4.9.3/include/c++/bits/stl_uninitialized.h:279:63:   required from '_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, std::allocator<_Tp>&) [with _InputIterator = __gnu_cxx::__normal_iterator<const NoncopyableItem*, std::vector<NoncopyableItem> >; _ForwardIterator = NoncopyableItem*; _Tp = NoncopyableItem]'
/usr/lib/gcc/x86_64-pc-cygwin/4.9.3/include/c++/bits/stl_vector.h:324:32:   required from 'std::vector<_Tp, _Alloc>::vector(const std::vector<_Tp, _Alloc>&) [with _Tp = NoncopyableItem; _Alloc = std::allocator<NoncopyableItem>]'
/cygdrive/c/Users/DELL/ClionProjects/destructor_test/main.cpp:7:7:   required from here
/usr/lib/gcc/x86_64-pc-cygwin/4.9.3/include/c++/bits/stl_construct.h:75:7: error: use of deleted function 'constexpr NoncopyableItem::NoncopyableItem(const NoncopyableItem&)'
     { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); }
       ^
/cygdrive/c/Users/DELL/ClionProjects/destructor_test/main.cpp:2:7: note: 'constexpr NoncopyableItem::NoncopyableItem(const NoncopyableItem&)' is implicitly declared as deleted because 'NoncopyableItem' declares a move constructor or move assignment operator
 class NoncopyableItem {
       ^
CMakeFiles/destructor_test.dir/build.make:62: recipe for target 'CMakeFiles/destructor_test.dir/main.cpp.o' failed

According to cppreference.com, NewIterator() should meet the requirement of RVO. However, it seems that the compiler trys to call the default copy constructor of Iterator, then fails because Iterator.buffer_ is noncopyable.

Well, to my suprise, if I delete the destructor of Iterator in L#13, the code works fine.

Why does the destructor affect the RVO behavior of the compiler?

1 Answers1

11

First of all, forget about RVO in this context. It is a legal optimization, but even when it does happen, the code must be legal without it.

So with this in mind, we look at

auto it = fa.NewIterator();

This line attempts to construct a new Iterator from a temporary Iterator. For this to work, we need either of the following two:

Iterator(const Iterator&); //or
Iterator(Iterator&&);

Now in the code you posted, trying to use the implicitly declared Iterator(const Iterator&); will result in the compiler error you showed because the copy constructor of the non-static member buffer_ fails to compile.

The second candidate is not generated because Iterator has a user defined destructor.

If you remove the user defined destructor, the compiler will generate the move constructor Iterator(Iterator&&); and use it as we are constructing from a temporary. Or maybe it won't and do RVO instead, but it could use it, and that is the important part.


Or some other user declared constructor that makes the line legal of course. But above are the two commonly compiler generated ones you apparently ask about.

Baum mit Augen
  • 49,044
  • 25
  • 144
  • 182
  • 1
    Not quite right. `Iterator(const Iterator&);` will be implicitly declared and defined as defaulted (not deleted). However, the copy constructor of `std::vector` will fail to instantiate. (`std::vector` has an accessible copy constructor as far as the check for member copyability is concerned.) Hence the OP's error. – T.C. Dec 08 '15 at 18:32
  • @T.C. Thank you, fixed. It will however be defined as deleted as of 12.7/11 in N4140, right? – Baum mit Augen Dec 08 '15 at 18:40
  • No, `Iterator`'s copy constructor is not actually deleted, because the vector nominally *does have* an accessible and non-deleted copy constructor. `static_assert(std::is_copy_constructible{}, "");` will *not* fire. The implicit definition of `Iterator`'s copy constructor triggers the instantiation of the vector's copy constructor, which causes a hard error. – T.C. Dec 08 '15 at 18:44