6

I think Boost::variant is busted in 1_54. I am trying to use a std::unique_ptr with as a bounded type in boost variant.

According to the 1_54 documentation, variant needs to be copy constructable or Move Constructable.

http://www.boost.org/doc/libs/1_54_0/doc/html/variant/reference.html

So I implemented the move constructors and disabled the copy constructors in my code.

When I try to assign something to the variant object it fails to compile. I have tried various different things including using std::move to assign data to the variant object, but nothing seems to work. Following the compilation error stack trace I determined the problem is in variant.hpp, where its trying to backup the rhs data. I would like to know what you guys think and let me know if I right to assume boost variant documentation is wrong.

Thanks in advance.

I am compiling with vs2010 and using C++11.

Here is my Test Code:

#include <iostream>
#include <memory>
#include <utility>
#include <vector>
#include <string>


#pragma warning (push)
#pragma warning (disable: 4127 4244 4265 4503 4512 4640 6011)
#include <boost/optional.hpp>
#include <boost/variant.hpp>
#include <boost/ref.hpp>
#include <boost/shared_ptr.hpp>
#pragma warning (pop)

#include <boost/foreach.hpp>
#include <boost/format.hpp>
using boost::format;
using boost::str;

using namespace std;

class UniqueTest
{
};

class Foo
{
  public:
  std::unique_ptr<UniqueTest> testUniquePtr;

     Foo()      { std::cout << "Foo::Foo\n";  }
     Foo (Foo&& moveData)
     {
     }               

     Foo& operator=(Foo&& moveData)
     {
     return *this;
     }

  private:
     Foo(Foo& tt);
     Foo& operator=(const Foo& tt);

};


int main()
{

  Foo x = Foo();
  boost::variant<std::wstring,Foo> m_result2;
     std::wstring  testString = L"asdf";

  m_result2 = testString; //Fails
  //m_result2 = std::move(testString); //Fails
  //m_result2 = std::move(x); //Fails

  boost::get<Foo>(m_result2).testUniquePtr.get ();
  return 0;
}
David Brown
  • 13,336
  • 4
  • 38
  • 55
luke signh
  • 139
  • 2
  • 13
  • 1
    That's not what "disabling the copy constructor" means :-S And you've only "implemented" the move constructor in the widest sense of the word. Tip: Remove *all* your manual move and copy constructors; the implicily provided ones are just fine. – Kerrek SB Nov 01 '13 at 00:23
  • unique_ptr object is not copyable so I have to provide the move constructors. Move constructors are not auto generated. I only implement the move constructors at a very basic level, because I would like to get it to compile first. – luke signh Nov 01 '13 at 00:31
  • 1
    move constructors _are_ auto-generated, if you don't have user-defined copy operations or destructor. Remove all your broken special member functions and let the compiler provide them correctly – Jonathan Wakely Nov 01 '13 at 00:36
  • Following your suggestions, I also removed all my manual move and copy constructors and the code did not compile because of the unique_ptr member variable. std::unique_ptr member is not copy construct able by definition – luke signh Nov 01 '13 at 00:41
  • Oddly, for me [this](http://pastebin.com/DVRwC6RM) compiles, while [this](http://pastebin.com/58pEA5xV) does not (on both gcc 4.7 and clang 3.2) – David Brown Nov 01 '13 at 00:42
  • 1
    @lukesignh, then that's a MSVC bug, it compiles with a conforming C++11 compiler – Jonathan Wakely Nov 01 '13 at 00:43
  • 1
    @DavidBrown, you need to make the user-defined move ops `noexcept` (as the defaulted ones are implicitly). `variant` will not use move ops if doing so could throw, to avoid data loss, in order to provide the strong exception safety guarantee – Jonathan Wakely Nov 01 '13 at 00:45
  • @Jonathan ah yes, I always forget about `noexcept` – David Brown Nov 01 '13 at 00:47
  • 1
    @lukesignh, I don't know if it will make any difference for VS2010, but you could try making your move constructor `noexcept`, to see if that makes `variant` use it in preference to your private (and therefore unusable) copy constructor. That should be implicit if you don't define it manually, but VS2010 predates the C++11 standard and I have no idea how much of it is supported – Jonathan Wakely Nov 01 '13 at 00:51
  • @JonathanWakely: It's not an MSVC bug, that wording wasn't Standard at the time of implementation. They also don't have `noexcept`, `=default` or quite a lot more. – Puppy Nov 01 '13 at 00:58
  • @David in your code you do not have a unique::ptr member variable object and that is key to my troubles. I can get things to compile if I do not use a unique ptr just fine. So I think the issue is that I am using a std::unique_ptr variable as a bounded type in a boost variant. According to the documentation in Boost 1_53, I should not be able to use a unique_ptr in a boost::variant because a copy constructor is required; However, for 1_54 either a move constructor or a copy constructor is required, so I should be able to use it. My hypothesis is that boost:: 1_54 documentation is wrong. – luke signh Nov 01 '13 at 01:10
  • here is 1_54 vs 1_53 http://www.boost.org/doc/libs/1_54_0/doc/html/variant/reference.html http://www.boost.org/doc/libs/1_53_0/doc/html/variant/reference.html – luke signh Nov 01 '13 at 01:11
  • Does my code as is compile for anyone? – luke signh Nov 01 '13 at 01:11
  • 1
    @lukesignh: I think more likely, Boost simply doesn't support VS2010 for that feature. It doesn't have enough C++11 support. – Puppy Nov 01 '13 at 01:14
  • 1
    @lukesignh, you're using a pre-C++11 compiler that doesn't support C++11 properly, don't blame the Boost documentation. I've already said it compiles with a conforming compiler (I tried G++ and Clang) if you _either_ let the compiler generate all the special members, _or_ make the move ctor `noexcept` – Jonathan Wakely Nov 01 '13 at 01:31
  • You should also remove all the unused cruft from your example, you don't use `` or `` or most of the other headers you include. – Jonathan Wakely Nov 01 '13 at 01:40
  • @lukesignh you can check it yourself using http://coliru.stacked-crooked.com/, http://ideone.com etc. – sehe Nov 01 '13 at 07:38
  • @sehe thanks. I did not know there were so many online compilers, I could use to test my code. – luke signh Nov 04 '13 at 18:38
  • Further looking into the boost documentation I found this for 1_54_0: Current Approach: Temporary Heap Backup (Talking about Boost variant design) .... The algorithm for assignment would proceed as follows: Copy-construct the content of the right-hand side to the heap; call the pointer to this data p. Destroy the content of the left-hand side. Copy p to the left-hand side storage. link: http://www.boost.org/doc/libs/1_54_0/doc/html/variant/design.html#variant.design.never-empty.heap-backup-solution This leads me to believe boost variant by design needs to be copy constructable. – luke signh Nov 04 '13 at 18:38
  • so I cannot use a unique::ptr as a boost::variant bounded type.. it should not be a compiler issue. – luke signh Nov 04 '13 at 18:44

1 Answers1

4

Does my code as is compile for anyone?

No it doesn't, variant will try to invoke the copy constructor, which is missing. (Foo::Foo(Foo const&) isn't even declared):

boost/variant/variant.hpp|756 col 9| error: no matching function for call to ‘Foo::Foo(const Foo&)’

Even if you did declare it, it wouldn't work:

test.cpp|43 col 6| error: ‘Foo::Foo(const Foo&)’ is private

It has been mentioned in the comments but you need

  • to make the copy constructor a copy constructor (for good style)

    Foo(Foo const& tt) = delete;
    Foo& operator=(const Foo& tt) = delete;
    
  • to make the move constructor/assignment noexcept, otherwise (like std::vector) variant will refuse to move things because it wouldn't be exception safe.

    Foo(Foo && moveData) noexcept { }    
    Foo& operator=(Foo && moveData) noexcept { return *this; }
    

Here's a simplified sample that compiles on GCC and Clang:

#include <iostream>
#include <memory>
#include <string>
#include <boost/variant.hpp>

struct UniqueTest { };

struct Foo
{
public:
    std::unique_ptr<UniqueTest> testUniquePtr;

    Foo() { std::cout << "Foo::Foo\n"; }
    Foo(Foo && moveData) noexcept { }

    Foo& operator=(Foo && moveData) noexcept { return *this; }

    Foo(Foo const& tt) = delete;
    Foo& operator=(const Foo& tt) = delete;
};


int main()
{
    Foo x = Foo();

    boost::variant<std::wstring, Foo> m_result2;

    std::wstring  testString = L"asdf";
    m_result2 = testString; //Fails
    //m_result2 = std::move(testString); //Fails
    //m_result2 = std::move(x); //Fails
    boost::get<Foo>(m_result2).testUniquePtr.get();
}
sehe
  • 374,641
  • 47
  • 450
  • 633
  • I just saw your post. Well, VS2010 does not support nodelete or noexcept keywords. Do I have any alternatives besides defining them myself? – luke signh Nov 04 '13 at 18:53